Skip to content

fix(cli): snapshot rejects a negative --at and builds its sheet from its own frames - #6179

Open
miguel-heygen wants to merge 4 commits into
mainfrom
fix/snapshot-at-and-sheet-from-run-files
Open

miguel-heygen wants to merge 4 commits into
mainfrom
fix/snapshot-at-and-sheet-from-run-files

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

What

Two fixes to hyperframes snapshot:

  1. --at now refuses a negative, infinite, non-numeric or empty time before anything runs, with --at must be comma-separated times in seconds, each 0 or more and exit 1. Before, --at -1 captured a still named frame-00-at--1s.png, --at Infinity captured one at Infinity, a bad entry like NaN was silently dropped, 1junk was read as 1, and --at "" fell back to the default frames.
  2. The contact sheet is built from the frames this run wrote, not from every frame-*.png in the output folder. Before, a user's own frame-hero.png in an --output folder landed in the sheet.

Why

Stacked on #6176, which makes --output clear only the images whose names match the snapshot pattern. A frame-00-at--1s.png does not match that pattern, so the next run would leave it behind; and a user's frame-hero.png now survives the clear, so the folder glob put it in the sheet.

Related work

Stacked on #6176 (base is its branch). Will retarget to main once #6176 merges.

How

  • parseSnapshotAt sits beside parseSnapshotRange, parses each entry the same way (Number, empty is invalid) and throws the same way (the CLI boundary prints the message and exits 1). It runs next to the --range parse, so the refusal comes before lint, the output folder or the browser. --keep's duplicate-name check moved with it, unchanged.
  • createSnapshotContactSheet takes the list of frame paths instead of a folder. Its only caller passes the paths the capture returned (the same list it already reports as artifacts). Order is capture order; labels still come from the at-<time>s part of each name. --keep and --review behave as before.
  • Choice worth a look: an empty entry (--at 1,) and a value with trailing text (--at 1.5s) are now refused, matching --range. The documented examples all use plain numbers.

Test plan

Through the real command (runCommand(snapshotCommand, ...)), in snapshot-contactSheet.test.ts:

  • refuses --at <value> before writing anything for -1, 0.5,-1, Infinity, NaN, 1,, 1junk and an empty value: the run rejects with the message, takes no screenshot and creates no output folder.
  • builds the sheet from this run's frames, not another frame-*.png in the folder: an output folder holding frame-hero.png plus a two-frame run; a stub image library records every file the sheet reads, which must be exactly the two frames.

Measured: on #6176's code the new tests fail (9 failed, 15 passed: the seven --at cases resolve or fail with only Command failed, the sheet reads frame-hero.png, and the updated call-shape assertion sees a folder). With the fix: 24 passed, 3 runs in a row, together with snapshotSampling.test.ts. oxfmt --check, oxlint and tsc --noEmit for packages/cli are clean.

Not exercised: a real image library building a real sheet (the test stubs it), the CLI binary end to end, and the rest of the CLI suite.

  • Unit tests added/updated
  • Manual testing performed
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments"

Base automatically changed from fix/snapshot-output-deletes-only-its-own-frames to main October 24, 2026 08:58

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head c6552c15

The --at parse now runs before lint, the output folder and the browser. The contact sheet is built only from the paths this run returned. CI is green on this head.

Scope

Read snapshotSampling.ts, snapshot.ts, contactSheet.ts and the test diff. --at "" now reaches parseSnapshotAt (=== undefined instead of truthiness) and is refused; a bare --at (citty true → "true") is refused too. Sheet order goes from a filename sort to capture order; that is the same order because the names carry a frame-NN index. The branch predates #6176's squash-merge, but its 6176 commits match main and GitHub reports it mergeable.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant