fix: bound invalid render durations

[jrusso1020]

Summary

• fail closed before static-dedup allocates frame-index Sets for malformed, non-finite, unsafe, or oversized durations
• validate browser-probed duration in both regular and distributed render paths with a shared typed validator
• correct the infinite-repeat linter repair hint and add an observable warning for unclamped Math.floor(...)-1
• replace conflicting repeat: -1 / Math.ceil guidance in Claude Design and Remotion conversion references

Root cause

GSAP repeat: -1 reports a very large sentinel duration. Static-frame analysis converted that duration to hundreds of billions of frame indexes and attempted to fill a Set before producer duration validation ran, producing Set maximum size exceeded. The regular renderer also had no equivalent of the distributed duration check.

The previous repair hint had a secondary hazard: Math.floor(duration / cycleDuration) - 1 becomes -1 when the scene is shorter than one cycle, recreating GSAP infinite repeat. The canonical form is now Math.max(0, Math.floor(duration / cycleDuration) - 1).

Validation

• engine focused suite: 5 passed
• lint GSAP suite: 137 passed
• producer validator suite: 27 passed
• engine, lint, and producer typechecks passed
• oxlint and oxfmt passed on changed files
• skill manifest and skill lint passed

Rollout dependency

This needs a patch release followed by coordinated pin bumps in experiment-framework before the inactive Claude MCP delivery-gate flags are enabled in DEV.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
hyperframes	🟢 Ready	View Preview	Jul 21, 2026, 3:07 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
hyperframes	🟡 Building	–	Jul 21, 2026, 3:06 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[miga-heygen]

SSOT Review: fix: bound invalid render durations

PR #2671 — 13 files, +193/-30

---

SSOT inventory

Concept	Source of truth	Consumers	Verdict
Duration validation logic	validateRenderDuration	validateDistributedDuration (wrapper), renderOrchestrator	Single owner
Max duration constant	MAX_RENDER_DURATION_SECONDS (alias of MAX_DISTRIBUTED_DURATION_SECONDS)	validateRenderDuration, tests	Single owner — alias, not duplicate
Error code	RENDER_DURATION_OUT_OF_RANGE (alias of DISTRIBUTED_DURATION_OUT_OF_RANGE)	validateRenderDuration, tests	Single owner — alias, not duplicate
Static dedup frame limit	MAX_STATIC_DEDUP_ANALYSIS_FRAMES	isStaticDedupFrameAnalysisSafe, computeStaticFrameSet, tests	Single owner
Frame analysis safety check	isStaticDedupFrameAnalysisSafe	computeStaticFrameSet	Single owner
Clamped repeat formula	Math.max(0, Math.floor(d / c) - 1)	Lint messages (×3), Claude Design guide, Send-to guide, Remotion skill refs (×3)	Consistent across all 9 sites

What I checked

1. Defense in depth layering — Two independent guards, different layers:

   • isStaticDedupFrameAnalysisSafe (1M frame cap) → graceful degradation, returns eligible: false. Prevents the immediate crash (Set maximum size exceeded).
   • validateRenderDuration (24h × fps ≈ 2.6M at 30fps) → hard error via PlanValidationError. Prevents operational disaster (billions of frames queued for capture).

   For sentinel-sized durations, the static dedup guard fires first (lower threshold) and the render skips dedup optimization. Then validateRenderDuration at renderOrchestrator.ts:1972 catches it and throws. The two layers are not redundant — they own different failure modes at different points in the pipeline.

2. isStaticDedupFrameAnalysisSafe guard completeness — Number.isFinite && Number.isSafeInteger && > 0 && <= MAX. Rejects NaN, ±Infinity, values beyond 2^53, zero, negative, and oversized. The test covers: exact boundary (accepts), boundary+1 (rejects), +Infinity, MAX_SAFE_INTEGER+1, and 0. Correct.

3. Sentinel duration test — Mocks a page returning duration: 10_000_000_000. Asserts eligible: false, reason contains "frame analysis limit", empty Set, and only one page.evaluate call (no clip-boundary scan happened). This proves the guard fires before any frame-index allocation. Correct.

4. Alias alignment test — expect(MAX_RENDER_DURATION_SECONDS).toBe(MAX_DISTRIBUTED_DURATION_SECONDS) and expect(RENDER_DURATION_OUT_OF_RANGE).toBe(DISTRIBUTED_DURATION_OUT_OF_RANGE). Explicit SSOT alignment assertion — if someone changes one without the other, the test breaks. The test also calls validateRenderDuration at the boundary value to confirm it doesn't throw. Good.

5. validateDistributedDuration backward compatibility — Now a thin wrapper calling validateRenderDuration. Existing callers in the distributed path keep working; the error code stays stable for workflow retry policies (matches the docstring motivation). Correct.

6. Regular render path now validates — renderOrchestrator.ts:1972 adds validateRenderDuration right after the browser probe resolves probeResult.duration and probeResult.totalFrames. Previously only the distributed path had duration validation. The call site passes fpsToNumber(job.config.fps) for fps, matching the same pattern used throughout the orchestrator. Correct.

7. Lint rule gsap_repeat_floor_unclamped — Regex: repeat\s*:\s*Math\.floor\s*\([^)]+\)\s*-\s*1. This matches repeat: Math.floor(...) - 1 but NOT repeat: Math.max(0, Math.floor(...) - 1) because the token after repeat: would be Math.max, not Math.floor. Tests confirm both paths. Severity is warning, not error — appropriate since unclamped floor-minus-one is safe when duration >= cycleDuration. Correct.

8. Formula consistency across all docs and skills — Checked all 9 sites where the repeat formula appears:

   • claude-design-hyperframes.md — updated ✓
   • claude-design-send-to-hyperframes.md — updated ✓
   • gsap.ts repeat:-1 error message — updated ✓
   • gsap.ts repeat:-1 fixHint — updated ✓
   • gsap.ts ceil→floor fixHint — updated ✓
   • gsap.ts new unclamped warning fixHint — uses clamped formula ✓
   • remotion-to-hyperframes/references/sequencing.md — updated with concrete example ✓
   • remotion-to-hyperframes/references/limitations.md — updated, explicitly says "never use repeat: -1" ✓
   • remotion-to-hyperframes/references/api-map.md — updated ✓

9. Sequencing.md example — The code sample now shows const repeat = Math.max(0, Math.floor(availableDuration / cycleDuration) - 1) with concrete variables. The old code had repeat: -1 directly. The new version is a working bounded-repeat pattern. Correct.

Blocking SSOT issues

None found.

Non-blocking observation

stripComments: false in the new gsap_repeat_floor_unclamped rule means the regex scans raw script text including comments. A commented-out // repeat: Math.floor(d/c) - 1 would trigger the warning. Since this is a warning (not an error) and commented-out unclamped floor patterns are still worth flagging (they get copy-pasted into real code), this is acceptable — but worth noting if false positive reports come in.

Verdict

Approve. The fix addresses the root cause at two layers (static dedup graceful degradation + render pipeline hard gate), updates the secondary hazard (Math.floor - 1 can evaluate to -1) across all lint/doc/skill surfaces consistently, and the backward-compatible wrapper keeps the distributed API stable. Test coverage locks down the boundary conditions, the alignment assertions, and the sentinel-duration path.

--- Miga

[james-russo-rames-d-jusso]

Reviewed at 344d9c0.

🔴 CI is red on this PR — «Producer: integration tests» failed at job 88532165363, and I don't think Miga's SSOT read caught it. One concrete blocker + a couple of nits and a question inline. The defense-in-depth structure (fail-closed static-dedup cap + validateRenderDuration parity on the regular render path + gsap_repeat_floor_unclamped author-time warning + doc consistency across all 9 sites) is the right shape — this is a real gap-closer once the message assertion is reconciled.

Root cause of the CI failure: the error message in planValidation.ts:167 was generalized from "[planValidation] Distributed render duration is out of range: …" to "[planValidation] Render duration is out of range: …" (dropped "Distributed"), but the pre-existing integration test at packages/producer/src/services/distributed/planSizeCap.test.ts:196 still asserts toMatch(/distributed/i) on that same message. That assertion now fires — see inline anchor. The PR body's "producer validator suite: 27 passed" was accurate for the unit tests in planValidation.test.ts, but the integration suite invoked by bun run producer:test:integration wasn't run locally.

Two ways forward: (a) update line 196 to /render duration/i (fits the generalization intent), or (b) restore "Distributed" in the message body (loses the semantic clean-up). Option (a) is the smaller diff and matches the PR's direction.

Rest of the diff scans clean: the isStaticDedupFrameAnalysisSafe predicate covers the four fail-closed cases (non-finite, non-safe-integer, non-positive, oversize), the guard runs before frame-Set allocation, and the fail-closed branch preserves eligibility signals for downstream so normal capture proceeds. renderOrchestrator.ts:1972-1976 closes the parity gap between distributed and regular render paths. Backward-compat aliases (MAX_RENDER_DURATION_SECONDS, RENDER_DURATION_OUT_OF_RANGE, wrapper validateDistributedDuration) keep external retry policies keyed on the old code stable — the alignment test locks that down.

Not stamping — leaving after the test fix + green CI.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🔴 Blocker — this message change breaks a pre-existing integration test. The generalization from "Distributed render duration is out of range: …" to "Render duration is out of range: …" drops the word Distributed that packages/producer/src/services/distributed/planSizeCap.test.ts:196 asserts on: expect(String((caught as Error).message)).toMatch(/distributed/i);. That assertion runs under bun run producer:test:integration and is what turned the «Producer: integration tests» CI job red. The unit-test suite (planValidation.test.ts) that the PR body notes as passing doesn't exercise this assertion. Smallest fix: update planSizeCap.test.ts:196 to /render duration/i (matches the new message and stays specific). If keeping the legacy wording is preferred, revert the message body change instead — but the current state is the failing intermediate. — Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Nit — regex only catches inline repeat: Math.floor(...) - 1, not the variable-assigned form. \brepeat\s*:\s*Math\.floor requires Math.floor to sit directly after repeat:. So the equivalent pattern via a local variable — const count = Math.floor(duration / cycle) - 1; gsap.timeline({ repeat: count }); — is invisible to the rule. Not blocking (the direct-write form is by far the most common), but if HF authors reach for that idiom the warning never fires. Worth either (a) a follow-up rule that tracks the Math.floor(...) - 1 binding and its use in repeat:, or (b) a rule-doc note saying the check is inline-form-only so authors don't assume the linter covers all paths. — Rames D Jusso

[james-russo-rames-d-jusso]

❓ Question — warning severity for a pattern that can reach the deterministic-capture-breaking -1 sentinel? repeat: -1 fires as error on the same rule module, and this pattern can evaluate to the same value at runtime when duration < cycleDuration. Warning is defensible if the intent is «this is a smell, not a guaranteed bug» (only bites when the composition is short enough), but if downstream CI thresholds swallow warnings, an author landing this pattern in a short composition gets no signal. Was warning an explicit choice over error, or defaulting because the pattern only sometimes evaluates to -1? — Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Nit — snippet uses compositionDuration without defining it. The new form reads const availableDuration = compositionDuration - 3; but the reader has no context for where compositionDuration comes from (HF runtime global? computed elsewhere? Remotion carry-over?). The prior repeat: -1 snippet was self-contained; the replacement asks the reader to fill in a piece they may not have. Suggest either inlining a concrete value (e.g. const compositionDuration = 30;) or a one-line comment naming the source (// resolved from data-duration on the root composition, or similar). Nit-level — doesn't block understanding but hurts copy-paste-ability. — Rames D Jusso

[jrusso1020]

Additive to Miga + RDJ (not re-litigating — RDJ's integration-test blocker is resolved by the test: align distributed duration assertion commit; Producer: integration tests is green now). The core fix is sound: the isStaticDedupFrameAnalysisSafe cap sits before the Set-fill loops in computeStaticFrameSet and, living inside the engine function, protects every caller (CLI/Studio/cloud/probe), and validateRenderDuration fails fast at the probe stage in the regular path. A few things a structural pass surfaces:

1. (completeness, non-blocking) There's an unguarded structural twin in the same file. computeTimelineAtRiskFrames (frameCapture.ts ~L2825) builds a frame-index Set with the identical unbounded shape — const hi = Math.ceil(end*fps)+1; for (let f = Math.max(0,lo); f <= hi; f++) frames.add(f) — with no isStaticDedupFrameAnalysisSafe clamp, and totalFrames isn't computed until after the loop (L2911) so there's nothing to clamp against. It's not reachable via repeat:-1 (its interval ends come from single-iteration child.duration(), not the totalDuration() sentinel), so no live crash — but the engine has no duration validator of its own (validateRenderDuration is producer-only), so a standalone-engine capture with a contrived huge single-iteration tween duration would still OOM here. Worth the same clamp for defense-in-depth symmetry, since the fix is currently asymmetric within its own file.

2. (lint rule, non-blocking) gsap_repeat_floor_unclamped false-negatives on the compute-into-a-variable form. The matcher repeat\s*:\s*Math\.floor\s*\([^)]+\)\s*-\s*1 — [^)]+ can't cross a nested ), so it misses (a) function-call-inside repeat: Math.floor(tl.duration()/cycle) - 1, (b) variable-indirection const repeat = Math.floor(d/c) - 1; gsap.timeline({ repeat }) — which is the exact idiom the PR's own updated sequencing.md example teaches — and (c) Math.floor(d/c - 1). Warning-only + the runtime guard is the real defense, so non-blocking, but it shouldn't be leaned on as the primary catch for the variable form.

3. (coverage) No end-to-end regression test renders a repeat:-1 composition through executeRenderJob asserting a clean PlanValidationError instead of Set maximum size exceeded. Each layer is well-unit-tested, but the regression isn't locked at the level it actually occurred.

4. (backward-compat — verified safe) The "Distributed"→"Render" message generalization doesn't affect retry semantics — distributed retry classification keys on the error code (DISTRIBUTED_DURATION_OUT_OF_RANGE, unchanged + still re-exported), not the message text; the only in-repo consumer of the old string (planSizeCap.test.ts) was updated in the alignment commit, and I found no other. (External EF message-scraping is covered by the PR's pin-bump note.)

Net: core fix is correct + complete for the reported crash. Findings 1-3 are non-blocking follow-ups (I'd prioritize the twin-clamp in #1). Not stamping — routes to James.

— Jerrai

[vanceingalls]

Reviewed at head 52da8d3cdb6e2a5645fdb67c7278ad25b2009c52 after the test: align distributed duration assertion commit. CI is green (42 passing, 0 failed). Approving to clear the gate.

What I verified independently

• Head SHA matches what Rames validated end-to-end. The empirical worktree reproduction Rames ran (sentinel-duration comp → PlanValidationError + static-dedup frame analysis limit (1000000) degradation, no RangeError) was against this exact commit. His counterfactual (same fixture crashes on main at frameCapture.ts:2525 with Set maximum size exceeded) is the load-bearing empirical evidence for this fix — reproducing an author-side fix through the real plan() path with live headless Chromium is a stronger check than any unit suite would give.
• File list matches the described defense-in-depth. Both guards present: frameCapture.ts +26 (isStaticDedupFrameAnalysisSafe at 1M cap, degrades to non-dedup), renderOrchestrator.ts +6 (validateRenderDuration closes the regular-path gap), planValidation.ts +20/-13 (generic validator + backward-compat validateDistributedDuration wrapper). The Math.max(0, Math.floor(d/c) - 1) sweep landed across gsap.ts (lint), plus 4 doc/skill files.
• CI-red fix landed. planSizeCap.test.ts +3/-1 — the /distributed/i → /render duration/i assertion Rames identified. Confirms the message-change contract update is aligned with existing tests.
• Alias alignment test locked in. The backward-compatible validateDistributedDuration wrapper preserves existing distributed-path adopters while validateRenderDuration becomes canonical.

Non-blockers acknowledged (deferred per James)

Rames flagged three follow-up items — twin clamp on computeTimelineAtRiskFrames, lint false-negative on the variable-assigned repeat idiom, and a missing e2e regression test locking the repeat: -1 case. James chose to ship the core fix as-is and address these separately. That's the right call — the crash reported is bounded by both guards; the twin isn't repeat:-1-reachable today; the lint false-negative is a completeness ask for a rule that already fires at author time on the more common form.

Verdict

Approve. Substantive review completed by Miga (structural), Rames (completeness + contract-audit + empirical worktree validation). My stamp closes the gate.

Rubber-stamp on vance's/James's behalf; substantive review by Miga + Rames — see Rames's GH review and his validation post.