Skip to content

fix(producer): render -c <scene> uses the scene's own duration, not the project's#2087

Draft
miguel-heygen wants to merge 1 commit into
mainfrom
fix/render-c-scene-duration
Draft

fix(producer): render -c <scene> uses the scene's own duration, not the project's#2087
miguel-heygen wants to merge 1 commit into
mainfrom
fix/render-c-scene-duration

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Problem

Rendering a single sub-composition standalone —

hyperframes render -c compositions/scene.html

— renders it for the whole project's duration instead of the scene's own. A 2s scene mounted in a 12s project renders as 12s. When the project's master root derives its length from sibling mounts (rather than a literal data-duration), the same path instead throws:

[FrameCapture] Composition has zero duration

This is a false-negative against the documented QA flow: the scene snapshots and validates correctly standalone, so the wrong length only shows up in the final render.

Root cause

To render a scene standalone, the producer reuses the real index.html shell and isolates the matching mount (extractStandaloneEntryFromIndexcreateStandaloneEntryRenderClone). It shallow-clones the master root and hangs only the scene's mount under it — but the shallow clone keeps the master's data-duration, so the extracted composition advertises the project length. The compiler reads that root duration verbatim.

Fix

Re-point the extracted wrapper's data-duration at the scene's own, read from the scene file's <template> root (the source of truth for that scene), with a fallback to the mount's data-duration. Full-project renders never take the extraction branch, so they're unaffected.

Verification

End-to-end via the pre-capture duration gate on a 3-scene project (master 12s; scene1 2s; scene2 10s):

render target before after
-c scene1 (own 2s) 12s ❌ 2s
-c scene2 (own 10s) 12s ❌ 10s
index.html (master) 12s ✅ 12s ✅

Adds unit coverage for both the scene-file and mount-fallback paths.

…he project's

When rendering a single sub-composition standalone (`hyperframes render -c
compositions/scene.html`), the producer extracts the scene's mount from
index.html and wraps it in a shallow clone of the master root. That clone
kept the master's `data-duration`, so the standalone composition advertised
the whole project's length instead of the scene's own: a 2s scene rendered
for the full 12s project, and a master that derives its length from sibling
mounts (now removed) produced "Composition has zero duration".

Re-point the extracted wrapper's `data-duration` at the scene's own, read
from the scene file's `<template>` root (the source of truth for that scene),
with a fallback to the mount's `data-duration`. Full-project renders are
unaffected — they never take the extraction branch.

Verified end-to-end via the pre-capture duration gate: a 2s scene now resolves
to 2s and a 10s scene to 10s (both were 12s), while the full index render stays
at 12s. Adds unit coverage for both the scene-file and mount-fallback paths.

@terencecho terencecho 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.

LGTM with one test-coverage nit.

Cross-checks

  • Scope limited to target-scene branch. extractStandaloneEntryFromIndex is only invoked from the target-scene branch of executeRenderJob (renderOrchestrator.ts:1324). Full-project render path is untouched — matches the PR's "unaffected" claim.
  • Wrapper-only mutation. The mount (host) retains its own data-duration unchanged in the cloned tree; only the wrapper root's attribute is swapped. Downstream trim/seek/frame-capture consumers that read the mount's window are unaffected.
  • Fallback chain handles the linkedom inert-template gap. readSceneRootDuration re-parses template.innerHTML (matching the htmlBundler pattern) and degrades: scene <template> root → document root → mount data-durationremoveAttribute. On null the wrapper defers to child-derivation, which matches the pre-fix zero-duration case.
  • Master-value negative assertion is included. Both new tests assert not.toContain('data-duration="12"'), which catches the "scene N got N" false-positive shape for the master's specific duration.

Nit (non-blocking, test-coverage gap)

renderOrchestrator.test.ts:120-130 — the "re-points the wrapper duration at the scene's own" test uses data-duration="2" for BOTH the scene template root AND the mount, while master is 12. If readSceneRootDuration returned null and fell through to the mount-fallback path, the assertion would still pass. Distinct values (e.g. scene template root="3", mount="2") would prove scene-file precedence over the mount-fallback. Scene-vs-master direction is still covered — this is a coverage gap, not a correctness gap.

Diff-scope: +79/-2, 2 files. No prior reviewers.

— Review by tai (pr-review)

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: 🟢 LGTM at 5facd7c

Concurring with @terencecho. The fix targets the actual leak site: master root's data-duration was propagating through the shallow-clone at renderOrchestrator.ts:958; the new readSceneRootDuration reads the scene template's root, and createStandaloneEntryRenderClone stamps it onto the wrapper (line 968). The signature change on extractStandaloneEntryFromIndex is backward-compatible (entryHtml? optional). Full-project renders take the non-extraction path at executeRenderJob, untouched — matches the PR's "unaffected" claim.

One additional observation on top of Terence's scope check: readSceneRootDuration's fallback query ([data-composition-id] outside a template) could pick up a nested sub-composition if a scene file is authored without a wrapping template. The startsWith("<template") gate at renderOrchestrator.ts:1315 prevents that in practice — the standalone-scene path always has a template — but a defensive check that the matched element is at document root (not nested) would be belt-and-suspenders. Not blocking.

Both added tests exercise the scene-file and mount-fallback paths against the reported repro shape. Duration source chain (scene template root → mount's data-duration → drop attr entirely) is honest.

R1 by Via

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.

3 participants