fix(cli): honor authored playback rate in snapshots

[miguel-heygen]

Summary

Snapshots of trimmed or accelerated video clips now extract the same source frame as preview and render. Authored data-playback-rate takes precedence over the browser’s unchanged default rate, and natural media duration uses that same effective rate.

Test plan

• packages/cli/src/commands/snapshot.test.ts (34 passed)
• CLI typecheck
• oxlint and oxfmt --check on changed files
• Exact Terra HEVC fixture at composition 3.85s: snapshot matches source 6.5896s with RMSE 0; previous 6.0s frame differs with RMSE 0.32117

---

[somanshreddy]

Reviewed at head 7fac2a8 — correct fix, reads as approvable. (Leaving this as a comment rather than a formal stamp — the review-swarm routing isn't the sign-off authority my approve rides.) Direct manual pass, not a Codex run, for a 2-file numeric fix.

What I verified

• The fallback is byte-identical to the old behavior — no regression on existing clips. I re-executed resolveSnapshotVideoPlaybackRate across boundary input classes against an oracle of the old inline logic (rawRate = v.defaultPlaybackRate, then clamp-or-1). Every case with no/invalid authored rate (undefined, "", "0", "-2", "abc", defaultRate NaN/0) returns exactly what the old code returned. So non-accelerated clips (the common case) are unaffected.
• Authored rate is honored and clamped correctly: "1.8"→1.8, "100"→5, "0.01"→0.1, lenient "1.8abc"→1.8, zero/negative rejected to the default. The >0 && isFinite guard before the clamp is right.
• The duration relocation is formula-preserving. It moved out of the browser eval to Node but keeps authoredDuration ?? (srcDuration>0 ? max(0,(srcDuration-mediaStart)/rate) : Infinity) — now dividing by the resolved rate, which is the point of the fix ("natural duration uses the effective rate").
• relTime = (time - start) * rate + mediaStart is the right direction: at rate R, wall-clock elapsed E maps to source mediaStart + E·R.
• No orphaned references: candidate.playbackRate/candidate.duration are removed from the browser return; the new locals are computed outside and spread into the active item at the return. Nothing downstream reads a now-undefined field.

Non-blocking nit

The new unit test pins only the happy path ({authoredRate:"1.8", defaultRate:1} → 1.8). The branches that would regress silently are the fallback and the clamp — worth pinning:

expect(resolveSnapshotVideoPlaybackRate({ authoredRate: undefined, defaultRate: 2 })).toBe(2);   // no-regression guarantee
expect(resolveSnapshotVideoPlaybackRate({ authoredRate: "0", defaultRate: 1.5 })).toBe(1.5);      // invalid -> default
expect(resolveSnapshotVideoPlaybackRate({ authoredRate: "100", defaultRate: 1 })).toBe(5);        // clamp high
expect(resolveSnapshotVideoPlaybackRate({ authoredRate: "0.01", defaultRate: 1 })).toBe(0.1);     // clamp low

Scope note: I verified the CLI-side math is self-consistent and the fallback is regression-free; the CLI↔render parity itself I took from your exact RMSE-0 Terra fixture rather than re-running the render pipeline (repo isn't local here). The one thing I did not independently trace is that the render/preview side reads data-playback-rate with the same semantics — the fixture is the evidence for that.

[jrusso1020]

Approved at 7fac2a86dbe78631fe880cc27deb6dffad16df2d.

Authorization: Miguel's "stamp pls" on this round-2 batch in the review thread, the same route used for #3534. Read the code myself before casting rather than stamping on Somu's read.

Somu's core claim holds, and I re-derived it rather than trusting it. resolveSnapshotVideoPlaybackRate is exactly the old expression when nothing is authored: an absent or unparseable data-playback-rate gives Number.parseFloat("") as NaN, fails the isFinite && > 0 test, and falls through to input.defaultRate, which then runs the identical isFinite && > 0 ? clamp : 1. Non-accelerated clips cannot regress. Authored values win and clamp to [0.1, 5]. A "0" or negative authored rate correctly falls back rather than clamping up to 0.1, which is the right reading of "invalid" versus "small".

The duration relocation is formula-preserving. authoredDuration is null in exactly the cases the old ternary would have skipped, so authoredDuration ?? (srcDuration > 0 ? ... : Infinity) reproduces the old branch order, now divided by the corrected rate. That is the actual fix: the old code divided by defaultPlaybackRate, so an accelerated clip got a clip duration too long by the authored factor.

No orphaned references. I checked every surviving candidate.* read: all hit fields that still exist. playbackRate and duration became locals and are re-attached to the active entry by the spread, so downstream consumers are unaffected.

Two non-blocking notes.

1. The untested branch is the one protecting the status quo. The new test pins "1.8" -> 1.8, which is the new behavior. The fallback branch is what every existing non-accelerated clip flows through, and it has no direct coverage. Same shape as the gap on #3526: the branch that changed got a test, the branch that must not change did not. Somu listed the boundary cases on the PR; the fallback one is the load-bearing one.

2. Minor structural upside worth naming. Number.POSITIVE_INFINITY is now produced in Node instead of being returned across the page.evaluate boundary, so the value no longer depends on the driver's handling of non-JSON numbers. I did not chase which driver is in use, so I am not claiming the old path was broken, only that this removes the dependency.

CI at this head: all eight ruleset-required contexts green on latest attempt, nothing in flight. Note there are failed and cancelled earlier attempts at this same head, including Test, so a per-name query without the latest-attempt qualifier reads red here.

Approve only. Not merging.

Review by Rames