feat(producer): renderStretch to re-time short compositions across longer scenes

[xuanruli]

Linear: VA-1859

Problem

For a fit_to_scene B-roll where the composition's intrinsic timeline (e.g. data-duration=1.0s → 30 frames) is shorter than the scene it fills (e.g. 4.8s narration), the producer renders only the intrinsic 30 frames and the downstream compositor frame-holds/PTS-stretches that fixed clip to the scene length. Spreading 30 unique frames over 4.8s starves motion to ~6 effective fps → a visibly choppy result. Root cause: the producer welds one composition.duration to both the frame count and the 1:1 seek mapping, with no notion of a target output length.

Fix

Add optional renderStretch: number (default 1.0 = no-op), renderStretch = intrinsic / target:

• Frame count comes from the target: outputDuration = intrinsic / renderStretch, totalFrames = outputDuration × fps (probeStage.ts). composition.duration stays intrinsic (drives video/audio windows).
• Per-frame seek is scaled: time = (frameIndex / fps) × renderStretch, so the N output frames map across [0, intrinsic] — a fresh frame per output frame.

All seek sites go through a single shared outputFrameToTimelineSeconds(frameIndex, fps, renderStretch) helper (core.types.ts), consumed by every capture path so none can silently diverge:

• parallel (parallelCoordinator.ts), sdr_streaming (captureStreamingStage.ts ×3), sdr_disk (captureStage.ts), HDR loops.
• DrawElement + static self-verify (frameCapture.ts) — ground-truth seek uses the same mapping, so PSNR compares like-for-like (no spurious verification failure on stretched comps).
• Distributed path: renderStretch threaded through DistributedRenderConfig → chunk workers, and folded into the plan hash only when != 1 so a pre-stretch cached plan is never reused.

With renderStretch = 1 (or omitted → ?? 1): every seek is ×1.0 (IEEE-754 identity), frame counts unchanged, and the plan hash is byte-identical — a provable no-op. player.ts absolute-seek is untouched.

Verify

• typecheck (core + engine + producer): pass. lint/format/fallow/commitlint: pass. planHash + renderRequest unit suites: pass.
• Adversarial self-review found + fixed three capture-path gaps (streaming, self-verify, distributed) before this revision.
• Not yet runtime-verified on a real render — needs a fit_to_scene render at renderStretch < 1 confirming N distinct frames over the target length (draft until then).

Paired with experiment-framework#42766, which computes and forwards renderStretch = hf intrinsic / scene duration.

🤖 Generated with Claude Code

[xuanruli]

• feat(producer): renderStretch to re-time short compositions across longer scenes #2676 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[james-russo-rames-d-jusso]

Reviewed at 130737c.

Careful re-time. The threading of renderStretch through 18 files is symmetric — every capture path (parallel coordinator × 2, streaming × 3, HDR hybrid, HDR sequential, disk, distributed chunk worker, drawElement self-verify) now routes through the same outputFrameToTimelineSeconds helper, so drift between paths is structurally hard to introduce. The renderStretch = 1 no-op story (×1.0 IEEE identity + conditional-append plan-hash + ?? 1 fallback at every consume site) is defensible for byte-identity on the untouched path. Nice work on the adversarial self-review that surfaced the streaming / self-verify / distributed gaps before submission.

Acknowledging your own draft-flag — «Not yet runtime-verified on a real render» — so this is a structural pass while you finish the empirical verification, not a stamp-track review. Two concerns, two nits, one bounding question inline.

Non-findings (verified clean):

• padOrTrimAudioToVideoFrameCount pads intrinsic audio with silence up to the encoded video's frame count. For the VA-1859 use case (fit_to_scene b-roll where narration lives on the scene, not the comp) this is the desired behavior. Just worth calling out somewhere so future maintainers reaching for music/audio-heavy comps at renderStretch != 1 know the audio won't time-stretch — see concern below.
• Both buildRenderPerfSummary sites correctly source compositionDurationSeconds from composition.duration (intrinsic) for telemetry; the earlier CaptureOptions site correctly sources from job.duration (now output). The split is intentional; the naming is what's confusing (see concern).
• job.duration = probeResult.duration cascades OUTPUT duration into every durationSeconds: job.duration call site — verified they're all render-scale decisions (streaming eligibility, retry plans, observability) where OUTPUT is the right value.
• Byte-identity for renderStretch = 1: seek arithmetic is IEEE identity, planHash suffix is conditional-append. plan.dimensions.renderStretch will differ (undefined vs 1) but the derived hash is stable.

Not stamping — the runtime-verified render + a couple of the items below would put it in landable shape.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 Concern — field name and semantic have drifted. Post-PR, CaptureOptions.compositionDurationSeconds no longer means «the composition's intrinsic duration» — it means «the OUTPUT (drained) duration = intrinsic / renderStretch», per frameCapture.ts:3603 inline comment. The type declaration here doesn't document that. A future author looking at compositionDurationSeconds and assuming intrinsic will do wrong math (e.g. computing intrinsic frame counts for verification vs the real intrinsic duration from composition.duration). Two ways forward: (a) JSDoc on this field body saying «post-stretch output duration; intrinsic is only in composition.duration», or (b) rename to outputDurationSeconds and audit every setter. Option (a) is the smaller diff. This is subtle enough that it will silently drift again the next time someone touches this surface. — Rames D Jusso

[james-russo-rames-d-jusso]

🟠 Concern — audio behavior under renderStretch < 1 deserves an explicit call-out in the PR body / docs. I traced padOrTrimAudioToVideoFrameCount (audioPadTrim.ts:268) to confirm what happens: intrinsic composition audio (say 1s) gets silence-padded to match the encoded video's frame count (say 4.8s). For the VA-1859 use case (b-roll fit_to_scene, narration on the scene, not the comp) this is exactly right. But for a music-heavy or dialogue-carrying comp used at renderStretch != 1, the composer would (probably wrongly) expect audio to time-stretch alongside video and instead get 3.8s of silence appended. Worth adding a line in the PR body / renderStretch JSDoc: «intended for silent / audio-agnostic comps; comp audio is silence-padded, not time-stretched, so audio-heavy comps aren't supported». Non-blocking for VA-1859 but load-bearing for the field's reusability. — Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Nit — zero unit tests for outputFrameToTimelineSeconds, the heart of the change. The PR body notes planHash + renderRequest unit suites: pass — those exercise the plan-hash byte-identity claim and request-validation guards, but not the seek arithmetic itself. This helper is called from 8+ capture sites and its correctness under renderStretch != 1 is what the runtime verification will empirically confirm. A handful of unit tests would lock the math in place BEFORE the runtime pass finds a subtle IEEE-754 issue at rational fps ({num: 30000, den: 1001} × renderStretch = 0.2083, etc.): boundary frame (i=0), last output frame lands at ≈ intrinsic, no-op case (renderStretch=1 returns exactly (i * den) / num). Cheap insurance, fits the PR's adversarial-self-review posture. — Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Nit — plan.json byte-identity is not preserved even though planHash is. The conditional-append here (renderStretch !== undefined && renderStretch !== 1) keeps the hash byte-identical. But plan.dimensions.renderStretch = config.renderStretch in plan.ts writes the value verbatim, so a plan built with renderStretch = 1 (explicit) produces different JSON than one built with renderStretch omitted. Both hash the same and both run the same at capture time (?? 1), so runtime is safe — but if anyone downstream does structural plan-JSON diffs (cache validity checks that go byte-level rather than hash-level, e.g. debugging cross-run plan drift), they'll see spurious differences on stretched-but-no-op plans. Suggest normalizing on write: only write renderStretch into plan.dimensions when !== 1. Aligns write-side with hash-side. — Rames D Jusso

[james-russo-rames-d-jusso]

❓ Question — any upper bound needed on renderStretch beyond > 0? Validation accepts any finite positive number. A 10s intrinsic with renderStretch = 0.0001 → ~10⁵ s ≈ 27.8 hr output → correctly rejected by validateRenderDuration (24hr ceiling) that landed in #2671. Good, transitive bound. But renderStretch = 0.5 on a 60,000-second (huge) intrinsic works out to a 120,000s output → also rejected. Bound only fires via the intrinsic × stretch product, which is arithmetic the author had to derive. Would an explicit renderStretch ≥ 0.01 (or similar) at request-validation-time surface a clearer error («renderStretch too aggressive» vs «output duration too long» — different diagnostic path)? Not a blocker; feels like the transitive bound holds. Curious whether you considered an explicit ceiling. — Rames D Jusso

[xuanruli]

Thanks — all three actionable items addressed in dee1c3c37:

(a) compositionDurationSeconds naming/meaning — added a JSDoc line on the field (packages/engine/src/types.ts): "When renderStretch != 1 this is the OUTPUT (stretched) duration, not intrinsic." It's populated from job.duration = probeResult.duration (which probe now sets to intrinsic/renderStretch), so the drawElement self-verify frame count is already in output space.

(b) Audio silence-pad vs time-stretch — documented on the public renderStretch field (renderRequest.ts): "Audio is silence-padded to output length, not time-stretched." Correct + intended for VA-1859 (fit_to_scene b-roll is silent — the narration is a separate track), and EF only ever emits renderStretch for a fit_to_scene b-roll whose intrinsic < scene, never for an audio-carrying comp. Time-stretching embedded audio for the general case is a deliberate follow-up, not in scope here.

(c) Seek-helper unit tests — added packages/core/src/outputFrameToTimelineSeconds.test.ts (4 cases): no-op when omitted, renderStretch=1 byte-identical to raw frame time, 1s→4.8s stretch mapping (frame 0 → 0, last of 144 frames lands just under intrinsic 1.0s, never past), and exact-rational NTSC fps.

Bounding question (renderStretch range): validators reject ≤ 0 on both the HTTP (assertOptionalPositiveNumber) and distributed (renderConfigValidation) paths. > 1 (comp longer than scene → compress) is arithmetically valid — the last output frame still maps < intrinsic, no clamp needed — but EF never sends it (guarded to intrinsic < scene, so always ≤ 1). If you meant a different bound, point me at it.

Still respecting the draft flag: the runtime-verified render (a real fit_to_scene render at renderStretch < 1 confirming N distinct frames over the target length) is the remaining gate before this leaves draft — devbox can't produce render output, so it needs a real producer run.

[james-russo-rames-d-jusso]

R2 against dee1c3c37e501e7063641416da0e1825f70dfb04 — delta vs R1 head is 3 files, +32/-1 (test file + two doc comments). Structural verdict on R1 items below.

R1 findings — status

#	R1 item	Status	Note
1	🟡 Semantic drift on CaptureOptions.compositionDurationSeconds	🟡 partial	packages/engine/src/types.ts:117 adds "When renderStretch != 1 this is the OUTPUT (stretched) duration, not intrinsic." Pragmatic — a rename ripples across ~20 call sites. Doc-only is defensible; readers of the type get the caveat before they trip on it.
2	🟡 Audio pipeline silence-pads intrinsic audio to output frame count	🟢 closed	packages/producer/src/renderRequest.ts:52 on renderStretch now reads "Audio is silence-padded to output length, not time-stretched." Right shape — surprises the caller before they opt in. Music/dialogue callers can decide.
3	🟡 Zero unit tests for outputFrameToTimelineSeconds	🟢 closed	New packages/core/src/outputFrameToTimelineSeconds.test.ts covers: default-omitted, renderStretch=1 byte-identity, compressed source (rs = 1/4.8), NTSC rational fps (30000/1001). Math checks out — the "last of 144 output frames lands just under intrinsic 1.0s" assertion is exactly the invariant that matters.
4	🟡 plan.json byte-identity at renderStretch=1	🟠 open	Not addressed. planHash.ts:132-134 conditionally omits the x<rs> suffix so the hash stays stable, but I haven't confirmed the emitted plan.json itself is byte-identical to pre-PR (i.e. no renderStretch: 1 field appearing in the serialized output). Low-severity — worth a follow-up grep on the plan serializer if you want the golden-plan tests to stay unchanged.
5	❓ renderStretch upper/lower bound	❓ open	Not addressed in code. The added tests exercise only rs = 1 and rs = 1/4.8 ≈ 0.208. Is renderStretch > 1 (source compression, output shorter than intrinsic) a supported mode, and if so has the caption/audio silence-pad path been walked with it? If the intended domain is (0, 1], worth a runtime clamp or JSDoc range.

What I didn't verify

• Plan.json emitted-serialization byte-identity (see #4 above) — the hash stability is a proxy, not proof.
• Runtime behavior in a real stretched render — Xuanru's own verification stands.

LGTM from my side on the R2 delta; three items above are non-blocking. Approval routing continues to sit with James.

— Review by Rames D Jusso

[xuanruli]

Thanks for the LGTM. Closed both remaining items in 9391ad964:

plan.json byte-identity at rs=1 — verified and now test-locked. freezePlan writes plan.json (incl. dimensions) via JSON.stringify(…, null, 2) (freezePlan.ts), and dimensions.renderStretch is a straight config.renderStretch passthrough — so at rs=1/omitted it's undefined, which JSON.stringify drops → byte-identical plan.json. The hash side has the same property (suffix appended only when != 1). Added computePlanHash — renderStretch byte-identity to planHash.test.ts: renderStretch = 1 and undefined both hash identically to a pre-renderStretch plan, and renderStretch = 2 gets a distinct hash. So a future change that defaulted it to 1 in dimensions (which would break byte-identity) now fails a test.

renderStretch > 1 support — decision: not a first-class mode. It's arithmetically valid (compress — last output frame still maps < intrinsic, no clamp) and the validators only reject ≤ 0, so it won't crash, but it's untested and EF never emits it (guarded to intrinsic < scene → always ≤ 1). Treating it as "defined behavior, unsupported/untested path" rather than adding a hard reject, so a future compress use-case isn't blocked by a validator. Happy to tighten to reject > 1 instead if you'd prefer it be impossible rather than merely unused — your call.

Only gate left is the runtime-verified render (draft flag stays until a real producer run at renderStretch < 1 confirms N distinct frames over the target length).

[james-russo-rames-d-jusso]

R3 against 9391ad964a0c31fdad8c7e76b44f37da789e3c2d — delta vs R2 is +20 lines to planHash.test.ts.

Two new assertions in computePlanHash — renderStretch byte-identity:

1. renderStretch: 1 and renderStretch: undefined and omitted-key all hash identically to a pre-renderStretch plan — locks in the conditional-append at planHash.ts:132-134 against future regressions. Closes R2 item #4 from the hash-stability angle (existing goldens won't shift when they render at rs=1).
2. renderStretch: 2 gets a distinct hash from the baseline — implicitly confirms renderStretch > 1 is a supported input mode, which was R2 item #5.

Nothing outstanding from my side. LGTM. Stamp routing continues to sit with James.

— Review by Rames D Jusso

[vanceingalls]

Additive R1 pass, layered on top of Rames's R1/R2/R3.

The core plumbing that Rames stress-tested holds up from every additive angle I probed. renderStretch flows through the full runtime chain — RenderRequestOptions → parseRenderOptions → DistributedRenderConfig → SerializableDistributedRenderConfig (which is what renderToLambda and renderToCloudRun accept, so cloud SDKs pick it up for free) → plan.dimensions → plan.json → per-chunk seek time in renderChunk. Numeric validation is genuinely tight: assertOptionalPositiveNumber at packages/producer/src/renderRequest.ts:110 and the equivalent block at renderConfigValidation.ts:187-198 both reject 0, negatives, NaN, and Infinity. Operator-precedence in the intrinsic-vs-output fallback at frameCapture.ts:3606-3611 (compositionDurationSeconds ?? (page.__hf?.duration ?? 0) / renderStretch) parses correctly — / binds tighter than ??, so the fallback branch (intrinsic) gets stretch-adjusted while the direct branch (already output) is used verbatim. Rames's R3 test additions on planHash close the byte-identity concern cleanly.

Findings — gap-filling axes Rames didn't emphasize:

P2 — CLI arg wiring absent on every render surface. No file in packages/cli/src/commands/ touches renderStretch. I grepped render.ts, render/plan.ts, render/execute.ts, render/present.ts, batchRender.ts, cloud.ts, cloudrun.ts, lambda.ts, and preview.ts at head — nothing. The programmatic SDK path is fully wired (compositors calling renderToLambda(opts) with SerializableDistributedRenderConfig.renderStretch set will work end-to-end, since renderConfigValidation.ts:33 Omits only logger|abortSignal|producerConfig and the field passes through), but hyperframes render --render-stretch <n> doesn't exist on any of the 5 render surfaces. Nor is the field readable from hyperframes.json — ProjectConfig at projectConfig.ts:35 doesn't include it. If this is deliberately compositor-only for VA-1859 b-roll, that's fine but the PR description should say so; if it's meant to reach CLI users eventually, it's a follow-up. Not raising as P1 because the PR is explicitly feat(producer).

P2 (contingent on above) — Telemetry blind spot when this feature is exercised. packages/cli/src/telemetry/events.ts:193-283 emits rich composition metadata on render_complete (compositionDurationMs, compositionWidth, compositionHeight, totalFrames) plus a full RenderObservabilityTelemetryPayload at renderObservability.ts:11-72, but render_stretch is nowhere in either. Once CLI/cloud users engage this knob, a maintainer investigating "why is this render 4.8× a weird duration" via PostHog can't see whether re-time was engaged. Fires only when the CLI wiring above lands, so paired.

Nit — Two seek call sites open-code the mapping instead of using the shared helper. Rames praised in R1 that "every capture path routes through the shared outputFrameToTimelineSeconds", and that's true of the 5 stage files touched. But frameCapture.ts:2664 (verifyStaticFramesSafe) and :3660 (captureDeVerificationFrames) still hand-roll quantizeTimeToFrame((frameIdx / fps) * renderStretch, fps). Arithmetically equivalent today, but if the shared helper ever gains rounding/quantization logic, these two paths won't follow. Follow-up cleanup, not a blocker.

Nit — CaptureOptions.renderStretch has no JSDoc. Sibling compositionDurationSeconds picked up a JSDoc note in this PR (types.ts:117); the new field at types.ts:126 is bare renderStretch?: number;. RenderRequestOptions and PlanDimensions both got proper JSDoc; the engine-layer type didn't.

Color on Rames's R2/R3 open item — plan.json byte-identity has a subtle edge. Rames's R3 test locks renderStretch: 1 / undefined / omitted-key to the same hash. Worth noting: they're byte-identical in plan.json only when the field is undefined (JSON.stringify drops undefined keys), but an explicit renderStretch: 1 at plan.ts:1043 embeds "renderStretch": 1 in plan.json while the plan-hash still matches (because the hash-suffix code at planHash.ts:132 excludes === 1). Callers that check plan.json bytes vs the hash for cache-key derivation would see a mismatch that legitimately shouldn't matter — worth a comment either at the plan write or the hash function.

Verdict: APPROVE with follow-ups. Nothing new I found rises to a blocker — the producer-scoped runtime and cache/hash invariants are solid, and the two P2s are scope-fence questions (is this a CLI feature yet? if yes, add telemetry) rather than defects in what shipped. The three nits are cleanup, not correctness.

Review by Via