fix: offset nested template video timing

[miguel-heygen]

What

Fix scene-local video timing inside later template-mounted composition hosts, and add a producer regression whose encoded output proves that the later host remains visible.

Why

Nested media inside a template-mounted composition is authored relative to that composition. The runtime and snapshot path instead evaluated that local media start against root time. In sequential hosts, this hid the video in a later host while the host itself was active, producing black output without a render error.

How

• Resolve media visibility from the nearest data-composition-src / data-composition-file host start before applying the media local start. Top-level media keeps its existing timing.
• Apply the same host offset when CLI snapshot capture selects and extracts active video frames.
• Add a minimal producer fixture with two sequential template hosts reading different offsets of one generated H.264 source. The second host must encode the green half rather than black.
• Add the fixture to the producer shard schedule.

Risk is limited to nested media timing inside template-mounted hosts; top-level media has no host offset and remains unchanged. Rollback is the two timing changes plus this regression fixture.

Test plan

• Restored the affected product files to origin/main, rebuilt the modular runtime, and ran the focused producer regression: exited 1 with 49 visual failures, with every later-host checkpoint at PSNR 8.54; compilation and audio passed.

• Restored the fixed head and regenerated the baseline from the updated fixture.

• Generated the producer baseline and ran the focused regression in the pinned Docker test image: compilation clean, artifact validated, 0 visual failures, audio passed.

• Ran the focused local producer regression: 0 visual failures and audio passed.

• Ran core runtime tests: 66 passed.

• Ran CLI snapshot tests: 29 passed.

• Ran producer shard-plan tests: 18 passed.

• Ran core, CLI, and producer typechecks.

• Ran changed-product-file oxlint, fixture/template formatting checks, and git diff --check.

• Unit tests added/updated

• Manual testing performed

• Documentation updated (not applicable)

[vanceingalls]

Review

Thesis check: The PR fixes scene-local video timing inside template-mounted composition hosts by resolving the nearest [data-composition-src] | [data-composition-file] ancestor's start and adding it to the video's authored start. This is applied at both the runtime visibility path (isTimedElementVisibleAt in packages/core/src/runtime/init.ts) and the CLI snapshot capture path (captureSnapshots in packages/cli/src/commands/snapshot.ts). The fix is correct for the reported single-level nesting failure and the producer regression fixture proves the second host renders its green half instead of black. Two related-but-adjacent gaps below.

Findings

• P2 — packages/cli/src/commands/snapshot.ts:419-421 — Snapshot path reads parseFloat(templateHost.dataset.start ?? "0") — the raw attribute — not a recursively-resolved root time. The runtime path (packages/core/src/runtime/init.ts:635-637) uses resolveStartForElement(mediaCompositionHost, 0), which composes host offsets recursively via resolveHostOffsetForElement (packages/core/src/runtime/startResolver.ts:118-127) and is correct at any nesting depth. For a video 2+ template hosts deep (e.g., host B inside host A, both with data-composition-src), the snapshot path only applies B's authored data-start and drops A's offset. HF already supports depth-3 nesting (packages/producer/tests/nested-subcomp-depth-3/), so this is a real gap — though still an improvement over pre-fix behavior (which used 0). Suggested follow-up: walk every [data-composition-src], [data-composition-file] ancestor in the page.evaluate block and sum their data-start values, or reuse the runtime resolver.

• P2 — packages/core/src/runtime/init.ts:739 and packages/core/src/runtime/timeline.ts:199-201 — Related duration-floor sites still call resolveMediaStartSeconds(node, 0) / read raw data-start, so resolveMediaWindowDurationSeconds / resolveMediaWindowEndSeconds can undercount a nested media's end (uses scene-local start + duration instead of root start + duration). Practical impact is bounded because root data-duration typically dominates the floor, but the same class of bug the PR just fixed lives here too. Same pattern in packages/producer/src/services/htmlCompiler.ts:2062, 2133 (browser media discovery + audio-volume automation windows). Out-of-scope for this PR, worth a follow-up.

• P3 — packages/producer/tests/nested-sequential-video-local-start/meta.json — minAudioCorrelation: 0 accepts any audio correlation. The fixture is intentionally muted-video-only, so this is fine, but it means the harness is silent on audio timing; audio-side nested-host miscompilation (finding 2, htmlCompiler) would not be caught here. Not a blocker.

Test verification: The three added tests cover the failure mode well:

• resolveSnapshotVideoClipStart unit test (snapshot.test.ts:158-175) exercises both branches (template-host present, top-level templateHostStart=null).
• Runtime visibility test (init.test.ts:918-966) mounts two sequential template hosts with scene-local data-start=0 videos and asserts at renderSeek(4) that only the second is visible — this is exactly the reported failure surface.
• Producer regression fixture with a real MP4 baseline covers the encoded output.
• Missing: no assertion for 2+ level template nesting (see finding 1) and no explicit assertion for the composition of data-media-start with the host offset (the later fixture uses data-media-start="1" but tests don't isolate that interaction).

Off-by-one direction check: authoredStart + templateHostStart matches the runtime own + fallback inside resolveMediaStartSeconds (init.ts:619-620). Not doubled — .closest on a video finds an ancestor, and the host itself isn't a media element, so isMedia gating prevents self-offset. Single-scene / non-nested case: no template-host ancestor → mediaCompositionStart = 0 → identical to pre-fix. All three adversarial paths check out.

Verdict: APPROVE — grade A-.

— Review by Via

[james-russo-rames-d-jusso]

Reviewed at b8a15cf.

Solid, targeted fix — visibility gate + snapshot both correctly add the template-host offset for scene-local media starts, and the producer fixture with encoded output is a great regression to guard against black-frame regressions on the later host. New runtime + CLI unit tests read cleanly.

The concerns I've left inline are architectural: the fix closes today's runtime↔snapshot asymmetry for numeric hosts + single-level nesting, but leaves two adjacent asymmetries — the snapshot's parseFloat-based resolution can't handle reference-expression host starts or multi-level nesting, and the visibility gate's [data-composition-src], [data-composition-file] selector is narrower than the sibling syncMediaForCurrentState's [data-composition-id]. Neither is a bug for the fixture's shape, but they're the same class of divergence that motivated this fix, and worth either documenting or closing.

What I didn't verify

• Whether inline sub-compositions (data-composition-id without -src/-file) exist by the time isTimedElementVisibleAt runs, or are always lowered to template hosts by the bundler/inliner.
• Whether real-world compositions use reference-expression host starts (data-start="scene-a + 2" on a template host).
• The current regression rollup check is red ("cancelled" != "success") — appears to be stale-shard cascade noise from a superseded run; the current regression-shards are still pending. Not blocking, but worth confirming they land green.

— Review by Rames D Jusso

[miguel-heygen]

Review follow-up at 72174e8fe: snapshot, visibility, media sync, and the runtime media-duration floor now share one canonical absolute-start resolver. Focused verification is green: core runtime 68/68, CLI snapshot 30/30, both package typechecks, changed-file lint/format, Fallow, and commit hooks. The producer/compiler timing surfaces noted as adjacent in review remain outside this runtime/snapshot patch.

[vanceingalls]

R2 @ 72174e8

Delta verify vs R1

• RESOLVED — P2 depth-3 snapshot recursion. packages/cli/src/commands/snapshot.ts:96-102, 411-467 — snapshot now reads window.__hfResolveMediaStartSeconds (installed by the runtime at packages/core/src/runtime/init.ts:620-661) and only falls back to authoredStart when the runtime resolver is absent/non-finite. Because the runtime resolver closes over resolveStartForElement (recursive via resolveHostOffsetForElement), any depth is composed at snapshot time — the R1 raw-attribute gap is closed by unifying the two paths through one canonical resolver instead of duplicating recursion in the browser eval block. The resolves media starts through arbitrarily nested composition hosts test (init.test.ts:929-968) asserts depth-3 (main→outer@2→inner@3→video@1) yields 6.

• PARTIAL / OUT-OF-SCOPE-DEFERRED — P2 related duration-floor sites. packages/core/src/runtime/init.ts:766 (resolveMediaWindowDurationSeconds) now routes through resolveAbsoluteMediaStartSeconds(node), so the runtime duration-floor site is fixed. packages/core/src/runtime/timeline.ts:198-207 (resolveMediaWindowEndSeconds) and packages/producer/src/services/htmlCompiler.ts:2055-2062 + 2135-2145 still read raw getAttribute("data-start") — Miguel's follow-up at 72174e8fe explicitly notes "producer/compiler timing surfaces noted as adjacent in review remain outside this runtime/snapshot patch," so deferred by author sign-off.

• OUT-OF-SCOPE-DEFERRED — P3 meta.json minAudioCorrelation:0. Unchanged at packages/producer/tests/nested-sequential-video-local-start/meta.json:7. Fixture is muted-video-only by design; R1 called this non-blocking.

Fresh adversarial pass

• P3 — packages/core/src/runtime/init.ts:637-661 — the overlapsHostWindow heuristic discriminates scene-local from legacy root-global PIP via authoredEnd > inheritedStart where authoredEnd is measured in scene-local time and inheritedStart in root time. Mixing frames means a scene-local video whose duration exceeds the host's root offset can be misclassified as legacy PIP and returned unshifted. Concrete: host@t=1 root, scene-local video data-start="0" data-duration="5" → authoredEnd=5, inheritedStart=1, 5>1 true, authoredStart<hostEnd true → returns 0 instead of 1. Not a regression (pre-fix scene-local was broken universally), and the shape rarely appears in shipped content (needs authoredDuration > inheritedStart), but the heuristic could bite when a nested scene sits early in root time with a long clip. Follow-up: gate the legacy branch on authoredStart >= inheritedStart (or persist an explicit "authored-frame" attribute) so start-time frames are compared consistently.

• P3 — packages/core/src/runtime/init.ts:625-628 — the inheritedStart <= 0 guard short-circuits the new branch when the host lives at root t=0, falling through to resolveStartForElement(element, inheritedStart). This matches expected behavior (host@0 needs no offset) and the test covers it (firstHost@0 at init.test.ts:889-891 passes), but the guard means the heuristic is inactive for the "leading host at zero" case — a note, not a bug.

• Clean — Concurrency: window.__hfResolveMediaStartSeconds is installed inside initSandboxRuntimeModular and cleaned up via runtimeCleanupCallbacks with identity check to avoid stale-resolver deletion (init.ts:663-668). Snapshot's page.evaluate serializes into the ready runtime; no race.

• Clean — Lifecycle: fixture registered in packages/producer/tests/shard-schedule.json (nested-sequential-video-local-start: 8). Producer's plan-regression-shards.mjs auto-discovers from disk and fails the build on unlisted fixtures — build passed, fixture packed into a shard.

• Clean — Depth boundaries: depth-1 (root, inheritedStart<=0 branch), depth-2 (later-host test), depth-3 (arbitrarily nested test asserting __hfResolveMediaStartSeconds=6), plus the PIP invariant test at init.test.ts:1585-1587 asserting ≈45.4 for the legacy-shape case. All four covered.

• Clean — Off-by-one composition: no double-add. element.closest("[data-composition-id]") stops at the nearest composition ancestor, and resolveStartForElement(compositionRoot, 0) walks up from there — the video itself isn't a composition host, so closest never returns the element itself.

• Clean — data-media-start composition: later.html uses data-media-start="1" (source-clip offset) independently of host offset; snapshot's mediaStart term is untouched and composes with the corrected start. Regression fixture proves the encoded output shows the green half of source.mp4 in the second host window.

• Clean — startAt/endAt clamping: resolveSnapshotVideoFrameTime receives the corrected start verbatim; clamp math unchanged.

• Clean — No new lint/typecheck warnings; Lint, Typecheck, Format, Fallow audit all green.

CI at head: required checks green — Test, Typecheck, Lint, Format, Producer unit+integration, CLI smoke, CLI npx shim (linux/macos/windows), Render on windows-latest, regression-shards shard-9 (contains sub-composition-video). Remaining shards 1-8 + Tests on windows-latest still in progress; nothing red. Miguel confirmed focused verification green in the follow-up comment.

Verdict: APPROVE — grade A.

Fix went beyond R1 scope: unified snapshot/visibility/media-sync/duration-floor through one canonical resolveAbsoluteMediaStartSeconds, closed depth-N via a single resolver on window, added three targeted runtime tests plus the producer regression fixture. Producer/compiler timing surfaces explicitly deferred by author. One residual heuristic edge case flagged for follow-up but not a blocker at this scope.

— Review by Via

[james-russo-rames-d-jusso]

Reviewed at 72174e8 (delta from R1 at b8a15cf7).

All three of my R1 concerns are fully addressed, and the fix goes further than I asked for.

• Runtime↔snapshot resolution asymmetry — closed structurally. window.__hfResolveMediaStartSeconds is now the canonical resolver (declared in packages/core/src/runtime/window.d.ts, owned by the runtime), called by snapshot.ts via page.evaluate. resolveSnapshotVideoClipStart collapses to runtimeResolvedStart ?? authoredStart — a compatibility fallback for older-runtime pages. This is the "expose runtime-computed absolute starts for snapshot to consume" option I sketched, cleaner than what I proposed (dedicated fn on window, no __clipTree shoehorn needed).

• Narrow-selector divergence with sibling syncMediaForCurrentState — closed by extracting a shared resolveMediaCompositionContext helper (broader [data-composition-id] selector) and routing the visibility gate + syncMediaForCurrentState + resolveMediaWindowDurationSeconds all through the new resolveAbsoluteMediaStartSeconds. Single canonical path — the two sibling code paths I flagged now can't drift.

• Test coverage — extended with "resolves media starts through arbitrarily nested composition hosts" (multi-level: main → outer@2 → inner@3 → video@1 resolves to 6) and "uses the canonical resolver for reference starts, auto-start media, and inline hosts" (all three of my suggested cases at once). The legacy PIP test at init.test.ts:1585 also gets expect(window.__hfResolveMediaStartSeconds?.(pipVideo)).toBeCloseTo(45.4).

Bonus that landed beyond my asks: the overlapsHostWindow heuristic in resolveAbsoluteMediaStartSeconds catches a legacy PIP-authoring convention (host@45.4 + video@45.4 authored as root-global rather than composition-local) that I hadn't flagged but which would've broken shipped content. The docstring locks the trade-off ("both timing conventions exist in shipped projects"). Ambiguity when authoredStart === inheritedStart is unavoidable without an authoring-mode marker; the picked side (preserve legacy) is the safer default.

CI is fully green now — my R1 note about the stale regression rollup being cascade noise has resolved, all shards and platform matrices report green.

LGTM from my side — leaving as a comment.

— Review by Rames D Jusso