Skip to content

test(producer): add png-sequence distributed fixture#847

Merged
jrusso1020 merged 3 commits into
mainfrom
05-14-test_producer_add_png-sequence_distributed_fixture
May 15, 2026
Merged

test(producer): add png-sequence distributed fixture#847
jrusso1020 merged 3 commits into
mainfrom
05-14-test_producer_add_png-sequence_distributed_fixture

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 14, 2026

Description

Phase 4 of the distributed rendering plan: test fixtures (see DISTRIBUTED-RENDERING-PLAN.md §11 Phase 4 + §10 test strategy). This is PR 4.4 of the Phase 4 remainder, stacked on PR 4.2 (#845) for the harness discovery + LFS pattern infrastructure.

This PR adds the png-sequence fixture (tests/distributed/png-sequence/) plus the harness extensions required to render and compare a frame-directory output. The composition is a 2-second (60-frame) scene at 30fps with a transparent background, a crossfade transition straddling the frame-30 chunk seam, and a continuously rotating SVG icon. renderConfig.format: "png-sequence" and chunkSize: 15 produces N=4 chunks; assemble() for png-sequence merges chunk frame directories rather than concat-copying mp4 files. The assertion is per-frame byte equality (Buffer.equals) — distributed-simulated produces a frames directory byte-identical to the in-process baseline. All 60 frames match.

Harness extensions:

  1. validateMetadata accepts format: "mp4" | "webm" | "mov" | "png-sequence".
  2. checkDistributedSupport accepts the same set; only webm is refused at plan time.
  3. Output path handling branches on isPngSequence: file vs directory, output.<ext> vs frames/.
  4. runDistributedSimulatedRender call passes the user-supplied format through (was previously hardcoded to "mp4").
  5. Visual comparison branches: PSNR sampling for video formats, per-frame Buffer.equals for png-sequence. The frame count check fails fast on a frame-count mismatch.
  6. Audio comparison skips entirely for png-sequence (no audio in a frame-directory output).
  7. Failure-detail extraction branches: copies the failing PNGs directly for png-sequence (extractFrameAsImage would try to invoke ffmpeg on a directory).
  8. --update uses cpSync({recursive: true}) for png-sequence to roundtrip the whole frames directory.

LFS:

  • .gitattributes adds packages/producer/tests/distributed/*/output/frames/*.png so baseline frames don't bloat the working repo. 60 frames × ~12 KB each ≈ 700 KB; with more fixtures the unfiltered footprint would grow, so LFS-tracking from the first fixture forward keeps the repo lean.

Testing

  • bun run --cwd packages/producer docker:test:update png-sequence — baseline generated inside Dockerfile.test (60 RGBA PNGs in output/frames/)
  • bun run --cwd packages/producer docker:test png-sequence — in-process passes (60/60 frames byte-identical against baseline)
  • bun run --cwd packages/producer docker:test:distributed png-sequence — distributed-simulated passes (60/60 frames byte-identical against baseline)
  • bun run --cwd packages/producer docker:test:distributed font-variant-numeric many-cuts gsap-letters-render-compat style-1-prod sub-composition-video mp4-h264-sdr -- --sequential — smoke set + the prior fixture all pass (6/6)
  • bunx oxlint + bunx oxfmt --check clean on changed files
  • bunx tsc --noEmit (producer package) clean

🤖 Generated with Claude Code

miguel-heygen
miguel-heygen previously approved these changes May 14, 2026
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

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

Re-reviewed as part of the fixture stack. LGTM.

vanceingalls
vanceingalls previously approved these changes May 14, 2026
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

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

Additive review — Miguel already approved; one substantive concern around the byte-identity threshold and a few nits. The PR is solid.

Calibrated strengths:

  • The fixture meaningfully stresses chunk boundaries: chunkSize=15 with 60 frames produces 4 chunks ([0-14]/[15-29]/[30-44]/[45-59]); the GSAP crossfade at 0.9s + 0.2s straddles frame 30 (the chunk-1→chunk-2 seam) and the continuous icon rotation spans every seam (tests/distributed/png-sequence/src/index.html:1219-1221). This is a real test of mergePngFrameDirs's global re-numbering correctness, not a smoke test.
  • --update is rejected at parse time for --mode=distributed-simulated (regression-harness.ts:176-186). That keeps the invariant "in-process is the source of truth for baselines" enforced — without it, a developer could regenerate baselines from a buggy distributed run and lock in the bug.
  • Both naming sites (in-process: encodeStage.ts:134; distributed merge: assemble.ts:255) use the same frame_${(i+1).padStart(6, "0")}.png pattern. The defensive renderedFrameName === snapshotFrameName check at regression-harness.ts:906 will loudly catch any future padding drift on either side — exactly the right shape of defense.

Important:

  • nit-leaning-importantmaxFrameFailures: 0 is the strictest possible threshold for the first byte-identity fixture in the suite. tests/distributed/png-sequence/meta.json:6 pins zero per-frame drift. The PR comment (regression-harness.ts:866-869) defends this — "metadata-chunk reorder would also be a regression" — and the defense is real. But the upstream inputs (Chrome's CDP screenshot bytes, libpng deflate output) can shift on a Chromium or zlib version change in the Docker image, and when that happens this fixture will fail with no code change. That's the intended invariant, but worth surfacing as a known tradeoff: the day Chromium upgrades, this fixture is the canary that goes red first. A short note in the fixture's description (or a KNOWN-DRIFT-SOURCES.md adjacent to the fixture) flagging that the failure mode on a Chrome bump is "regenerate baselines" rather than "investigate regression" would save a future on-call cycle.

Nits:

  • regression-harness.ts:882-890 — the frame-count-mismatch early-return reports failedFrames: Math.abs(rendered - snapshot) and forces result.passed = false regardless of maxFrameFailures. That's correct behavior (count mismatch IS pathological), but the harness now has two unequal contracts for "what maxFrameFailures gates": count-mismatch always fails; per-frame byte drift respects the threshold. A one-line comment at the early-return clarifying that count mismatch is always fatal would prevent a future reader from "fixing" this to honor the threshold.
  • tests/distributed/png-sequence/meta.json:8-9minAudioCorrelation and maxAudioLagWindows are vestigial for png-sequence (audio path is skipped at regression-harness.ts:999). Schema requires them and keeping the schema uniform is right. No action; flagging for future schema cleanup if other audio-less fixture shapes land.
  • regression-harness.ts:887 + 1018-1022 — two sites set the same "audio defaults to pass" result object. If audio handling ever evolves (e.g., a new "audio-required" assertion), both sites would need to change. Small consolidation opportunity, not load-bearing.

Verdict: APPROVE
Reasoning: The fixture exercises real chunk-boundary semantics, naming conventions match between in-process and distributed paths so the byte-equality assertion is sound, and the --update gate prevents the baseline-corruption failure mode. The maxFrameFailures: 0 tradeoff is defensible; the note above is a heads-up, not a blocker.

Review by Vai

…or png-sequence fixture

Address @vanceingalls review on #847: the maxFrameFailures=0 byte-identity
threshold will fail when Chromium's CDP screenshot bytes or libpng's
deflate output shifts on a Docker image bump. Pin the recovery procedure
in the fixture's description so a future on-call sees 'regenerate
baselines' rather than spending time investigating a non-regression.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@jrusso1020 jrusso1020 force-pushed the 05-14-test_producer_add_png-sequence_distributed_fixture branch from 7dbdb7b to 3c29060 Compare May 14, 2026 23:52
@jrusso1020
Copy link
Copy Markdown
Collaborator Author

Thanks @vanceingalls — addressed in commit 4423ac62. The fixture description now flags that the maxFrameFailures: 0 byte-identity gate will fail on a Chromium or zlib bump in Dockerfile.test and that the recovery is docker:test:update <name> rather than "investigate regression." The other nits (count-mismatch fatal vs threshold comment; vestigial audio fields) are minor — leaving as follow-ups.

Base automatically changed from 05-14-test_producer_add_mp4_h.264_sdr_distributed_fixture to main May 15, 2026 00:25
@jrusso1020 jrusso1020 dismissed stale reviews from vanceingalls and miguel-heygen May 15, 2026 00:25

The base branch was changed.

@jrusso1020 jrusso1020 merged commit 85dd8bb into main May 15, 2026
34 checks passed
@jrusso1020 jrusso1020 deleted the 05-14-test_producer_add_png-sequence_distributed_fixture branch May 15, 2026 01:07
jrusso1020 added a commit that referenced this pull request May 15, 2026
…852)

## Description

Phase 4 of the distributed rendering plan: test fixtures (see DISTRIBUTED-RENDERING-PLAN.md §11 Phase 4 + §10 test strategy). This is PR 4.7 of the Phase 4 remainder — the final fixture PR.

This PR adds six per-adapter chunk-boundary fixtures under `tests/distributed/{gsap,anime,three,lottie,css,waapi}-boundary/` plus a single `bun:test` driver (`chunkBoundary.test.ts`) that exercises each fixture's seek-determinism contract. Each fixture is a 60-frame composition (2s @ 30fps, 320×180) that drives the named adapter through the HyperFrames runtime's seek hook. The test renders each at `chunkSize=60` (N=1 chunk, no seams) and `chunkSize=15` (N=4 chunks, three seams at frames 15/30/45), then asserts every PNG frame is byte-identical across the two runs.

**Why png-sequence**: mp4 bitstreams encode keyframe placement directly. At `chunkSize=60` libx264 emits 1 IDR; at `chunkSize=15` it emits 4 IDRs at frames 0/15/30/45. Those are legitimately different bytes even when the captured pixels are identical. png-sequence's assemble path merges chunk frame directories with no re-encode, so per-frame byte equality is exactly pixel equality — the strongest contract a distributed render can satisfy.

**Fixture design** (each is ~60 lines of HTML):
- `gsap-boundary` — single GSAP `tl.to(...)` driving translateX + rotation linearly across 2s.
- `anime-boundary` — anime.js v4 timeline registered via `window.__hfAnime`.
- `three-boundary` — minimal Three.js scene; cube rotation derived from `window.__hfThreeTime`.
- `lottie-boundary` — inline Lottie JSON (rectangle layer animating position+rotation) loaded via `lottie-web` and registered via `window.__hfLottie`.
- `css-boundary` — pure `@keyframes` animation; the HyperFrames CSS adapter seeks via `animation-delay`.
- `waapi-boundary` — `element.animate()` with linear keyframes; runtime sets `currentTime` per frame.

The fixtures intentionally omit `meta.json` so the regression-harness discovery skips them with a clear `missing meta.json` log (they're driven exclusively by `chunkBoundary.test.ts`). The test passes `rejectOnSystemFonts: false` because some adapter bundles (notably anime.js's IIFE) embed CSS-shaped strings inside their JS source — `font-family: ui-monospace, monospace` for internal devtools styling — which `validateNoSystemFonts`'s document-wide regex would otherwise false-positive on every adapter fixture that loads such a bundle. The fixtures display no text, so the relaxed font validation doesn't affect the contract under test.

The 7th test case is a layout sanity check that asserts every expected `*-boundary` fixture directory exists.

## Testing

- `bun test packages/producer/src/services/distributed/chunkBoundary.test.ts` — 7 tests pass on host (6 adapters × byte-identical N=1 vs N=4 + the layout check, 41.6s)
- `bun test packages/producer/src/services/distributed/` — all 49 distributed unit tests pass (43.6s)
- `bun run --cwd packages/producer docker:test:distributed font-variant-numeric many-cuts gsap-letters-render-compat style-1-prod sub-composition-video mp4-h264-sdr png-sequence mov-prores mp4-h265-sdr -- --sequential` — full smoke set + all four prior stacked fixtures pass (9/9)
- `bunx oxlint` + `bunx oxfmt --check` clean
- `bunx tsc --noEmit` (producer package) clean

## After this stack lands

The Phase 4 fixture set is complete: PR 4.6 (#844) pins cross-worker idempotency; 4.2/4.3/4.4/4.5 (#845/#851/#847/#848) prove each format produces correct chunked output at the fixture's `minPsnr`; 4.3-pre (#850) added the codec knob H.265 needed; and this PR proves each first-party adapter's seek-determinism survives chunk seams. Phase 5 (CLI surface for `hyperframes plan/chunk/assemble`) and Phase 6 (AWS Lambda turnkey) are unblocked.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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