Feat/readme screenshot

[nva138]

Closes #83.

Screenshot is now a file in the repo, npm run screenshots:update regenerates it and npm run screenshots:check compares. The check is a step in the existing Playwright workflow, and the diff image lands in test-results/, which already gets uploaded.

I went with plain scripts instead of a spec, because a test writing into the repo felt wrong: every local test:e2e would quietly rewrite the committed image. Put them in scripts/ next to the other two. Viewport and locale come from devices['Desktop Chrome'] so nothing is copy-pasted out of the Playwright config.

Tolerance is 0.1%. Two runs here give a byte-identical file, and shifting the signature by 20px gives 0.24%, so that felt like a reasonable spot. My reference image is from macOS though and CI regenerates on Linux, so the first run might say otherwise.

One thing: the empty white box above the signature is the invisible icon from #107. If that goes in first I'll regenerate.

[vitormattos]

The current CI is showing 1.9069% difference because the committed screenshot was generated on macOS and the CI generates it on Linux.

Could we make screenshots:update and screenshots:check generate the image in the same reproducible environment? For example, both could use the same Linux/Playwright environment.

I don't think increasing the tolerance is enough here, because the PR description says that moving the signature by 20px gives only 0.24%. A tolerance high enough to accept the macOS/Linux difference could also hide real visual changes.

[nva138]

Thanks, makes sense.

2, 3 and 4: agreed. 4 is in the latest commit, the generated image now goes to test-results/ next to the diff. Viewport and regeneration I'd do together once #107 is in, both need a fresh image anyway.

For 1, you're right that a bigger tolerance is the wrong fix. Idea: generate inside mcr.microsoft.com/playwright:v1.63.0-noble (matches our @playwright/test) and run the CI check in the same image, then tolerance can go to zero. Downside is the container needs its own npm ci, rollup ships platform specific binaries so a macOS node_modules won't work in there. Slow, but it's a rare command.

Is that roughly what you meant or were you after something lighter?

Also merged current main into the branch. The PDF.js CSP changes did not move the screenshot, still zero pixels difference locally.

[vitormattos]

I think your container idea can work, but before deciding I would prefer to measure the cost.

Since this check will run on pull requests, could you test both approaches in CI?

• reuse the current Playwright job and run the screenshot check there;
• run the screenshot generation/check inside the Playwright container as you suggested.

Then we can compare the workflow time and see if the extra container setup and npm ci have a relevant cost here.

Without measuring it in this repository, I think we would just be guessing which approach is better.

[vitormattos]

I also noticed that you updated the branch by merging upstream/main into it.

For this kind of update, I suggest using rebase instead of merge. It keeps the PR history linear and avoids an extra merge commit that is only there to bring the branch up to date with main.

A common flow would be:

git fetch upstream
git rebase upstream/main
git push --force-with-lease

If there are conflicts, resolve them during the rebase, run git add for the resolved files, then continue with:

git rebase --continue

Using --force-with-lease is important here because the rebase rewrites the branch history, while still protecting against overwriting remote changes you do not have locally.

[nva138]

Measured both in the same run:

Container job: 39s — image pull 26s, npm ci 6s, check 3s. No setup-node, no playwright install.
Existing job: 61s — setup 5s, npm ci 6s, playwright install 22s, e2e 20s, check 3s.

The 26s pull is basically paid for by the 22s of playwright install we skip. They run in parallel and the container job finished 22s ahead of the e2e one, so the workflow doesn't get slower, it just costs runner minutes.

Interesting bit: the container reports 2.2648% against the committed image, the runner 1.9069%. Both Linux, different fonts. So Linux alone isn't enough, it has to be the same pinned image on both sides.

[nva138]

Both done. 900x1000 now, the header block is a fixed ~320px so making it shorter just gave the PDF less room, narrower plus taller worked better. Regenerated after #107, the action icons are visible now.

[vitormattos]

Thanks for the benchmark. The numbers are enough to choose the direction.

Please keep the screenshot validation in the separate pinned Playwright container job. It runs in parallel and, based on your measurements, does not add relevant wall-clock time to the workflow.

For the final implementation, please:

• remove npm run screenshots:check from the existing e2e job, so the screenshot is checked only once;
• keep the separate container job as the screenshot validation job;
• regenerate img/screenshot/demo.png using the same pinned container environment used by CI, so the committed baseline and CI use the same browser/fonts/rendering environment;
• make sure the screenshot job uploads the generated screenshot and visual diff when it fails;
• address the remaining open review comment in scripts/check-screenshots.mjs so the generated screenshot is also saved when the image dimensions are different.

After these changes, the screenshot workflow should be stable and green. The other review comments about the viewport and regenerating after #107 are already resolved.

[vitormattos]

Suggested change

	await page.locator('.overlay').first().click({ position: { x: 100, y: 300 } })
	await page.locator('.overlay').first().click({ position: { x: 140, y: 260 } })

[vitormattos]

I made a local test and reduced a bit more. What do you think about this? I also changed the element position too. I think that a smaller screenshot like this will be best to stay at README.

Suggested change

	viewport: { width: 900, height: 1000 },
	viewport: { width: 840, height: 670 },

[nva138]

Good catch, fixed. It writes demo-screenshot-current.png before throwing now. No diff in that case, pixelmatch can't compare two different sizes anyway.

[vitormattos]

The screenshot validation is already handled by the dedicated container job, so we don't need to run it again in the e2e job.

Suggested change

	- name: Check README screenshot
	run: npm run screenshots:check

[vitormattos]

This was useful while benchmarking, but now this is the screenshot validation job. If the committed screenshot is outdated, this job should fail.

Suggested change

continue-on-error: true

[vitormattos]

The generated screenshot and visual diff are written to test-results/, but this job does not upload them yet. We need these artifacts when the comparison fails.

Suggested change

	- name: Check README screenshot
	run: npm run screenshots:check
	- name: Check README screenshot
	run: npm run screenshots:check
	- name: Upload screenshot artifacts
	if: ${{ failure() }}
	uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
	with:
	name: screenshot-diff
	path: test-results/
	if-no-files-found: ignore
	retention-days: 7

[vitormattos]

After applying these changes:

1. regenerate the screenshot inside the pinned Playwright container;
2. commit the updated img/screenshot/demo.png;
3. push the commit and confirm that the screenshot validation job is green.