refactor(producer): /simplify Phase 4 distributed-rendering changes

Address findings from a three-agent code-review pass over the Phase 4 stack:

- regression-harness: hoist `readdirSync` out of the per-checkpoint
  failure-extraction loop (was running 20 redundant syscalls on every
  failing png-sequence test). Drop redundant `existsSync` guards before
  `mkdirSync(recursive: true)` and `rmSync(force: true)`. Replace the
  three-deep ternary that built the output filename suffix with a
  single `Record<format, ext>` lookup.
- regression-harness-distributed: flatten the `format === "mp4" ? {...} : {...}`
  branching in the `plan()` call into a single config object with a
  conditional spread. `plan()` already accepts `codec: undefined` for
  non-mp4 formats, so the duplicate object was unnecessary.
- chunkBoundary.test: rename the stale "byte-identical mp4" test title
  to "byte-identical frames" (the test now uses png-sequence). Trim the
  10-line comment justifying `rejectOnSystemFonts: false` to the
  essential WHY.
- renderChunk / plan.test / regression-harness: drop trailing-edge
  comment phrases that pinned the prose to the PR's calendar context
  ("today", "v1.5", "pre-codec-knob output", section-numbered cross-
  references to the planning doc).

No behavior change. All 49 distributed unit tests pass. Smoke + four
distributed format fixtures pass in --mode=distributed-simulated.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Author: jrusso1020

packages/producer/src/regression-harness-distributed.ts

@@ -153,38 +153,25 @@ export async function runDistributedSimulatedRender(
   mkdirSync(planDir, { recursive: true });
   mkdirSync(chunksDir, { recursive: true });
 
-  // Step A: plan. `codec` is only forwarded when the format actually
-  // accepts it — `plan()` throws if codec is set for a non-mp4 format,
-  // and a caller passing `format: "mov", codec: undefined` would still
-  // surface that field in the resulting object. We omit it conditionally
-  // to keep the off-path planDir identical to pre-codec-knob output.
+  // Step A: plan. `plan()` throws when `codec` is set with a non-mp4 format,
+  // but `codec: undefined` is a no-op — so we forward it directly for mp4
+  // and elide it for the others rather than branching the entire config.
+  // hdrMode is pinned to force-sdr so the harness's behavior is independent
+  // of any future auto-detect changes.
   const planResult = await plan(
     input.projectDir,
-    input.format === "mp4"
-      ? {
-          fps: input.fps,
-          width: 1920,
-          height: 1080,
-          format: "mp4",
-          codec: input.codec,
-          chunkSize: input.chunkSize,
-          maxParallelChunks: input.maxParallelChunks,
-          hdrMode: "force-sdr",
-        }
-      : {
-          fps: input.fps,
-          // Required-by-type but overridden by the composition's own attrs;
-          // see docstring above. Any positive integer works.
-          width: 1920,
-          height: 1080,
-          format: input.format,
-          chunkSize: input.chunkSize,
-          maxParallelChunks: input.maxParallelChunks,
-          // Force the SDR path explicitly — `auto` would still resolve to
-          // force-sdr in distributed mode, but pinning it here keeps the
-          // harness's behavior independent of any future auto-detect changes.
-          hdrMode: "force-sdr",
-        },
+    {
+      fps: input.fps,
+      // Required-by-type but overridden by the composition's `data-width` /
+      // `data-height` attrs; any positive integer works.
+      width: 1920,
+      height: 1080,
+      format: input.format,
+      ...(input.format === "mp4" && input.codec !== undefined ? { codec: input.codec } : {}),
+      chunkSize: input.chunkSize,
+      maxParallelChunks: input.maxParallelChunks,
+      hdrMode: "force-sdr",
+    },
     planDir,
   );
 

packages/producer/src/regression-harness.ts

@@ -372,12 +372,10 @@ function discoverTestSuites(
     if (!statSync(dir).isDirectory()) continue;
     if (entry === "node_modules" || entry.startsWith(".")) continue;
 
-    // `tests/distributed/<name>/` is the home for fixtures authored
-    // specifically for the distributed pipeline (see tests/README.md and
-    // DISTRIBUTED-RENDERING-PLAN.md §10.2). Recurse one level deeper so
-    // each `<name>` becomes a first-class fixture ID (`mp4-h264-sdr`,
-    // `mov-prores`, …) the user can target on the CLI without their
-    // namespace prefix.
+    // `tests/distributed/<name>/` holds fixtures authored for the
+    // distributed pipeline. Recurse one level deeper so each `<name>`
+    // becomes a first-class fixture ID the user can target on the CLI
+    // without a namespace prefix.
     if (entry === "distributed") {
       for (const sub of readdirSync(dir)) {
         const subDir = join(dir, sub);
@@ -547,9 +545,7 @@ function saveFailureDetails(
   snapshotHtml?: string,
 ): void {
   const failuresDir = join(suite.dir, "failures");
-  if (!existsSync(failuresDir)) {
-    mkdirSync(failuresDir, { recursive: true });
-  }
+  mkdirSync(failuresDir, { recursive: true });
 
   // Save compilation failures
   if (result.compilation && !result.compilation.passed) {
@@ -608,30 +604,36 @@ function saveFailureDetails(
     const framesToExtract = failedCheckpoints.slice(0, 10);
     if (framesToExtract.length > 0) {
       const framesDir = join(failuresDir, "frames");
-      if (!existsSync(framesDir)) {
-        mkdirSync(framesDir, { recursive: true });
-      }
+      mkdirSync(framesDir, { recursive: true });
 
       const renderedIsDir =
         existsSync(renderedVideoPath) && statSync(renderedVideoPath).isDirectory();
       logPretty(`Extracting ${framesToExtract.length} failed frames...`, "📸");
 
+      // For directory output, sort both frame lists once — they're static for
+      // the duration of the failure-extraction loop, so the per-checkpoint
+      // readdir+filter+sort the loop did before was wasted syscalls.
+      const renderedDirFrames = renderedIsDir
+        ? readdirSync(renderedVideoPath)
+            .filter((n) => n.toLowerCase().endsWith(".png"))
+            .sort()
+        : null;
+      const snapshotDirFrames = renderedIsDir
+        ? readdirSync(snapshotVideoPath)
+            .filter((n) => n.toLowerCase().endsWith(".png"))
+            .sort()
+        : null;
+
       for (const checkpoint of framesToExtract) {
         const timeStr = checkpoint.time.toFixed(2).replace(".", "_");
         try {
-          if (renderedIsDir) {
+          if (renderedDirFrames && snapshotDirFrames) {
             const frameIndex = Math.max(
               0,
               Math.round(checkpoint.time * fpsToNumber(suite.meta.renderConfig.fps)),
             );
-            const renderedFrames = readdirSync(renderedVideoPath)
-              .filter((n) => n.toLowerCase().endsWith(".png"))
-              .sort();
-            const snapshotFrames = readdirSync(snapshotVideoPath)
-              .filter((n) => n.toLowerCase().endsWith(".png"))
-              .sort();
-            const renderedFrame = renderedFrames[frameIndex];
-            const snapshotFrame = snapshotFrames[frameIndex];
+            const renderedFrame = renderedDirFrames[frameIndex];
+            const snapshotFrame = snapshotDirFrames[frameIndex];
             if (renderedFrame !== undefined) {
               copyFileSync(
                 join(renderedVideoPath, renderedFrame),
@@ -717,17 +719,15 @@ async function runTestSuite(
   const tempDownloadDir = join(tempRoot, "downloads");
   const outputFormat = suite.meta.renderConfig.format ?? "mp4";
   const isPngSequence = outputFormat === "png-sequence";
-  // png-sequence output is a directory; encoded video outputs (mp4/mov/webm)
-  // are single files. `outputSuffix` is appended to the in-temp + baseline
-  // names so both shapes round-trip cleanly.
-  const outputSuffix = isPngSequence
-    ? ""
-    : outputFormat === "mp4"
-      ? ".mp4"
-      : outputFormat === "mov"
-        ? ".mov"
-        : ".webm";
-  const outputBasename = isPngSequence ? "frames" : `output${outputSuffix}`;
+  // png-sequence output is a directory (basename = "frames"); encoded video
+  // formats produce a single file (basename = "output.<ext>"). One lookup
+  // covers both shapes for the in-temp render and the on-disk baseline.
+  const VIDEO_EXT: Record<"mp4" | "mov" | "webm", string> = {
+    mp4: ".mp4",
+    mov: ".mov",
+    webm: ".webm",
+  };
+  const outputBasename = isPngSequence ? "frames" : `output${VIDEO_EXT[outputFormat]}`;
   const renderedOutputPath = join(tempRoot, outputBasename);
 
   // Snapshot files stored in test's output/ directory. For png-sequence the
@@ -882,10 +882,9 @@ async function runTestSuite(
       }
       if (isPngSequence) {
         // Frames directory — recursive copy so every PNG lands at
-        // `<snapshotDir>/frames/<frame-N>.png`.
-        if (existsSync(snapshotVideoPath)) {
-          rmSync(snapshotVideoPath, { recursive: true, force: true });
-        }
+        // `<snapshotDir>/frames/<frame-N>.png`. `rmSync(..., force: true)`
+        // tolerates a missing path, so the prior existsSync gate was redundant.
+        rmSync(snapshotVideoPath, { recursive: true, force: true });
         cpSync(renderedOutputPath, snapshotVideoPath, { recursive: true });
       } else {
         copyFileSync(renderedOutputPath, snapshotVideoPath);

packages/producer/src/services/distributed/chunkBoundary.test.ts

@@ -1,34 +1,23 @@
 /**
  * Per-adapter chunk-boundary contract: rendering the same composition at
  * chunkSize=N (single chunk, no seams) vs chunkSize=N/4 (four chunks, three
- * seams at frames 15, 30, 45) MUST produce byte-identical *frames*. This
- * is the strongest contract a distributed render can satisfy — anything
+ * seams at frames 15, 30, 45) MUST produce byte-identical *frames*. Anything
  * weaker means the worker's seek-determinism leaks across chunk boundaries.
  *
- * Output format is png-sequence rather than mp4 because mp4 bitstreams
- * encode keyframe placement directly: chunkSize=60 emits 1 IDR; chunkSize=15
- * emits 4 IDRs at frames 0/15/30/45. Those are legitimately different bytes
- * even when the captured pixels are identical. The png-sequence assemble
- * path merges chunk frame directories with no re-encode, so per-frame
- * byte equality round-trips a pixel-level contract.
+ * Output is png-sequence rather than mp4 because mp4 bitstreams encode
+ * keyframe placement directly: chunkSize=60 emits 1 IDR; chunkSize=15 emits
+ * 4 IDRs at frames 0/15/30/45. Those are legitimately different bytes even
+ * when the captured pixels are identical. The png-sequence assemble path
+ * merges chunk frame directories with no re-encode, so per-frame byte
+ * equality is exactly pixel equality.
  *
  * For each first-party adapter (GSAP, Anime.js, Three.js, Lottie, CSS,
  * WAAPI), `tests/distributed/<adapter>-boundary/src/index.html` is a
  * 60-frame composition that drives the adapter through its registered seek
- * hook. The test:
- *
- *   1. plan() + renderChunk() × N + assemble() at chunkSize=60 → N=1 chunk.
- *   2. Same at chunkSize=15 → N=4 chunks.
- *   3. Per-frame `Buffer.equals` across the two output frame directories.
- *
- * Fixtures with no checked-in baseline aren't compared by the regression
- * harness — they're driven from here via `bun test`. CI exercises them
- * through the same `bun test` step inside `Dockerfile.test`.
- *
- * Soft-skip behavior matches `renderChunk.test.ts`: if the host's
- * `chrome-headless-shell` can't render (no SwiftShader, missing GL stack),
- * the test logs a warning and returns. The Docker harness covers the real
- * contract against a known-good image.
+ * hook. The fixtures intentionally lack a `meta.json` so they're invisible
+ * to the regression harness; this test owns them. On hosts whose
+ * chrome-headless-shell can't render (no SwiftShader / missing GL stack),
+ * each subtest soft-skips and the Docker harness covers the contract.
  */
 
 import { afterAll, beforeAll, describe, expect, it } from "bun:test";
@@ -86,15 +75,11 @@ async function planAndAssemble(input: {
       // affects keyframe placement in the bitstream.
       format: "png-sequence",
       chunkSize: input.chunkSize,
-      // Some adapter bundles (notably anime.js's IIFE) embed CSS-shaped
-      // strings inside their JS — `font-family: ui-monospace, monospace`
-      // for internal devtools styling. `validateNoSystemFonts` scans the
-      // entire compiled HTML and matches those JS string literals, which
-      // would false-positive every chunk-boundary fixture that loads
-      // such a bundle. Disable the check for this test only; the fixtures
-      // never display text and the byte-identity contract is independent
-      // of which fonts the page would resolve. This is the documented
-      // escape hatch for the option.
+      // anime.js's IIFE bundle embeds `font-family: ui-monospace, monospace`
+      // as a string literal inside its JS, which `validateNoSystemFonts`'s
+      // document-wide regex false-positives. These fixtures display no text,
+      // so disabling the check (the documented escape hatch on this flag) is
+      // safe.
       rejectOnSystemFonts: false,
     },
     planDir,
@@ -122,7 +107,7 @@ describe("per-adapter chunk-boundary byte equality", () => {
 
   for (const adapter of ADAPTERS) {
     it(
-      `${adapter}: chunkSize=60 (N=1) vs chunkSize=15 (N=4) produces byte-identical mp4`,
+      `${adapter}: chunkSize=60 (N=1) vs chunkSize=15 (N=4) produces byte-identical frames`,
       async () => {
         const fixtureDir = join(testsDistributedDir, `${adapter}-boundary`);
         if (!existsSync(join(fixtureDir, "src", "index.html"))) {

packages/producer/src/services/distributed/plan.test.ts

@@ -242,9 +242,8 @@ describe("plan() — codec knob", () => {
         readFileSync(join(planDir, "meta", "encoder.json"), "utf-8"),
       ) as Record<string, unknown>;
       expect(encoder.encoder).toBe("libx265-software");
-      // SDR 8-bit yuv420p, same as h264. Distributed mode is SDR-only —
-      // anyone reading this and tempted to bump to 10-bit, that's HDR
-      // territory and lives in v1.5.
+      // SDR 8-bit yuv420p, same as h264 — distributed mode is SDR-only and
+      // 10-bit / HDR pixelFormat selection is not exposed on this surface.
       expect(encoder.pixelFormat).toBe("yuv420p");
     },
     TIMEOUT_MS,