Skip to content

test(producer): add cross-worker idempotency unit test#844

Merged
jrusso1020 merged 1 commit into
mainfrom
05-14-test_producer_add_cross-worker_idempotency_unit_test
May 15, 2026
Merged

test(producer): add cross-worker idempotency unit test#844
jrusso1020 merged 1 commit into
mainfrom
05-14-test_producer_add_cross-worker_idempotency_unit_test

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.6 of the Phase 4 remainder; PR 4.1 (#827) landed the harness mode and the Phase 3 follow-up fixes the harness surfaced.

This PR adds a cross-worker idempotency unit test at packages/producer/src/services/distributed/crossWorkerIdempotency.test.ts. The test plans a 5-frame composition once per format, renders chunk 0 twice into two distinct temp paths, and asserts the resulting bytes are identical — both via the engineered `ChunkResult.sha256` fingerprint and via explicit `Buffer.equals` comparison of every output file. Covers both SDR mp4 (BeginFrame + libx264 encode) and png-sequence (screenshot capture, no encoder) per §10.1 axis 2. Runs via `bun test`, no Docker required — soft-skips when the host's chrome-headless-shell stack can't render (the Docker harness covers the determinism contract against a known-good image).

The existing `renderChunk.test.ts` already pins the byte-identical-retry contract for png-sequence by comparing the sha256 fingerprint; this file complements it with (1) explicit byte-level verification (independent of the sha256 helper) and (2) the mp4 path that `renderChunk.test.ts` skips to dodge BeginFrame on dev hosts.

Testing

  • `bun test packages/producer/src/services/distributed/crossWorkerIdempotency.test.ts` — both subtests pass on host (6.4s, 13 expect() calls)
  • `bun test packages/producer/src/services/distributed/` — all 41 distributed unit tests pass (7.9s)
  • `bunx oxlint` + `bunx oxfmt --check` clean
  • `bunx tsc --noEmit` (producer package) clean

🤖 Generated with Claude Code

Copy link
Copy Markdown
Collaborator Author

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

@jrusso1020 jrusso1020 force-pushed the 05-14-test_producer_add_cross-worker_idempotency_unit_test branch from 93e5ba2 to 4b9a17d Compare May 14, 2026 23:31
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)
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.

Cross-worker idempotency test — explicit byte-level verification + mp4 path coverage. Clean addition to the distributed test suite. LGTM.

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.

Full Review: Cross-worker idempotency test

Well-written test. Exercises both rendering paths (png-sequence via screenshot capture, mp4 via BeginFrame + libx264) and verifies idempotency at two layers: ChunkResult.sha256 fingerprint and raw Buffer.equals on every output file. Frame-dir assertion also catches missing/extra frames.

Flaky risk: Low. Chrome dependency is soft-skipped via HOST_CHROME_FAILURE_PATTERNS, ffmpeg dependency likewise. 120s timeout is generous for cold Chrome starts. Temp dirs cleaned in afterAll.

Minor note: Soft-skip uses console.warn + return rather than a test framework skip API, so CI reports "pass" instead of "skip" when Chrome is unavailable. Consistent with renderChunk.test.ts pattern — acceptable, but test.skip() at runtime would improve visibility if bun:test adds it.

No changes requested. LGTM.

@jrusso1020 jrusso1020 merged commit 808b10c into main May 15, 2026
39 checks passed
@jrusso1020 jrusso1020 deleted the 05-14-test_producer_add_cross-worker_idempotency_unit_test branch May 15, 2026 03:33
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.

Post-merge advisory review (already merged into main at 808b10cb). Findings flagged for follow-up, not gating. Prior coverage: @miguel-heygen's first review approved the diff and his second flagged the console.warn-vs-test.skip() visibility nit — additive review below.

Strengths

  • crossWorkerIdempotency.test.ts:114-118 — explicit Buffer.equals after the sha256 equality assertion is the right shape. Comment honestly names the rationale ("if the sha256 helper ever regresses... this assertion still fails the test honestly"); that's the value-add over renderChunk.test.ts's sha-only path.
  • crossWorkerIdempotency.test.ts:101-108assertBytesEqual in "frame-dir" mode compares readdirSync().sort() listings first, so missing/extra frames trip the assertion before the per-file bytes check. Cheap, complete.
  • mp4-path coverage closes a real gap — renderChunk.test.ts only exercises png-sequence (it forces format: "png-sequence" to dodge BeginFrame), so without this file the BeginFrame + libx264 path had no byte-identity unit pin at all.

Findings

should-be-follow-up-ticket — producer unit tests don't run in CI. ci.yml:134 runs bun run --filter '!@hyperframes/producer' test, explicitly excluding @hyperframes/producer. windows-render.yml:398-400 does the same with a comment ("producer package is skipped because its tests require Docker"). Dockerfile.test's ENTRYPOINT is tsx src/regression-harness.ts, not bun test. Net: this file — and the 7 other .test.ts siblings in services/distributed/ (assemble, chunkBoundary, plan, planFormatBanlist, planSizeCap, publicExports, renderChunk) — are not invoked by any CI job on this repo. The PR's "Testing" section accurately reports the local bun test runs, but no CI gate enforces this on future PRs. This is pre-existing debt, not a regression introduced by #844, and the Docker harness exercises the determinism contract end-to-end against a known-good chrome+ffmpeg image — so the production property is still covered. But the unit assertions in this file (including the load-bearing Buffer.equals byte-identity check it's designed to add) will silently rot if a future PR breaks them. Worth a follow-up ticket: either (a) add a producer-unit-tests CI job that runs bun test packages/producer/src/services/distributed/ on hosts where Chrome can't render (the soft-skip already covers that case), or (b) wire these .test.ts files into the Docker harness.

important — chunk 0 only. crossWorkerIdempotency.test.ts:130, 178 — both subtests render renderChunk(planDir, 0, ...). The byte-identity contract is (planDir, chunkIndex) → bytes for every chunkIndex, and chunk 0 is the chunk most likely to be special-cased by the IDR + GOP-locking logic. A regression that broke determinism specifically for chunks N>0 (e.g. mishandled seek offset, wrong frame indexing into video-frames/) wouldn't trip either subtest. Cheap fix on a follow-up: parameterize over chunk index, run at least one non-zero chunk per format.

nit — soft-skip regex duplicated. crossWorkerIdempotency.test.ts:46-47 hoists HOST_CHROME_FAILURE_PATTERNS to a top-level const, but renderChunk.test.ts:184-186 keeps the same literal inline. They match exactly today; if one drifts, the soft-skip behavior diverges silently. Either lift both to a shared services/distributed/__test_utils__ module, or accept the duplication explicitly with a code comment pointing each file at the other.

nit — frame-dir mismatch reports differently from file mismatch. crossWorkerIdempotency.test.ts:97-108 — file mode uses expect().toBe(true), frame-dir mode throws a raw Error with custom byte-length detail. Both are useful info, but the inconsistency means failure ergonomics depend on which output kind the test is exercising. Pulling the frame-dir path through expect() (e.g. expect({ name, ok: a.equals(b) }).toEqual({ name, ok: true })) gets uniform bun-test failure framing.

nit — outDir asymmetry between subtests. :172 puts both mp4 outputs in a shared mp4-chunks/ dir; png-sequence uses two top-level dirs. Both are correct (mp4 outputs are files, png-sequence outputs are dirs); the symmetry break is cosmetic.

Verdict: COMMENT (post-merge: With fixes)
Reasoning: Test itself is well-shaped — explicit byte-level verification plus mp4-path coverage is a real gap-fill over renderChunk.test.ts. Two follow-ups worth tracking: (1) producer unit tests aren't wired into any CI job, so this file silently rots if it regresses; (2) chunk 0 only — extend to non-zero chunks on a follow-up.

Review by Vai

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