From 6b3ad09436d0c6467031fa2cfc874d729d38d230 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 14 May 2026 23:51:38 +0000 Subject: [PATCH] fix(producer): tighten chunk-boundary test gates + narrow VIDEO_EXT indexing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address @vanceingalls and @miguel-heygen review findings on #852: 1. Asymmetric soft-skip — only the N=1 plan+render+assemble call was wrapped in the host-Chrome-failure catch; an SwiftShader / cold-Chrome flake on the N=4 call would hard-fail instead of soft-skip. Factor a local runRender() helper and wrap both calls. 2. Vacuously-passing length assertion — 'expect(framesOne.length).toBe( framesFour.length)' passes when both runs produce 0 frames. Pin the absolute count (EXPECTED_FRAME_COUNT = 60) so a regression that identically truncates both renders shows red. 3. CDN version drift — anime-boundary loaded gsap@3.14.2 from jsdelivr while every other boundary fixture loaded 3.12.2 from cdnjs. Unify on cdnjs@3.12.2 so the next reader doesn't have to wonder why one fixture diverges. (gsap is an empty duration-driver in all six fixtures so the version was never load-bearing — but the divergence reads as intentional and isn't.) 4. VIDEO_EXT type narrowing — the lookup is Record<"mp4"|"mov"|"webm"> but outputFormat includes "png-sequence". The isPngSequence ternary short-circuits before png-sequence can reach the indexing site, but TS can't narrow through that. Add an explicit cast at the indexing site (not the lookup definition — over-widening to include "png-sequence": undefined would defeat the existence guarantee). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- packages/producer/src/regression-harness.ts | 9 ++- .../distributed/chunkBoundary.test.ts | 58 +++++++++++-------- .../distributed/anime-boundary/src/index.html | 2 +- 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/packages/producer/src/regression-harness.ts b/packages/producer/src/regression-harness.ts index cff48b16d..fee1bd8a0 100644 --- a/packages/producer/src/regression-harness.ts +++ b/packages/producer/src/regression-harness.ts @@ -722,12 +722,19 @@ async function runTestSuite( // png-sequence output is a directory (basename = "frames"); encoded video // formats produce a single file (basename = "output."). One lookup // covers both shapes for the in-temp render and the on-disk baseline. + // `VIDEO_EXT` is intentionally typed against only the encoded-video set — + // the `isPngSequence` ternary below short-circuits before `outputFormat` + // can be `"png-sequence"`, but TS can't narrow through that, so we + // assert the narrowing at the indexing site rather than over-widening + // the lookup table. const VIDEO_EXT: Record<"mp4" | "mov" | "webm", string> = { mp4: ".mp4", mov: ".mov", webm: ".webm", }; - const outputBasename = isPngSequence ? "frames" : `output${VIDEO_EXT[outputFormat]}`; + const outputBasename = isPngSequence + ? "frames" + : `output${VIDEO_EXT[outputFormat as "mp4" | "mov" | "webm"]}`; const renderedOutputPath = join(tempRoot, outputBasename); // Snapshot files stored in test's output/ directory. For png-sequence the diff --git a/packages/producer/src/services/distributed/chunkBoundary.test.ts b/packages/producer/src/services/distributed/chunkBoundary.test.ts index 5eba0b09e..6c598c187 100644 --- a/packages/producer/src/services/distributed/chunkBoundary.test.ts +++ b/packages/producer/src/services/distributed/chunkBoundary.test.ts @@ -37,6 +37,11 @@ const HOST_CHROME_FAILURE_PATTERNS = // assemble pipeline so no `output/` baseline is required. const ADAPTERS = ["gsap", "anime", "three", "lottie", "css", "waapi"] as const; +// Every adapter fixture is a 2-second composition at 30fps. Pin the absolute +// count so a regression that produces fewer frames in both runs (e.g. a +// probe stage that reads duration as 0s) doesn't pass vacuously. +const EXPECTED_FRAME_COUNT = 60; + let runRoot: string; let testsDistributedDir: string; @@ -122,30 +127,30 @@ describe("per-adapter chunk-boundary byte equality", () => { mkdirSync(workOne, { recursive: true }); mkdirSync(workFour, { recursive: true }); - let outOne: string; - try { - outOne = await planAndAssemble({ - projectDir, - workDir: workOne, - chunkSize: 60, - }); - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - if (HOST_CHROME_FAILURE_PATTERNS.test(message)) { - console.warn( - `[chunkBoundary.test] skipping ${adapter} — host Chrome can't render. ` + - "Docker harness covers the contract. Diagnostic:", - message.slice(0, 240), - ); - return; + // Soft-skip when host Chrome can't render. Wrap *both* renders — + // cold-Chrome / SwiftShader flakes happen on the second render + // as readily as the first, and a hard-fail on the N=4 path would + // diverge from the rest of the harness's soft-skip convention. + const runRender = async (workDir: string, chunkSize: number): Promise => { + try { + return await planAndAssemble({ projectDir, workDir, chunkSize }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (HOST_CHROME_FAILURE_PATTERNS.test(message)) { + console.warn( + `[chunkBoundary.test] skipping ${adapter} — host Chrome can't render. ` + + "Docker harness covers the contract. Diagnostic:", + message.slice(0, 240), + ); + return null; + } + throw err; } - throw err; - } - const outFour = await planAndAssemble({ - projectDir, - workDir: workFour, - chunkSize: 15, - }); + }; + const outOne = await runRender(workOne, 60); + if (outOne === null) return; + const outFour = await runRender(workFour, 15); + if (outFour === null) return; // Per-frame byte equality across the two frames directories. A // boundary regression in the adapter's seek-determinism would @@ -157,7 +162,12 @@ describe("per-adapter chunk-boundary byte equality", () => { const framesFour = readdirSync(outFour) .filter((n) => n.toLowerCase().endsWith(".png")) .sort(); - expect(framesOne.length).toBe(framesFour.length); + // Pin the absolute count, not just equality between the two runs. + // Otherwise a regression that truncates BOTH renders identically + // (e.g. a probe stage that misreads duration as 0s) would pass + // vacuously — `0 === 0` is true. + expect(framesOne.length).toBe(EXPECTED_FRAME_COUNT); + expect(framesFour.length).toBe(EXPECTED_FRAME_COUNT); expect(framesOne).toEqual(framesFour); for (let i = 0; i < framesOne.length; i++) { const frameName = framesOne[i]; diff --git a/packages/producer/tests/distributed/anime-boundary/src/index.html b/packages/producer/tests/distributed/anime-boundary/src/index.html index 76b64339c..b30885a61 100644 --- a/packages/producer/tests/distributed/anime-boundary/src/index.html +++ b/packages/producer/tests/distributed/anime-boundary/src/index.html @@ -3,7 +3,7 @@ chunk-boundary: anime.js - +