fix(engine): classify a single-keyframe video as problematic, not healthy

[miga-heygen]

Summary

• analyzeKeyframeIntervals (used only by an advisory compile-time warning for sparse-keyframe videos) returned isProblematic: false whenever a video had fewer than 2 detected keyframes — but exactly one keyframe is the worst case for seek performance, not the healthy one: every seek past frame 0 must decode the entire file. A library scan found dozens of single-GOP files (a few seconds to ~10s each) silently skipping this check.
• The new single-keyframe branch treats the remaining span of the video stream's own duration as one uninterrupted GOP and flags it against the same >2s threshold already used for the multi-keyframe case (now a single named constant instead of a literal repeated in two branches).
• Callers must now pass the video stream's own duration/start (VideoMetadata.videoStreamDurationSeconds/videoStreamStartSeconds), not a container-level duration — a container whose audio track outlasts a short video track would otherwise overstate the GOP span. ffprobe's frame timestamps are absolute presentation timestamps rather than relative to stream start, so a re-muxed/trimmed clip with a nonzero start time is normalized the same way the sibling function extractFinalVideoFrameTimestamp already does for the same quirk — including matching its cache-key convention (keyed by file + start + duration, not file alone), so a future caller passing a different duration for the same file can't silently collide with a stale cached result.
• The one production call site (compileForRender's advisory video-check loop) now probes metadata first, then feeds its resolved duration/start into the keyframe check, rather than probing both independently in parallel.
• Breaking change: analyzeKeyframeIntervals is a public export of the published @hyperframes/engine package. Its signature changes from a single filePath argument to (filePath, metadata), with metadata required (not optional) — any external caller of this function needs to pass a {videoStreamDurationSeconds, videoStreamStartSeconds} object alongside the path. The parameter is intentionally required rather than optional-with-a-fallback: an optional param would recreate the dormant "healthy by default" path this fix exists to close.
• The duplicated stream-window normalization (start fallback, finite guard, cache-key format) that extractFinalVideoFrameTimestamp and analyzeKeyframeIntervals each computed independently is now a single shared helper, resolveStreamWindow, so the two can't drift out of sync with each other going forward.

Test plan

• New regression tests: a single keyframe is flagged problematic; a single keyframe with a nonzero stream start is still correctly flagged (regression for the absolute-vs-relative timestamp bug); a single keyframe exactly at the 2s threshold stays healthy (strict >, not >=); a short single-keyframe video stays healthy; an unknown (NaN) duration doesn't propagate NaN into the result; a zero-keyframe video is unaffected.
• packages/engine/src/utils/ffprobe.test.ts and packages/producer/src/services/mediaProbeConcurrency.test.ts both pass (121 total, 117 passing) — the 4 failures are pre-existing, confirmed via git stash to reproduce identically on the unmodified base commit (an unrelated Git-LFS fixture that isn't fetched in this environment), not touched by this change.
• typecheck clean for both packages/engine and packages/producer.

[terencecho]

APPROVE at a7a3bf0f — both parts land: single-keyframe now flagged, interval measured from stream's own start

Head verified: a7a3bf0f2018eb33c02e96c5326242215581c8d2. Commit co-authors: miga-heygen (bot carrier) + miguel-heygen (trust-listed).

Part A — single-keyframe correctly flagged problematic

Old path if (timestamps.length < 2) return isProblematic: false folded both zero-keyframe and single-keyframe into "healthy." New code splits: length===0 stays healthy (nothing to measure); length===1 computes remaining stream span and flags on > 2s. Regression test pins the single-GOP 6.5s case → isProblematic: true, plus boundary test at exactly 2s → false (strict >, not >=).

Part B — interval measured from stream's own start, not container/file start

New required metadata param carries videoStreamDurationSeconds + videoStreamStartSeconds. Single-GOP branch computes streamEnd = start + duration then rawInterval = streamEnd - timestamps[0], correctly normalizing the absolute-pts vs. relative-start quirk (matches extractFinalVideoFrameTimestamp's own comment). Inverse regression: start=5, duration=3, keyframe pts=5.0 → interval=3, isProblematic: true. Would silently clear to -2 → 0 under the un-normalized path, or use container duration under a wrong-duration-source path. Stream-vs-container exercised at 1s video / 10s audio → measured off video-stream duration = healthy.

Inverse pin — multi-keyframe problematic path still catches

timestamps.length >= 2 branch is behaviorally unchanged, only refactored to use the extracted PROBLEMATIC_KEYFRAME_INTERVAL_SECONDS = 2 constant with the same > comparison. Existing tests above the diff continue to cover the real sparse-keyframe seek-breakage case — the fix doesn't silently broaden pass-through.

Cache-key correctness

resolveStreamWindow returns a shared filePath\0start\0duration key used by both extractFinalVideoFrameTimestamp and analyzeKeyframeIntervals. Prevents silent stale-cache collision when the same path is analyzed with different stream windows.

No surface widening

Required (not optional) metadata param is the right call — optional-with-fallback would recreate the dormant "healthy by default" bug this closes. Public-API break called out in PR body. Call site in htmlCompiler.ts reworked from Promise.all([keyframe, metadata]) to sequential metadata → keyframe, correctly threading; still fire-and-forget for the advisory warning surface.

CI

All non-skipped checks SUCCESS: CI, CodeQL, regression shards 1-9, Windows render, Player perf, Preview regression, Preflight + File-size on 4 workflows. Skipped jobs are path-scoped (Skills / CLI / preview-parity — unrelated to engine ffprobe change). mergeStateStatus: BLOCKED on REVIEW_REQUIRED only.

— Review by tai (pr-review)

[jrusso1020]

Approving at 78f75e62.

• The only change since Terence's approval at a7a3bf0f is comment wording. I compared the PR's own diff at both heads, and every changed line is a comment.
• Single-keyframe branch is right: the span runs from the keyframe's absolute pts to start + duration, so a nonzero stream start still gets flagged. A NaN or negative span is clamped to 0, and the 2s threshold is strict >, same as the multi-keyframe branch.
• The cache key is now file + start + duration via resolveStreamWindow, the same key extractFinalVideoFrameTimestamp uses. That means a different duration for the same file can't reuse a stale result.
• compileForRender is the only caller in the repo (git grep), and it now passes extractMediaMetadata's video-stream duration and start. Stills come back with duration 0, so they stay healthy. Running the two probes one after the other is fine: this is a background warning, and failures are still swallowed, as they were under Promise.all.

Nit, not blocking and not new here: the multi-keyframe branch still never measures the trailing GOP, from the last keyframe to the end of the stream. A file with keyframes at 0s and 0.5s followed by 8s of video is reported healthy.

— Rames