fix(capture,audio): close the three contract gaps raised in review

Review on #2892 (Rames, Magi) found the fixes correct inside the changed files but
incomplete at the contract level. All three hold up against source; two of the three
were reachable in production, and the plate one was self-inflicted by this PR.

**The plate guard checked a stale height.** `scrollHeight` was measured before the scroll
traversal and handed to the guard, but the plate is deliberately shot *after* it so lazy
content has loaded — and lazy loading grows the document. The guard's input therefore read
low on exactly the long pages it exists for, letting the check pass and a clipped plate
through, undetectable downstream because the skill only teaches the tile fallback when the
file is *absent*. `captureFullPagePlate` now measures the height itself at call time, and
verifies what Chrome actually produced by reading the PNG's IHDR before writing, since the
capture can trigger another round of loading. Over the cap, nothing is emitted.

**Assembly dropped the flag again.** `bgm_pending` survived into `audio_meta.json` but
`assemble-index.mjs` rebuilt its audio object from three named keys, so at the step that
actually builds the film "not ready yet" still looked like "silent by design" — this PR's
own framing of the defect, one layer further down. The flag rides along now, and a pending
bed with no file raises an anomaly instead of quietly assembling a silent cut against a
storyboard that promises music.

**The sibling adapters had both audio bugs, and there were two of them.** The review named
`faceless-explainer`; `pr-to-video` carries the same file. Its own test asserts the two are
byte-identical ("intentionally identical across the reusing skills"), so fixing one alone
broke that test — which is what caught the second copy. Both now carry the absence-sentinel
filter and the surviving `bgm_pending`, and `faceless-explainer` gets the same five
regression tests.

Also from review (Miga): the sticky-restore in `finally` is wrapped, so a page that broke
mid-capture cannot replace the real error with a cleanup one.

Validation: `vitest run src/capture` — 95 pass (5 new) · product-launch audio 13 pass ·
faceless-explainer audio 10 pass (5 new, incl. the byte-identity contract) ·
`bun run lint:skills` · oxlint/oxfmt clean · `tsc --noEmit` clean

Author: WaterrrForever

packages/cli/src/capture/screenshotCapture.test.ts

@@ -3,44 +3,62 @@ import { existsSync, mkdtempSync, readFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import type { Page } from "puppeteer-core";
-import { captureFullPagePlate, MAX_PLATE_HEIGHT_PX } from "./screenshotCapture.js";
+import { captureFullPagePlate, MAX_PLATE_HEIGHT_PX, pngHeight } from "./screenshotCapture.js";
+
+// A real 1920x800 PNG header, so the produced-height guard sees something valid.
+function pngBuffer(height: number, width = 1920): Buffer {
+  const buf = Buffer.alloc(24);
+  Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]).copy(buf, 0);
+  buf.writeUInt32BE(13, 8);
+  buf.write("IHDR", 12, "ascii");
+  buf.writeUInt32BE(width, 16);
+  buf.writeUInt32BE(height, 20);
+  return buf;
+}
 
 // The mocks declare their parameters so `mock.calls[i][0]` is a real slot — a zero-arg
 // vi.fn() types its call tuple as [] and indexing it is a compile error.
-function fakePage(overrides: Record<string, unknown> = {}) {
-  const evaluate = vi.fn(async (_script?: unknown) => undefined);
-  const screenshot = vi.fn(async (_opts?: unknown) => Buffer.from("PNG-BYTES"));
+// `docHeight` is what the in-function measurement returns; the plate reads the page height
+// itself now rather than trusting a value the caller measured before scrolling.
+function fakePage(
+  { docHeight = 8000, plateHeight = 8000 }: { docHeight?: number; plateHeight?: number } = {},
+  overrides: Record<string, unknown> = {},
+) {
+  const evaluate = vi.fn(async (script?: unknown) =>
+    String(script).includes("scrollHeight") ? docHeight : undefined,
+  );
+  const screenshot = vi.fn(async (_opts?: unknown) => pngBuffer(plateHeight));
   return { page: { evaluate, screenshot, ...overrides } as unknown as Page, evaluate, screenshot };
 }
 
 describe("captureFullPagePlate — the scroll shot's plate", () => {
   it("writes one full-page png and returns its relative path", async () => {
     const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
-    const { page, screenshot } = fakePage();
+    const { page, screenshot } = fakePage({ docHeight: 10962, plateHeight: 10962 });
 
-    const out = await captureFullPagePlate(page, dir, 10962);
+    const out = await captureFullPagePlate(page, dir);
 
     expect(out).toBe("screenshots/full-page.png");
     expect(screenshot).toHaveBeenCalledWith({ type: "png", fullPage: true });
-    expect(readFileSync(join(dir, "full-page.png"), "utf8")).toBe("PNG-BYTES");
+    expect(pngHeight(readFileSync(join(dir, "full-page.png")))).toBe(10962);
   });
 
   it("stays 1x: it never touches the viewport's deviceScaleFactor", async () => {
     const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
     const setViewport = vi.fn(async () => undefined);
-    const { page } = fakePage({ setViewport });
+    const { page } = fakePage({}, { setViewport });
 
-    await captureFullPagePlate(page, dir, 4000);
+    await captureFullPagePlate(page, dir);
 
     // A 2x plate would exceed the cap on exactly the long pages that want a scroll shot.
     expect(setViewport).not.toHaveBeenCalled();
   });
 
   it("skips a page taller than Chrome can capture, instead of writing a clipped plate", async () => {
     const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
-    const { page, screenshot } = fakePage();
+    const { page, screenshot } = fakePage({ docHeight: MAX_PLATE_HEIGHT_PX + 1 });
 
-    const out = await captureFullPagePlate(page, dir, MAX_PLATE_HEIGHT_PX + 1);
+    const out = await captureFullPagePlate(page, dir);
 
     expect(out).toBeNull();
     expect(screenshot).not.toHaveBeenCalled();
@@ -51,22 +69,24 @@ describe("captureFullPagePlate — the scroll shot's plate", () => {
     const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
     const { page, evaluate, screenshot } = fakePage();
 
-    await captureFullPagePlate(page, dir, 8000);
+    await captureFullPagePlate(page, dir);
 
     const scripts = evaluate.mock.calls.map((c) => String(c[0]));
-    expect(scripts).toHaveLength(2);
-    // Neutralise first — a fixed header would otherwise bake in mid-plate.
-    expect(scripts[0]).toContain("'fixed'");
-    expect(scripts[0]).toContain("'sticky'");
-    expect(scripts[0]).toContain("data-hf-plate-position");
-    // Then hand the page back unchanged: the caller keeps reading the DOM after this.
-    expect(scripts[1]).toContain("removeAttribute");
+    // height probe, neutralise, restore
+    expect(scripts).toHaveLength(3);
+    expect(scripts[0]).toContain("scrollHeight");
+    // Neutralise before the shot — a fixed header would otherwise bake in mid-plate.
+    expect(scripts[1]).toContain("'fixed'");
+    expect(scripts[1]).toContain("'sticky'");
     expect(scripts[1]).toContain("data-hf-plate-position");
-    expect(evaluate.mock.invocationCallOrder[0]).toBeLessThan(
+    // Then hand the page back unchanged: the caller keeps reading the DOM after this.
+    expect(scripts[2]).toContain("removeAttribute");
+    expect(scripts[2]).toContain("data-hf-plate-position");
+    expect(evaluate.mock.invocationCallOrder[1]).toBeLessThan(
       screenshot.mock.invocationCallOrder[0]!,
     );
     expect(screenshot.mock.invocationCallOrder[0]).toBeLessThan(
-      evaluate.mock.invocationCallOrder[1]!,
+      evaluate.mock.invocationCallOrder[2]!,
     );
   });
 
@@ -75,11 +95,63 @@ describe("captureFullPagePlate — the scroll shot's plate", () => {
     const screenshot = vi.fn(async (_opts?: unknown) => {
       throw new Error("capture failed");
     });
-    const { page, evaluate } = fakePage({ screenshot });
+    const { page, evaluate } = fakePage({}, { screenshot });
 
-    await expect(captureFullPagePlate(page, dir, 8000)).rejects.toThrow("capture failed");
+    await expect(captureFullPagePlate(page, dir)).rejects.toThrow("capture failed");
     // A page left with every sticky element forced static would corrupt the extraction
     // passes that run after this one.
     expect(String(evaluate.mock.calls.at(-1)?.[0])).toContain("removeAttribute");
   });
 });
+
+describe("captureFullPagePlate — guards against a silently clipped plate", () => {
+  it("measures the height itself, after lazy content has grown the page", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
+    // A page that measured 9000 before scrolling but is 20000 once lazy images land: the
+    // pre-scroll number would have passed the guard and emitted a clipped plate.
+    const { page, screenshot } = fakePage({ docHeight: 20000 });
+
+    expect(await captureFullPagePlate(page, dir)).toBeNull();
+    expect(screenshot).not.toHaveBeenCalled();
+  });
+
+  it("discards a plate Chrome clipped, even when the measurement passed", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
+    // Measurement said 16000, but the capture itself triggered more loading and came back
+    // over the cap. Emitting it would be undetectable downstream.
+    const { page } = fakePage({ docHeight: 16000, plateHeight: MAX_PLATE_HEIGHT_PX + 500 });
+
+    expect(await captureFullPagePlate(page, dir)).toBeNull();
+    expect(existsSync(join(dir, "full-page.png"))).toBe(false);
+  });
+
+  it("survives a restore that throws — the real error is what propagates", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
+    let call = 0;
+    const evaluate = vi.fn(async (script?: unknown) => {
+      call++;
+      if (String(script).includes("scrollHeight")) return 8000;
+      if (String(script).includes("removeAttribute")) throw new Error("page crashed");
+      return undefined;
+    });
+    const screenshot = vi.fn(async (_opts?: unknown) => {
+      throw new Error("capture failed");
+    });
+    const page = { evaluate, screenshot } as unknown as Page;
+
+    // Without the try/catch in `finally`, "page crashed" would mask "capture failed".
+    await expect(captureFullPagePlate(page, dir)).rejects.toThrow("capture failed");
+    expect(call).toBeGreaterThanOrEqual(3);
+  });
+});
+
+describe("pngHeight", () => {
+  it("reads the height out of the IHDR chunk", () => {
+    expect(pngHeight(pngBuffer(10962))).toBe(10962);
+  });
+
+  it("returns null for anything that is not a PNG", () => {
+    expect(pngHeight(Buffer.from("not a png at all, definitely not"))).toBeNull();
+    expect(pngHeight(Buffer.alloc(4))).toBeNull();
+  });
+});

packages/cli/src/capture/screenshotCapture.ts

@@ -27,6 +27,17 @@ import { join } from "node:path";
  */
 export const MAX_PLATE_HEIGHT_PX = 16384;
 
+/**
+ * Pixel height Chrome actually produced, read from the PNG's IHDR chunk: 8-byte signature,
+ * then 4 length + 4 type + 4 width + 4 height. Null when the buffer isn't a PNG.
+ */
+export function pngHeight(buf: Uint8Array): number | null {
+  // Byte math rather than Buffer helpers: page.screenshot() resolves to a Uint8Array.
+  if (buf.length < 24) return null;
+  if (buf[12] !== 0x49 || buf[13] !== 0x48 || buf[14] !== 0x44 || buf[15] !== 0x52) return null; // "IHDR"
+  return ((buf[20]! << 24) | (buf[21]! << 16) | (buf[22]! << 8) | buf[23]!) >>> 0;
+}
+
 /**
  * One tall image of the whole document — the plate a scroll shot slides its viewport over.
  *
@@ -48,9 +59,15 @@ export const MAX_PLATE_HEIGHT_PX = 16384;
 export async function captureFullPagePlate(
   page: Page,
   screenshotsDir: string,
-  scrollHeight: number,
 ): Promise<string | null> {
-  if (scrollHeight > MAX_PLATE_HEIGHT_PX) return null;
+  // Measured here rather than taken from the caller: the plate is deliberately shot AFTER the
+  // scroll traversal, and lazy content grows the document as it loads — a height measured
+  // before scrolling reads low on exactly the long pages this guard exists for, which would
+  // let the check pass and a clipped plate through.
+  const docHeight = (await page.evaluate(
+    `Math.max(document.body.scrollHeight, document.documentElement.scrollHeight)`,
+  )) as number;
+  if (docHeight > MAX_PLATE_HEIGHT_PX) return null;
 
   // Record the inline value before overwriting so the page is handed back unchanged — the
   // caller keeps using it (asset extraction, DOM reads) after this returns.
@@ -65,15 +82,27 @@ export async function captureFullPagePlate(
   );
   try {
     const buffer = await page.screenshot({ type: "png", fullPage: true });
+    // Confirm what Chrome produced instead of trusting the measurement: the capture itself can
+    // trigger another round of lazy loading. A clipped plate is undetectable downstream — the
+    // skill only teaches the tile fallback when the file is *absent* — so emit nothing rather
+    // than something silently wrong.
+    const produced = pngHeight(buffer);
+    if (produced != null && produced > MAX_PLATE_HEIGHT_PX) return null;
     writeFileSync(join(screenshotsDir, "full-page.png"), buffer);
     return "screenshots/full-page.png";
   } finally {
-    await page.evaluate(
-      `document.querySelectorAll('[data-hf-plate-position]').forEach((el) => {
-        el.style.position = el.getAttribute('data-hf-plate-position');
-        el.removeAttribute('data-hf-plate-position');
-      })`,
-    );
+    // A page that broke mid-capture will fail this too; letting that escape would replace the
+    // real error with a cleanup one. Nothing to restore if the page is already gone.
+    try {
+      await page.evaluate(
+        `document.querySelectorAll('[data-hf-plate-position]').forEach((el) => {
+          el.style.position = el.getAttribute('data-hf-plate-position');
+          el.removeAttribute('data-hf-plate-position');
+        })`,
+      );
+    } catch {
+      /* page unusable — the restore is moot */
+    }
   }
 }
 
@@ -212,7 +241,7 @@ export async function captureScrollScreenshots(page: Page, outputDir: string): P
     // dropped because 1/8 agents read it and the contact sheet covered the same ground — that
     // was about it as a *comprehension* artifact. The scroll shot is a different consumer: it
     // needs one continuous plate, which no set of viewport tiles can substitute for.)
-    const plate = await captureFullPagePlate(page, screenshotsDir, scrollHeight);
+    const plate = await captureFullPagePlate(page, screenshotsDir);
     if (plate) filePaths.push(plate);
   } catch {
     /* scroll screenshots are non-critical */

skills-manifest.json

@@ -6,7 +6,7 @@
       "files": 140
     },
     "faceless-explainer": {
-      "hash": "b772a9b6c8118c2c",
+      "hash": "b458fdb62c5e1402",
       "files": 22
     },
     "figma": {
@@ -58,11 +58,11 @@
       "files": 132
     },
     "pr-to-video": {
-      "hash": "44a9877e7ea1289e",
+      "hash": "01dc26f444bc20cd",
       "files": 29
     },
     "product-launch-video": {
-      "hash": "ead12de8df2ed55d",
+      "hash": "4412edf071681ceb",
       "files": 26
     },
     "remotion-to-hyperframes": {

skills/faceless-explainer/scripts/audio.mjs

@@ -105,14 +105,19 @@ function toProductLaunchMeta(neutral) {
         duration_s: neutral.bgm.duration_s ?? null,
       }
     : null;
+  // bgm_pending must survive the neutral → skill translation. A detached generate
+  // (Lyria/MusicGen) leaves `bgm: null, bgm_pending: true` until the track lands; dropping the
+  // flag makes "not ready yet" indistinguishable from "silent by design", so a later
+  // `fetch-sfx` snapshot turns a still-generating bed into no music at all with no signal.
+  const bgmPending = !!neutral.bgm_pending;
   const sfx = (neutral.sfx ?? []).map((s) => ({
     frame: Number(s.id),
     file: s.file,
     offset_s: s.offset_s ?? 0,
     duration_s: s.duration_s ?? 1,
     volume: s.volume ?? 0.35,
   }));
-  return { bgm, voices, sfx };
+  return { bgm, bgm_pending: bgmPending, voices, sfx };
 }
 
 // ── generate (TTS + BGM) ────────────────────────────────────────────────────
@@ -193,12 +198,16 @@ function runFetchSfx(argv) {
   const manifest = parseStoryboard(readFileSync(storyboardPath, "utf8"));
 
   // Per-frame `sfx:` cues (comma-separated) → engine lines carrying only sfx.
+  // `filter(Boolean)` alone is not enough: a storyboard that spells "no SFX here" as
+  // `sfx: none` reaches the engine as a cue literally NAMED "none", which then fails to
+  // resolve. The absence sentinels are part of the storyboard vocabulary, so drop them.
+  const SFX_NONE = new Set(["none", "no", "n/a", "na", "skip", "-", "—", "–"]);
   const lines = [];
   for (const f of manifest.frames) {
     const names = (f.extra?.sfx ?? "")
       .split(",")
       .map((s) => s.trim())
-      .filter(Boolean);
+      .filter((s) => s && !SFX_NONE.has(s.toLowerCase()));
     if (names.length && f.number != null) lines.push({ id: pad2(f.number), sfx: names });
   }
 
@@ -212,6 +221,15 @@ function runFetchSfx(argv) {
   const meta = toProductLaunchMeta(JSON.parse(readFileSync(neutral, "utf8")));
   writeFileSync(outPath, JSON.stringify(meta, null, 2));
   console.log(`✓ audio fetch-sfx: ${meta.sfx.length} SFX cue(s) → ${outPath}`);
+  // This pass rewrites audio_meta.json from the neutral sidecar. If a detached BGM generate is
+  // still running, the bed it eventually writes is NOT folded back in — the snapshot we just
+  // took has no music. Say so instead of leaving a silent film behind.
+  if (meta.bgm_pending && !meta.bgm) {
+    console.warn(
+      "⚠ audio fetch-sfx: a detached BGM generate is still pending, so this snapshot has no bed. " +
+        "Re-run `fetch-sfx` (or re-point audio_meta.json at the track) once it lands, before assembling.",
+    );
+  }
 }
 
 // ── sync-durations (local; rewrites STORYBOARD.md) ────────────────────────────