[kapelame]

What

A GIF with finite loop metadata still repeats when authored with data-loop="false". A two-frame, 0.4-second GIF with a loop count of 3 repeats three times inside a two-second clip instead of playing once and holding its last frame.

Why

GIF preparation resolves data-loop to a boolean that treats an explicit false and an absent override identically. The finite-loop branch then applies the source metadata in both cases. Removing the generated video's native loop attribute cannot undo the repetitions already baked into its WebM source.

How

Keep the nullable authored override through playback planning. Source loop counts apply only when no valid override was authored. Existing infinite-loop expansion and default finite-loop behavior remain covered by the surrounding tests.

Test plan

Standalone branch, based on f6041d7597c8c53d381d4c27feb86b305b3317a4:

• bun install --frozen-lockfile --ignore-scripts completed with an independent dependency tree.
• bun test packages/producer/src/services/animatedGifPrep.test.ts packages/producer/src/services/htmlCompiler.animatedGif.test.ts: 10 pass, 0 fail, 47 assertions. The regression covers explicit data-loop="false" with both infinite and finite source loop metadata; the existing default finite-loop test remains unchanged.
• Recursive import resolution checked 193 local workspace files and 423 workspace import edges. Both test entry points and the implementation resolve inside this standalone worktree, with no imports from the combined audit worktree and no producer/CLI build output reused. Only build output from unchanged dependency packages was copied.
• bunx oxlint packages/producer/src/services/animatedGifPrep.ts packages/producer/src/services/animatedGifPrep.test.ts, bunx oxfmt --check packages/producer/src/services/animatedGifPrep.ts packages/producer/src/services/animatedGifPrep.test.ts, and git diff --check pass.
• Unmodified pre-commit and commit-msg hooks passed: staged lint/format, tracked-artifact and large-file checks, fallow audit --base origin/main --fail-on-issues, Core/Studio/scripts type checks, and commitlint. Fallow reported advisory clone warnings but passed its new-issues gate.
• Documentation updated (not applicable; restores the existing override contract).

Prior combined audit

The regression was first reproduced against the base implementation (Expected: 1, Received: 3). An actual FFmpeg GIF transcode changed from 3 iterations plus a 0.8-second final-frame hold to 1 iteration plus a 1.6-second hold; both outputs were 2.000-second VP9 WebM. These before/after runs belong to the prior audit, not this standalone branch.

Earlier in the audit, the full workspace build passed while the separate initialization fix was still being completed. All workspace typechecks subsequently passed with the GIF and initialization fixes applied. The engine suite (1,737 tests, 3 skipped) and producer Bun unit lane (662 tests) also passed before the download fix was added. After adding that fix, the final combined checkout passed the CLI build/typecheck, repository lint/format checks, and CLI suite (3,213 tests, 3 skipped). These broader results were not rerun as whole-workspace gates on this standalone branch; its direct regression validation is the focused run above.

The prior full workspace run was not green on macOS arm64 / Node 25.9.0 / FFmpeg 8.1.1: an existing AAC loudness integration assertion also failed using the base implementation, browser suites encountered Node 25 native Web Storage behavior, and a producer integration expectation requires Linux BeginFrame capture. Linux/Windows CI, cloud adapters, and large-composition visual regression renders were not exercised locally.

Additional verification of the final combined state: after confirming that the original audit checkout contained exactly the three reviewed patches on f6041d7597c8c53d381d4c27feb86b305b3317a4, HYPERFRAMES_NO_TELEMETRY=1 bun run build and HYPERFRAMES_NO_TELEMETRY=1 bun run --filter '*' typecheck were rerun and both passed. Before/after source and diff hashes are identical. These new whole-workspace results cover the final three-fix combination; this standalone branch's own focused tests and hook checks remain documented separately above.

Current-upstream integration check: the three reviewed patches were also applied to a separate checkout of a0a6244be7a40956c5343f5846609e584ee5c6b5. With independent dependencies and a freshly rebuilt current Core runtime, 388 focused tests passed: audio route/transport (97), capture classification (19), render orchestration (222), GIF preparation/compiler (10), and init/download (40). A real local Chrome/FFmpeg smoke check reused the init-generated audio element in a minimal two-second scene with a finite-loop GIF and data-loop="false"; output was 160×90 H.264/AAC, 10 frames, 2.000 seconds, with one GIF playback and a 1.6-second tail hold. The screenshot retry branches were checked by unit tests, not an injected browser failure. Source/diff hashes remained unchanged. This verifies the combined patches against that upstream SHA; the independent commit and its original base remain unchanged.

[kapelame]

Hi! Friendly ping on this one — is there anything I can do to help move the review along? Happy to rebase, add tests, or clarify any of the changes if useful. Thanks for your time! 🙏

[kapelame]

Hi! Friendly ping on this one — is there anything I can do to help move the review along? Happy to rebase, add tests, or clarify any of the changes if useful. Thanks for your time! 🙏

[jrusso1020]

Approving. Reviewed at a301bb8b.

The bug is still on main. resolvePreparedPlayback gets a plain boolean, so an explicit data-loop="false" and no attribute at all look the same. Its else if (metadata.loopCount > 1) branch then bakes the GIF's own loop count into the WebM either way. Keeping the nullable override through to that branch, and applying the source loop count only when nothing was authored, is the right fix. Default behaviour (no attribute) and data-loop="true" are unchanged.

The test covers it. With loop count 3, main produces loopIterations: 3, the PR produces 1 with a 1.8s hold, and -stream_loop is gone from the transcode args. The loop count 0 case already passed on main, so it works as a control.

Small optional tidy-up: the call site still computes loop with resolveLoop (for the <video loop> attribute), and resolvePreparedPlayback now calls resolveLoop again internally. Passing loop in alongside loopOverride would avoid working it out twice, but it's fine as is.

Need: yes, bug present on main. Size: ok. Reuse: reuses the existing readLoopOverride / resolveLoop. Simpler: already minimal.

— Rames

[jrusso1020]

Thanks, @kapelame. This one is approved, and it'll go into the merge queue once these are sorted:

• CI hasn't run: only the Graphite check reported on this head. A rebase push will trigger it, and we'll approve the fork workflow runs.

Once it's green and signed, we'll queue it at your new head. If you push code changes beyond a rebase, we'll take another look first.

— Rames

[jrusso1020]

This is approved and still needed, but its CI runs are too old for GitHub to re-run, so no checks have reported. Could you rebase on current main and push? That starts a fresh run, and then we can get it merged.

— Rames

[kapelame]

Done — fresh CI is now running on the new head 4c3dd2cb3.

One deliberate deviation: I merged main into the branch instead of rebasing, so the already-pushed, signed commit a301bb8 keeps its exact hash and the push is a plain fast-forward (no force-push on pushed history). The resulting diff is unchanged — still just packages/producer/src/services/animatedGifPrep.ts and its test, and animatedGifPrep.test.ts passes locally (11/11).

If you'd rather have a linear history, say the word and I'll redo it as a true rebase.

[miguel-heygen]

Checked at the current head. I ran animatedGifPrep.test.ts (bun): 11 passed. With animatedGifPrep.ts put back to the merge base, the finite-loop case fails, so the test guards the change.

Verdict: APPROVE
Reasoning: honours a GIF's own loop count where GIF prep decides looping; test fails without it; CI green.