[miguel-heygen]

hyperframes snapshot no longer finishes "complete" when an on-screen video's frame could not be read; it fails after saving the stills, so an agent reviewing the sheet is told instead of looking at black or stale videos.

Cause

When a video's frame fails to read (a probe or extract error), snapshot warned and carried on, then exited 0 with ok: true. The warning is easy to miss; the sheet looks like a finished review.

Change

snapshot counts on-screen video frames whose read raised that warning. After saving the stills it fails (exit 1, ok: false with --json), naming how many frames and the folder they were saved to. A video with no source at all, and a readable clip held past the end of its source (no last frame to hold, as with HEVC in headless Chrome on Linux), stay warnings, as before.

Test

fails without the fix:

• snapshot.unreadableVideo.browser.test.ts › "fails, naming the folder, when a video on screen cannot be read" (a project whose clip.mp4 is not a video; red when the failure is removed)

Guard: the same file's "does not fail for a readable clip held past the end of its source" (a 1 s HEVC clip shown to 3.5 s) is red if every missing frame is counted as unread.

Checked on a clean run: the two snapshot browser files pass 3 runs in a row (4 tests), snapshot and contact-sheet unit tests pass (123), the ffprobe argv contract test passes, tsc and oxfmt clean.

Not changed: the -- before the input in the engine and snapshot ffprobe calls stays, as the argv contract test requires. An earlier version of this PR replaced it with a helper; that is kept aside until the question of whether any ffprobe rejects -- is settled.

Size

About 100 changed lines: one counter and one failure in snapshot.ts, plus two tests.

[heygengenesis]

Requesting changes at 3a7f890f. There are two blockers.

1. A valid HEVC clip held past its source end now fails snapshot, and the message calls it unreadable. Repro on Linux, using the harness from snapshot.unreadableVideo.browser.test.ts with a real clip in place of the text file:

• clip.mp4 is a 1 s libx265 clip (-tag:v hvc1) with data-start="1" data-duration="3". The snapshot is taken at --at 3.5, so the clip is still on screen after its source has ended.
• On this head: {"ok":false,"error":{"message":"✗ 1 video frame(s) could not be read, so those videos are black or stale…"}}, exit 1.
• On the base d17b9583: ok: true, with only the "frames missing" warning.
• Controls: the same HEVC clip at 1.5 s passes, and an H.264 clip at 3.5 s passes. H.264 passes because Chrome knows its srcDuration, so holdLastFrame is set.

Cause: headless Chrome on Linux can't decode HEVC, so srcDuration is 0 and holdLastFrame stays false. probeContainingSourceFrameSeek then returns null for time >= format.duration, and the new counter treats that null the same as a probe or extract that failed. The file can be read fine. The snapshot just has no last frame to hold. Phone footage is very often HEVC, so this turns a still that was previously a bit off into a hard failure for valid projects, which is the reverse of what the PR is for. Possible fixes: decide past-end holding from the probed format.duration when the browser gives no duration, or count only FrameExtractError and missing input as unread, not the past-end null. Please also add a test for this case.

2. The Comments workflow fails on this head. It's the comment-share ratchet: snapshot.ts 136→137 and ffprobe.ts 204→205, one added comment line each. In ffprobe.ts, the new JSDoc is stacked under runFfprobe's existing /** Spawn ffprobe… */. That orphans runFfprobe's doc onto inputPathArg. Moving inputPathArg (with its doc) above that block fixes the doc placement. You still need to cut one comment line in each file to get under the ratchet.

Verified OK:

• Both tests on the fails without the fix: line fail at the merge base and pass with the fix. The strict-ffprobe test fails with Unable to find a suitable output format for '--'. The browser test fails with expected true to be false.
• inputPathArg("-") → ./- no longer goes to stdin, which is fine.

[github-actions]

Edit accuracy: accurate 271 (base branch 271), smooth 110 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

• seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true

[miguel-heygen]

Pushed 1712123.

• Blocker 1 (valid HEVC clip held past its source end failed): the counter treated the past-end null like a failed read. It now counts a frame as unread only when its read raised a warning, the same test the --against path already uses (warnings.length > warned). Added does not fail for a readable clip held past the end of its source (your 1 s HEVC clip shown to 3.5 s): red when every null is counted, green now. The unreadable-clip test is still red without the failure.
• Blocker 2 (Comments ratchet): removed the field comment in snapshot.ts and the JSDoc on inputPathArg, and moved inputPathArg above runFfprobe's own doc so that doc is back on runFfprobe. Both files are back at their previous comment counts.
• On miga: the two snapshot browser files pass 3 runs in a row (4 tests), snapshot unit 98, tsc and oxfmt clean.

[heygengenesis]

Approving at 17121230. Both blockers from my review at 3a7f890f are cleared:

• The HEVC clip held past its source end no longer fails. An on-screen frame now counts as unread only when frameOrWarn recorded a FrameExtractError, so a past-end null from the seek probe stays a warning. The new test "does not fail for a readable clip held past the end of its source" is the repro from my review.
• Comments passes. runFfprobe's JSDoc is back on runFfprobe.

Verified locally at this head:

• snapshot.unreadableVideo.browser.test.ts passes 2/2 and snapshot.test.ts passes 98/98.
• With the counter guard reverted to unreadVideoFrames += 1, the new test fails with expected false to be true. All three fails without the fix: tests are confirmed: the other two failed at the merge base, as I checked in the earlier review.

Non-blocking nit: warned is a snapshot of the shared warnings array, but up to 4 extractions run at once. A past-end null from one clip can be counted when another clip fails at the same moment. The command fails either way because of the real failure, so only the reported count can be inflated. A per-call flag, such as catching inside the callback, would make the count exact.

Other CI on this head was still running when I approved.

[miguel-heygen]

Pushed (rebased on main), narrowed to the loud failure.

The helper that replaced -- is dropped from this PR. packages/producer/src/utils/ffprobeArgvContract.test.ts requires -- immediately before the input in every ffprobe call (it failed on the previous head), and it also broke the contact-sheet tests, which mock @hyperframes/engine without the new export. I also could not reproduce an ffprobe that rejects -- (two 8.x builds accept it), so changing a repo-wide contract on that report is not something this PR should do. The helper version is kept on a local branch.

What remains is the part that stands alone: a frame that could not be read now fails the snapshot, with both guard tests described in the body. Unit (123), the two browser files x3, the contract test, tsc and oxfmt pass on a clean run.

[heygengenesis]

Approving at 184448a7. The scope is now the unread-frame failure only. The -- removal and inputPathArg are gone, and snapshot.test.ts, ffprobe.ts and ffprobe.test.ts match main.

Verified locally at this head (merge base cd598426):

• snapshot.unreadableVideo.browser.test.ts passes 2/2 and snapshot.test.ts passes 97/97.
• fails without the fix: holds. With snapshot.ts at main, "fails, naming the folder, when a video on screen cannot be read" fails with expected true to be false. The HEVC held-past-end guard still passes there.
• The snapshot.ts change is the same counter and failure I approved at 17121230: a frame counts only when frameOrWarn recorded a warning.
• Comments passes.

If any ffprobe does reject --, those probe errors now make the snapshot fail loudly instead of passing black. That's consistent with this PR, and the error names the cause.

The non-blocking nit still stands: warned snapshots a warnings array shared across 4 concurrent extractions, so the reported count can be inflated when a real failure happens at the same moment. Other CI on this head was still running when I approved.