fix(capture,audio,docs): defects found running product-launch-video end to end

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

@@ -1,5 +1,5 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { isPrivateUrl, safeFetch } from "./assetDownloader.js";
+import { isPrivateUrl, safeFetch, toStandaloneSvg } from "./assetDownloader.js";
 
 describe("isPrivateUrl — SSRF denylist (security: F-003)", () => {
   it("blocks loopback, private, and metadata IPv4", () => {
@@ -91,3 +91,39 @@ describe("safeFetch — re-validates the denylist on every redirect hop (securit
     expect(fetchMock).not.toHaveBeenCalled();
   });
 });
+
+describe("toStandaloneSvg — scraped inline SVGs must survive as .svg files", () => {
+  it("adds the SVG namespace that outerHTML omits for inline SVG", () => {
+    const inline = '<svg viewBox="0 0 24 24"><path d="M0 0h24v24H0z"/></svg>';
+    const out = toStandaloneSvg(inline);
+    expect(out).toContain('xmlns="http://www.w3.org/2000/svg"');
+    // Nothing else may change — the path geometry is the brand mark.
+    expect(out).toContain('<path d="M0 0h24v24H0z"/>');
+    expect(out.endsWith("</svg>")).toBe(true);
+  });
+
+  it("leaves an SVG that already declares xmlns untouched", () => {
+    const already = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 8 8"><rect/></svg>';
+    expect(toStandaloneSvg(already)).toBe(already);
+  });
+
+  it("declares xmlns:xlink only when an xlink: attribute is actually used", () => {
+    const withXlink = '<svg viewBox="0 0 8 8"><use xlink:href="#a"/></svg>';
+    expect(toStandaloneSvg(withXlink)).toContain('xmlns:xlink="http://www.w3.org/1999/xlink"');
+    const without = '<svg viewBox="0 0 8 8"><use href="#a"/></svg>';
+    expect(toStandaloneSvg(without)).not.toContain("xmlns:xlink");
+  });
+
+  it("is idempotent and preserves attributes on the root", () => {
+    const inline = '<svg class="logo" width="120" height="24" fill="currentColor"><g/></svg>';
+    const once = toStandaloneSvg(inline);
+    expect(toStandaloneSvg(once)).toBe(once);
+    for (const attr of ['class="logo"', 'width="120"', 'height="24"', 'fill="currentColor"']) {
+      expect(once).toContain(attr);
+    }
+  });
+
+  it("returns non-SVG input unchanged rather than corrupting it", () => {
+    expect(toStandaloneSvg("<div>not an svg</div>")).toBe("<div>not an svg</div>");
+  });
+});

packages/cli/src/capture/assetDownloader.ts

@@ -17,6 +17,32 @@ function svgContentHashSlug(svgSource: string | Buffer, isLogo: boolean): string
   return isLogo ? `logo-${hash}` : `svg-${hash}`;
 }
 
+/**
+ * Make a scraped inline `<svg>` usable as a standalone `.svg` file.
+ *
+ * An inline SVG in an HTML document inherits the SVG namespace from the parser, so the DOM's
+ * `outerHTML` does not serialize `xmlns`. That string is fine pasted back into HTML but is NOT
+ * a valid standalone document: `<img src="logo-abc123.svg">` renders a broken-image icon, which
+ * is how these assets are actually consumed downstream. Declare the namespaces on the way to disk.
+ *
+ * `xlink:href` is deprecated but still emitted by plenty of sites; an undeclared `xlink:` prefix
+ * is a parse error in a standalone document, so declare that too — but only when it is used.
+ */
+export function toStandaloneSvg(outerHTML: string): string {
+  const open = outerHTML.match(/<svg\b[^>]*>/i);
+  if (!open) return outerHTML;
+  const original = open[0];
+  let tag = original;
+  const add: string[] = [];
+  if (!/\sxmlns\s*=/i.test(tag)) add.push('xmlns="http://www.w3.org/2000/svg"');
+  if (/\sxlink:[a-z-]+\s*=/i.test(outerHTML) && !/\sxmlns:xlink\s*=/i.test(tag)) {
+    add.push('xmlns:xlink="http://www.w3.org/1999/xlink"');
+  }
+  if (!add.length) return outerHTML;
+  tag = tag.replace(/^<svg\b/i, `<svg ${add.join(" ")}`);
+  return outerHTML.replace(original, tag);
+}
+
 export async function downloadAssets(
   tokens: DesignTokens,
   outputDir: string,
@@ -34,7 +60,9 @@ export async function downloadAssets(
   for (let i = 0; i < tokens.svgs.length && i < 30; i++) {
     const svg = tokens.svgs[i]!;
     if (!svg.outerHTML || svg.outerHTML.length < 50) continue;
-    const slug = svgContentHashSlug(svg.outerHTML, !!svg.isLogo);
+    // Hash the bytes that actually land on disk, so the filename still can't drift from content.
+    const svgFile = toStandaloneSvg(svg.outerHTML);
+    const slug = svgContentHashSlug(svgFile, !!svg.isLogo);
     let finalSlug = slug;
     let suffix = 2;
     while (usedSvgNames.has(finalSlug)) {
@@ -45,7 +73,7 @@ export async function downloadAssets(
     const name = `${finalSlug}.svg`;
     const localPath = `assets/svgs/${name}`;
     try {
-      writeFileSync(join(outputDir, localPath), svg.outerHTML, "utf-8");
+      writeFileSync(join(outputDir, localPath), svgFile, "utf-8");
       assets.push({ url: "", localPath, type: "svg" });
     } catch {
       /* skip */

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

@@ -0,0 +1,85 @@
+import { describe, expect, it, vi } from "vitest";
+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";
+
+// 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"));
+  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 out = await captureFullPagePlate(page, dir, 10962);
+
+    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");
+  });
+
+  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 });
+
+    await captureFullPagePlate(page, dir, 4000);
+
+    // 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 out = await captureFullPagePlate(page, dir, MAX_PLATE_HEIGHT_PX + 1);
+
+    expect(out).toBeNull();
+    expect(screenshot).not.toHaveBeenCalled();
+    expect(existsSync(join(dir, "full-page.png"))).toBe(false);
+  });
+
+  it("neutralises sticky/fixed chrome for the shot and restores it afterwards", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
+    const { page, evaluate, screenshot } = fakePage();
+
+    await captureFullPagePlate(page, dir, 8000);
+
+    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");
+    expect(scripts[1]).toContain("data-hf-plate-position");
+    expect(evaluate.mock.invocationCallOrder[0]).toBeLessThan(
+      screenshot.mock.invocationCallOrder[0]!,
+    );
+    expect(screenshot.mock.invocationCallOrder[0]).toBeLessThan(
+      evaluate.mock.invocationCallOrder[1]!,
+    );
+  });
+
+  it("restores the page even when the screenshot throws", async () => {
+    const dir = mkdtempSync(join(tmpdir(), "hf-plate-"));
+    const screenshot = vi.fn(async (_opts?: unknown) => {
+      throw new Error("capture failed");
+    });
+    const { page, evaluate } = fakePage({ screenshot });
+
+    await expect(captureFullPagePlate(page, dir, 8000)).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");
+  });
+});

packages/cli/src/capture/screenshotCapture.ts

@@ -21,6 +21,62 @@ import { join } from "node:path";
  * elements — screenshots show the page in its natural browsing state with
  * scroll-triggered animations fired.
  */
+/**
+ * Chrome caps a screenshot at 16384px per side (Skia's max texture dimension); past that the
+ * capture comes back clipped or fails outright. Long marketing pages do reach this.
+ */
+export const MAX_PLATE_HEIGHT_PX = 16384;
+
+/**
+ * One tall image of the whole document — the plate a scroll shot slides its viewport over.
+ *
+ * This is deliberately 1x. A 2x plate is what you'd want for pushing in without softening
+ * text, but doubling a long marketing page blows past `MAX_PLATE_HEIGHT_PX` exactly on the
+ * pages that most want a scroll shot; a frame that needs 2x should capture its own region
+ * instead. At 1x a 1920-wide plate is pixel-exact for a 1920x1080 viewport travelling down it.
+ *
+ * Two things have to be true for the plate to be usable, and both are why the earlier
+ * `full-page.png` was worth removing rather than keeping as-is:
+ *  · It must be taken AFTER the scroll traversal, so lazy images have loaded and
+ *    scroll-triggered reveals have fired. A plate shot on arrival is full of blank bands.
+ *  · Sticky/fixed chrome has to be neutralised first. `fullPage` bakes a fixed header in at
+ *    one position, so a nav ends up frozen across the middle of the plate. The viewport
+ *    shots keep sticky on purpose (natural browsing state); the plate cannot.
+ *
+ * Returns the relative path, or null when the page is too tall to capture in one piece.
+ */
+export async function captureFullPagePlate(
+  page: Page,
+  screenshotsDir: string,
+  scrollHeight: number,
+): Promise<string | null> {
+  if (scrollHeight > 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.
+  await page.evaluate(
+    `document.querySelectorAll('*').forEach((el) => {
+      const p = getComputedStyle(el).position;
+      if (p === 'fixed' || p === 'sticky') {
+        el.setAttribute('data-hf-plate-position', el.style.position || '');
+        el.style.position = 'static';
+      }
+    })`,
+  );
+  try {
+    const buffer = await page.screenshot({ type: "png", fullPage: true });
+    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');
+      })`,
+    );
+  }
+}
+
 export async function captureScrollScreenshots(page: Page, outputDir: string): Promise<string[]> {
   const screenshotsDir = join(outputDir, "screenshots");
   mkdirSync(screenshotsDir, { recursive: true });
@@ -151,7 +207,13 @@ export async function captureScrollScreenshots(page: Page, outputDir: string): P
     await page.evaluate(`window.scrollTo(0, 0)`);
     await new Promise((r) => setTimeout(r, 200));
 
-    // full-page.png removed — 1/8 agents read it, contact sheet covers the same content
+    // The scroll plate, last: everything above has loaded the page and fired its reveals, which
+    // is the only state a full-page shot is worth taking in. (An earlier full-page.png was
+    // 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);
+    if (plate) filePaths.push(plate);
   } catch {
     /* scroll screenshots are non-critical */
   }

skills-manifest.json

@@ -62,7 +62,7 @@
       "files": 29
     },
     "product-launch-video": {
-      "hash": "1a14737e16f6a154",
+      "hash": "ead12de8df2ed55d",
       "files": 26
     },
     "remotion-to-hyperframes": {

skills/product-launch-video/SKILL.md

@@ -54,7 +54,7 @@ Classify the input and choose the path. Explicit URL -> capture it and use the s
 
 Run capture with: `npx hyperframes capture "<URL>" -o ./capture`
 
-For a site tour or show-it-as-is brief, the captured page is the visual source of truth. Use the real screenshot instead of rebuilding the full website in HTML. If the shot needs internal movement, keep the screenshot as the base and overlay real captured assets at measured positions, or rebuild only the one component that moves. For a scroll shot, use a 2x full-page capture and animate the viewport over it. Recreate the whole page only when the user explicitly asks for a stylized interpretation or the capture is unusable.
+For a site tour or show-it-as-is brief, the captured page is the visual source of truth. Use the real screenshot instead of rebuilding the full website in HTML. If the shot needs internal movement, keep the screenshot as the base and overlay real captured assets at measured positions, or rebuild only the one component that moves. For a scroll shot, animate the viewport over `capture/screenshots/full-page.png` — the 1x plate of the whole document, pixel-exact for a 1920-wide viewport travelling down it. It is absent when the page was too tall to capture in one piece; fall back to the overlapping scroll-position shots in the same directory. Pushing in past 1:1 wants its own 2x capture of that region instead, since the plate has no headroom above 1x. Recreate the whole page only when the user explicitly asks for a stylized interpretation or the capture is unusable.
 
 If `GEMINI_API_KEY`, `GOOGLE_API_KEY`, or an OpenRouter key exists, capture auto-captions assets into `capture/extracted/asset-descriptions.md`. This is not a review gate. Without a vision key, use DOM context and continue.
 
@@ -128,7 +128,7 @@ Read `references/visual-design.md`, `../hyperframes-animation/blueprints-index.m
 
 For every visual frame, write a **time-coded shot sequence** into `STORYBOARD.md` per `visual-design.md`'s method: pick the frame's blueprint (or compose), instantiate it with THIS product's content, and pace each Scene's reveal to the voiceover so the frame develops across its full duration instead of front-loading then freezing. State layout and motion **inline** per Scene (vocabularies in `visual-design.md` and `motion-language.md`). Add one video-wide `## Video direction` block.
 
-When an element visibly continues across a frame boundary, give both workers the same numerical handoff in `STORYBOARD.md`: add `handoff_out:` to the outgoing frame and a matching `handoff_in:` to the incoming frame. Name the element and its exact x/y position, scale, opacity, and motion direction/speed at the cut. Omit these fields for a deliberate clean cut. The goal is simple: parallel workers must not invent two different versions of the same seam.
+When an element visibly continues across a frame boundary, give both workers the same numerical handoff in `STORYBOARD.md`: add `handoff_out:` to the outgoing frame and a matching `handoff_in:` to the incoming frame. Name the element and its exact x/y position, scale, opacity, and motion direction/speed at the cut — state every field even when it does not change, because a constant is `opacity: 1`, not an omission. Omit the whole block only for a deliberate clean cut. The goal is simple: parallel workers must not invent two different versions of the same seam.
 
 Do not change story, script, asset choices, `asset_candidates`, `transition_in`, or captured source material. Do not write HTML in this step.
 

skills/product-launch-video/scripts/audio.mjs

@@ -103,14 +103,19 @@ function toProductLaunchMeta(neutral) {
         duration_s: neutral.bgm.duration_s ?? null,
       }
     : null;
+  // bgm_pending must survive the neutral → PL translation. A detached generate (Lyria/MusicGen)
+  // leaves `bgm: null, bgm_pending: true` until the track lands; dropping the flag made
+  // "not ready yet" indistinguishable from "silent by design", so a later `fetch-sfx` snapshot
+  // turned a still-generating bed into no music at all with nothing to signal it.
+  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) ────────────────────────────────────────────────────
@@ -192,12 +197,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` used to reach the engine as a cue literally NAMED "none", which then failed
+  // 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 });
   }
 
@@ -211,6 +220,16 @@ 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 that the storyboard claims has a
+  // bed (observed live: the caller had to notice on its own and rebuild the entry).
+  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) ────────────────────────────