fix(core): render standalone sub-composition previews + surface captured video assets (cli) (#1631)

* fix(core): escape digit-leading id selectors in standalone sub-composition preview A CSS identifier cannot start with a digit, so an authored rule like `#01-wall-pushes-back { ... }` is an invalid selector and the browser drops the whole rule — taking the root's size/background with it. A full composition masks this (the host stretches/paints the frame), but a standalone preview has no host, so the root collapses to height:0 + transparent and renders blank. extractFullDocumentParts now rewrites `#<digit-leading-id>` selectors to their escaped valid form (`#\30 1-...`, still matching the element id), scoped to ids actually present and matched only as `#id` not followed by an ident char so hex colors are never touched. Also harden the <template> inner-HTML extraction to use the DOM instead of a greedy regex. Tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e43b377fc0) * feat(cli): surface captured video clips in asset descriptions generateAssetDescriptions now reads extracted/video-manifest.json and emits each downloaded clip first, tagged [video], with its DOM heading/caption and dimensions — motion clips are usually the strongest hero material and downstream planners key off the [video] marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(studio): import commitGsapPositionFromDrag from its actual module The function was split out into gsapDragPositionCommit.ts in #1605, but the test kept importing it from ./gsapDragCommit, which no longer exports it — yielding `is not a function` at runtime. Import from the correct module to match the production import in gsapRuntimeBridge.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(core): address review nits on standalone sub-composition preview Review follow-ups (#1631), all non-blocking polish: - contentExtractor: use path.basename() instead of localPath.split('/').pop() so video filenames resolve correctly on Windows-style paths too. - subComposition: document that only the leading digit needs CSS escaping (CSS Syntax L3 §4.3.11) on escapeLeadingDigitIdent. - tests: pin three previously-uncovered paths — multiple digit-leading ids in one composition, a digit-leading id inside compound/combinator selectors, and the promoteTemplateCompositionId no-op when the <template> has no id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-09-01 19:42:03 +00:00 · 2026-06-22 22:49:54 +08:00
parent 1967901b57
commit 0e75eb2510
3 changed files with 373 additions and 7 deletions
@@ -10,7 +10,7 @@

 import type { Page } from "puppeteer-core";
 import { existsSync, readdirSync, statSync, readFileSync } from "node:fs";
-import { join } from "node:path";
+import { basename, join } from "node:path";
 import type sharpType from "sharp";
 import type { CatalogedAsset } from "./assetCataloger.js";
 import type { DesignTokens } from "./types.js";
@@ -494,5 +494,36 @@ export function generateAssetDescriptions(
    /* no fonts dir */
  }

-  return [...captionedLines, ...uncaptionedLines, ...svgLines, ...fontLines];
+  // Describe videos — high-value motion clips. The video-manifest.json (written
+  // earlier by captureVideoManifest) carries each clip's DOM heading/caption +
+  // dims. Surfaced FIRST and tagged `[video]`: for a product/demo these moving
+  // clips are usually the strongest hero material, and downstream planners key off
+  // the `[video]` marker. (The `videos/` dir is skipped in the image walk above —
+  // its entries come from the manifest, which has the captions the bare files lack.)
+  const videoLines: string[] = [];
+  try {
+    const manifest = JSON.parse(
+      readFileSync(join(outputDir, "extracted", "video-manifest.json"), "utf-8"),
+    ) as Array<{
+      filename?: string;
+      localPath?: string;
+      caption?: string;
+      heading?: string;
+      width?: number;
+      height?: number;
+    }>;
+    for (const v of manifest) {
+      if (!v.localPath) continue; // only describe clips that actually downloaded
+      const base = basename(v.localPath) || v.filename || "";
+      if (!base) continue;
+      const desc =
+        (v.caption || v.heading || "").trim().replace(/\s+/g, " ").slice(0, 140) || "motion clip";
+      const dims = v.width && v.height ? `, ~${v.width}×${v.height}` : "";
+      videoLines.push(`${base} — [video] ${desc}${dims}`);
+    }
+  } catch {
+    /* no video manifest */
+  }
+
+  return [...videoLines, ...captionedLines, ...uncaptionedLines, ...svgLines, ...fontLines];
 }
@@ -134,4 +134,229 @@ describe("buildSubCompositionHtml", () => {
    expect(html).not.toContain("url('../poster.png')");
    expect(html).not.toContain('url("../fonts/brand.woff2")');
  });
+
+  it("promotes the <template>'s data-composition-id onto the root element when the content lacks one", () => {
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/02-music.html": `<template data-composition-id="02-music">
+  <style>#music-scene { background: #121212; }</style>
+  <div id="music-scene" class="clip" data-start="0" data-duration="3.213" data-track-index="0">
+    <div id="music-headline">ready when you are.</div>
+  </div>
+  <script>
+    window.__timelines = window.__timelines || {};
+    window.__timelines["02-music"] = gsap.timeline({ paused: true });
+  </script>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/02-music.html",
+      "/api/runtime.js",
+    );
+
+    // Root rendered element gains the id so the runtime can bind the timeline
+    // (attribute order is serializer-dependent, so match the tag as a whole).
+    expect(html).toMatch(
+      /<div\b(?=[^>]*\sid="music-scene")(?=[^>]*\sdata-composition-id="02-music")[^>]*>/,
+    );
+    // The <style>/<script> siblings must NOT receive it.
+    expect(html).not.toMatch(/<style[^>]*data-composition-id/i);
+    expect(html).not.toMatch(/<script[^>]*data-composition-id/i);
+  });
+
+  it("extracts the real <template> even when a head comment mentions the literal text <template>", () => {
+    // Regression: a greedy /<template>([\s\S]*)<\/template>/ regex latches onto
+    // the "<template>" inside the head comment, mis-slicing the capture so the
+    // real content stays wrapped in an inert <template> in the output — leaving
+    // the preview with no [data-composition-id] element and rendering blank.
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/03-force-pair.html": `<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <!-- head is metadata only; the HF runtime clones ONLY <template> contents -->
+  </head>
+  <body>
+    <template>
+      <style>#fp-root { width: 1920px; height: 1080px; }</style>
+      <div id="fp-root" data-composition-id="03-force-pair" data-width="1920" data-height="1080">
+        <div class="headline">forces come in pairs</div>
+      </div>
+      <script>
+        window.__timelines = window.__timelines || {};
+        window.__timelines["03-force-pair"] = gsap.timeline({ paused: true });
+      </script>
+    </template>
+  </body>
+</html>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/03-force-pair.html",
+      "/api/runtime.js",
+      "/api/projects/demo/preview/",
+    );
+
+    expect(html).not.toBeNull();
+    // The real composition content is rendered into <body> directly...
+    expect(html).toContain('data-composition-id="03-force-pair"');
+    expect(html).toContain("forces come in pairs");
+    expect(html).toContain('__timelines["03-force-pair"]');
+    // ...and is NOT re-wrapped in an inert <template> (which the browser would
+    // never render). The comment fragment must not leak through either.
+    const bodyStart = html!.indexOf("<body>");
+    const body = html!.slice(bodyStart);
+    expect(body).not.toContain("<template");
+    expect(body).not.toContain("clones ONLY");
+  });
+
+  it("escapes #id selectors whose id starts with a digit so the rule is not dropped", () => {
+    // A CSS ident can't start with a digit, so `#01-wall { ... }` is an invalid
+    // selector and the browser drops the whole rule — the root loses its size and
+    // background and a standalone preview renders blank. Escape it to a valid form.
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/01-wall.html": `<template>
+  <div id="01-wall" data-composition-id="01-wall" data-start="0" data-duration="5.5" data-track-index="0">
+    <style>
+      #01-wall { position: absolute; inset: 0; width: 1920px; height: 1080px; background-color: #F0EBDE; }
+      .arrow { background: #1F2BE0; color: #1A2B3C; }
+    </style>
+    <div class="arrow">action</div>
+  </div>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/01-wall.html",
+      "/api/runtime.js",
+    );
+
+    expect(html).not.toBeNull();
+    // The invalid `#01-wall` selector is rewritten to its escaped, valid form.
+    expect(html).toContain("#\\30 1-wall {");
+    expect(html).not.toMatch(/#01-wall\s*\{/);
+    // Digit-leading hex color VALUES (not element ids) must be left untouched.
+    expect(html).toContain("background: #1F2BE0;");
+    expect(html).toContain("color: #1A2B3C;");
+    // The element's id attribute itself is unchanged (only the selector is escaped).
+    expect(html).toContain('id="01-wall"');
+  });
+
+  it("does not add a duplicate data-composition-id when the root element already has one", () => {
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/04-allinone.html": `<template data-composition-id="04-allinone">
+  <div id="c04" data-composition-id="04-allinone" data-start="0" data-duration="5.695" data-track-index="0">
+    <div class="headline">all in one app.</div>
+  </div>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/04-allinone.html",
+      "/api/runtime.js",
+    );
+    const occurrences = html?.match(/data-composition-id="04-allinone"/g) ?? [];
+    expect(occurrences).toHaveLength(1);
+  });
+
+  it("escapes every digit-leading #id selector when several appear in one composition", () => {
+    // The fix iterates over all digit-leading ids present on elements; pin that
+    // a second id in the same document is escaped too, not just the first.
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/01-wall.html": `<template>
+  <div id="01-wall" data-composition-id="01-wall" data-start="0" data-duration="4" data-track-index="0">
+    <style>
+      #01-wall { width: 1920px; height: 1080px; }
+      #02-music { color: #1F2BE0; }
+    </style>
+    <div id="02-music">two</div>
+  </div>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/01-wall.html",
+      "/api/runtime.js",
+    );
+
+    expect(html).not.toBeNull();
+    expect(html).toContain("#\\30 1-wall {");
+    expect(html).toContain("#\\30 2-music {");
+    expect(html).not.toMatch(/#01-wall\s*\{/);
+    expect(html).not.toMatch(/#02-music\s*\{/);
+    // The hex color value is still left untouched.
+    expect(html).toContain("color: #1F2BE0;");
+  });
+
+  it("escapes a digit-leading #id inside compound and combinator selectors", () => {
+    // Only the leading-digit id token is escaped; the descendant/combinator and
+    // pseudo-class tail must be preserved verbatim.
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/01-wall.html": `<template>
+  <div id="01-wall" data-composition-id="01-wall" data-start="0" data-duration="4" data-track-index="0">
+    <style>
+      #01-wall .child > span { color: red; }
+      #01-wall:hover { opacity: 1; }
+    </style>
+    <div class="child"><span>x</span></div>
+  </div>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/01-wall.html",
+      "/api/runtime.js",
+    );
+
+    expect(html).not.toBeNull();
+    expect(html).toContain("#\\30 1-wall .child > span {");
+    expect(html).toContain("#\\30 1-wall:hover {");
+    // No unescaped form survives.
+    expect(html).not.toContain("#01-wall .child");
+    expect(html).not.toContain("#01-wall:hover");
+  });
+
+  it("leaves the DOM untouched when the <template> has no data-composition-id to promote", () => {
+    // No data-composition-id on the <template> tag → promoteTemplateCompositionId
+    // returns early. The content's own id must be the only one, with nothing
+    // injected onto the root or its siblings.
+    const dir = makeTempProject({
+      "index.html": `<!doctype html><html><head></head><body></body></html>`,
+      "compositions/frames/05-plain.html": `<template>
+  <style>#c05 { width: 1920px; height: 1080px; }</style>
+  <div id="c05" data-composition-id="05-plain" data-start="0" data-duration="3" data-track-index="0">
+    <div class="headline">plain</div>
+  </div>
+</template>`,
+    });
+
+    const html = buildSubCompositionHtml(
+      dir,
+      "compositions/frames/05-plain.html",
+      "/api/runtime.js",
+    );
+
+    expect(html).not.toBeNull();
+    // The authored id is preserved and is the ONLY data-composition-id — promote
+    // didn't fabricate one (no <template> id to copy) or duplicate it.
+    const occ = html?.match(/data-composition-id="05-plain"/g) ?? [];
+    expect(occ).toHaveLength(1);
+    expect(html).toMatch(
+      /<div\b(?=[^>]*\sid="c05")(?=[^>]*\sdata-composition-id="05-plain")[^>]*>/,
+    );
+    // The <style> sibling was not tagged.
+    expect(html).not.toMatch(/<style[^>]*data-composition-id/i);
+  });
 });
@@ -40,6 +40,59 @@ function rewriteRelativePaths(root: ParentNode, compPath: string): void {
  }
 }

+/**
+ * Escape a CSS identifier whose first character is a digit so it is a valid
+ * selector. A CSS ident cannot start with a digit, so it must be written as an
+ * escaped code point: `01-foo` → `\30 1-foo` (leading `0` → `\30 `, rest kept).
+ *
+ * Only the leading digit needs escaping (per CSS Syntax Level 3 §4.3.11): once
+ * the parser consumes the `\<hex> ` escape, the rest of the ident continues
+ * normally, so `123-scene` → `\31 23-scene` is valid (the `23-scene` tail is
+ * consumed as identifier continuation). The trailing space terminates the hex
+ * escape so a following hex digit isn't folded into the code point.
+ */
+function escapeLeadingDigitIdent(id: string): string {
+  return `\\${id.charCodeAt(0).toString(16)} ${id.slice(1)}`;
+}
+
+const REGEXP_SPECIALS = /[.*+?^${}()|[\]\\]/g;
+
+/**
+ * Fix `#<digit-leading-id>` selectors in the tree's `<style>` blocks.
+ *
+ * CSS identifiers cannot start with a digit, so an authored rule like
+ * `#01-wall-pushes-back { width: 1920px; height: 1080px; background: #F0EBDE }`
+ * is an invalid selector and the browser silently drops the WHOLE rule — taking
+ * the root's size and background with it. In a full composition the frame is
+ * stretched/painted by its `data-composition-src` host so the collapse is
+ * masked, but a standalone preview has no host: the root falls back to
+ * `height: 0` + transparent and the frame renders blank (black).
+ *
+ * Rewrite each such selector to its escaped, valid form (`#\30 1-wall-pushes-back`,
+ * which still matches `id="01-wall-pushes-back"`) so the rule applies and the
+ * whole declaration block — size, background, position, container-type — comes
+ * back. Scoped to ids that are actually present on elements in the content and
+ * matched only as `#id` not followed by another ident char, so hex colors
+ * (`#1F2BE0`) and other values are never touched (they are not element ids).
+ */
+function fixDigitLeadingIdSelectors(root: ParentNode): void {
+  const digitIds = new Set<string>();
+  for (const el of root.querySelectorAll("[id]")) {
+    const id = el.getAttribute("id");
+    if (id && /^\d/.test(id)) digitIds.add(id);
+  }
+  if (digitIds.size === 0) return;
+
+  for (const styleEl of root.querySelectorAll("style")) {
+    let css = styleEl.textContent || "";
+    for (const id of digitIds) {
+      const pattern = new RegExp(`#${id.replace(REGEXP_SPECIALS, "\\$&")}(?![\\w-])`, "g");
+      css = css.replace(pattern, `#${escapeLeadingDigitIdent(id)}`);
+    }
+    styleEl.textContent = css;
+  }
+}
+
 /**
 * Parse a full HTML document and extract its head elements and body
 * content separately, so they can be reassembled into a clean standalone
@@ -69,6 +122,9 @@ function extractFullDocumentParts(
  for (const target of rewriteTargets) {
    rewriteRelativePaths(target, compPath);
  }
+  // Run on the whole document: ids live in <body> but their rules may live in
+  // a <head> <style>, so the scope must span both.
+  fixDigitLeadingIdSelectors(doc);

  const headContent = doc.head?.innerHTML ?? "";
  const bodyContent = doc.body?.innerHTML ?? "";
@@ -80,6 +136,26 @@ function extractFullDocumentParts(
  return { headContent, bodyContent, htmlAttrs, bodyAttrs };
 }

+/**
+ * Extract the inner HTML of the composition's wrapping `<template>` element, or
+ * `null` if the source has no `<template>`.
+ *
+ * Located via the DOM rather than a regex. A greedy
+ * `/<template[^>]*>([\s\S]*)<\/template>/` can latch onto a literal
+ * `"<template>"` that appears inside an HTML comment — e.g. a head note such as
+ * "the HF runtime clones ONLY <template> contents" — and mis-slice the capture,
+ * leaving the real composition content re-wrapped in an inert `<template>` in
+ * the output. That template is never rendered by the browser, so the standalone
+ * preview has no `[data-composition-id]` element and no registered timeline, and
+ * renders blank. `querySelector("template")` only ever matches a real element
+ * node, so comment text can't fool it.
+ */
+function extractTemplateInnerHtml(rawComp: string): string | null {
+  const { document: doc } = parseHTML(rawComp);
+  const template = doc.querySelector("template");
+  return template ? template.innerHTML : null;
+}
+
 function extractElementAttrs(el: Element): string {
  const parts: string[] = [];
  for (let i = 0; i < el.attributes.length; i++) {
@@ -93,6 +169,38 @@ function extractElementAttrs(el: Element): string {
  return parts.join(" ");
 }

+const NON_RENDERED_TAGS = new Set(["SCRIPT", "STYLE", "LINK", "META", "TEMPLATE", "NOSCRIPT"]);
+
+/**
+ * Carry the `<template>`'s `data-composition-id` onto the content's root
+ * rendered element when the author declared it only on the `<template>` tag.
+ *
+ * In a full composition, each sub-composition is mounted under a wrapper
+ * element (the `data-composition-src` host) that carries the composition id,
+ * which is how the runtime binds `window.__timelines[id]` into the player's
+ * master timeline. A standalone preview has no such wrapper, so it relies on
+ * the frame's own root element carrying `data-composition-id`. If the id lives
+ * only on the inert `<template>` tag (a common authoring pattern), the rendered
+ * body has no `[data-composition-id]` element — the runtime then never selects
+ * a root composition, the registered GSAP timeline stays unbound, and seeking
+ * does nothing. The frame renders at its pre-animation state (GSAP `fromTo`
+ * pins `opacity:0`), producing a blank preview/thumbnail.
+ *
+ * This is a no-op when the content already exposes a `[data-composition-id]`
+ * element (e.g. the id is authored on the root div), so compositions that
+ * already render correctly are untouched.
+ */
+function promoteTemplateCompositionId(rawComp: string, body: Element): void {
+  const templateCompositionId = rawComp.match(
+    /<template[^>]*\sdata-composition-id\s*=\s*["']([^"']+)["']/i,
+  )?.[1];
+  if (!templateCompositionId) return;
+  if (body.querySelector("[data-composition-id]")) return;
+
+  const root = Array.from(body.children).find((el) => !NON_RENDERED_TAGS.has(el.tagName));
+  root?.setAttribute("data-composition-id", templateCompositionId);
+}
+
 /**
 * Add `data-composition-file="<compPath>"` to the comp's root composition
 * element (the first `[data-composition-id]` that lacks the attribute), so the
@@ -144,15 +252,16 @@ export function buildSubCompositionHtml(
  let htmlAttrs = "";
  let bodyAttrs = "";

-  const templateMatch = rawComp.match(/<template[^>]*>([\s\S]*)<\/template>/i);
+  const templateInner = extractTemplateInnerHtml(rawComp);

-  if (templateMatch) {
-    const content = templateMatch[1];
+  if (templateInner != null) {
    const { document: contentDoc } = parseHTML(
-      `<!DOCTYPE html><html><head></head><body>${content}</body></html>`,
+      `<!DOCTYPE html><html><head></head><body>${templateInner}</body></html>`,
    );
    rewriteRelativePaths(contentDoc, compPath);
-    rewrittenContent = contentDoc.body.innerHTML || content!;
+    fixDigitLeadingIdSelectors(contentDoc);
+    promoteTemplateCompositionId(rawComp, contentDoc.body);
+    rewrittenContent = contentDoc.body.innerHTML || templateInner;
  } else if (isFullHtmlDocument(rawComp)) {
    const parts = extractFullDocumentParts(rawComp, compPath);
    compHeadContent = parts.headContent;
@@ -164,6 +273,7 @@ export function buildSubCompositionHtml(
      `<!DOCTYPE html><html><head></head><body>${rawComp}</body></html>`,
    );
    rewriteRelativePaths(contentDoc, compPath);
+    fixDigitLeadingIdSelectors(contentDoc);
    rewrittenContent = contentDoc.body.innerHTML || rawComp;
  }