fix: storyboard-angle review follow-ups (M1 bg-on-clip, B3 slideshow, parser guard, CLI fixes) (#1791)

* fix(skills): storyboard review — bg-on-clip rule, slideshow output, parser parity guard

Addresses the storyboard-angle review (jrusso1020):

- M1 (invisible text): frame-worker.md (x3) + SKILL.md Step 5 (x3) now require a
  frame's full-bleed background on a class=clip layer, never the #root /
  data-composition-id element (the root is clip-gated to its scene window, so a
  background on it is not a dependable ground and dark text can land on the black
  host body). The assembler already paints frame.md's canvas onto index #root as
  the base ground; the per-frame clip rides on top.
- B3 (slideshow truncates to slide 1): slideshow/SKILL.md gains an Output section
  (decks render via 'present'; 'render index.html' captures only the first
  composition; linear main-line MP4 export is deferred).
- Parser drift: vendoredParity.test.ts guards the three vendored storyboard.mjs
  copies (byte-identical + parse-parity with @hyperframes/core).
- skills-manifest.json regenerated for the edited SKILL.md files.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(cli): storyboard review — lint, validate help, snapshot, inspect, capture, render

Addresses the CLI findings from the storyboard-angle review (jrusso1020):

- lint (@hyperframes/lint): accept vendor-prefixed system-font keywords
  -apple-system / BlinkMacSystemFont so a system stack with a generic fallback no
  longer trips font_family_without_font_face (+ test).
- help: list 'validate' under Project in 'hyperframes --help' (was runnable but
  undocumented).
- snapshot: honor -o/--output (the flag did not exist; output was hardcoded to
  snapshots/). The dir is resolved once and threaded through capture + contact
  sheet + Gemini.
- snapshot: split font status into loaded / error / unused with a one-line
  summary; only a real 'error' is reported as FAILED (an unrequested @font-face
  is 'unused', not a contradiction with 'loaded').
- inspect: suppress text_occluded across a scene-to-scene crossfade (occluder in
  a different data-composition-id mount while a scene is mid-fade); a same-scene
  or two-settled-scenes overlap still flags.
- inspect: suppress content_overlap between in-flow siblings governed by the same
  flex/grid container (tight stacks / number lockups are layout slop).
- capture: record source resolution (videoWidth/Height) in video-manifest.json
  alongside the DOM display box; consumers size off the source dims.
- render: warn when the target carries a slideshow island (render captures only
  the first scene, so the MP4 is truncated to slide 1; use 'present').

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
WaterrrForever
2026-06-30 16:58:00 +08:00
committed by GitHub
co-authored by Claude Opus 4.8
parent a4eacaec37
commit a4303137cb
18 changed files with 251 additions and 31 deletions
+5 -1
View File
@@ -511,6 +511,8 @@ export function generateAssetDescriptions(
heading?: string;
width?: number;
height?: number;
sourceWidth?: number;
sourceHeight?: number;
}>;
for (const v of manifest) {
if (!v.localPath) continue; // only describe clips that actually downloaded
@@ -518,7 +520,9 @@ export function generateAssetDescriptions(
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}` : "";
const dimW = v.sourceWidth || v.width;
const dimH = v.sourceHeight || v.height;
const dims = dimW && dimH ? `, ~${dimW}×${dimH}` : "";
videoLines.push(`${base} — [video] ${desc}${dims}`);
}
} catch {
+14
View File
@@ -277,6 +277,8 @@ interface VideoDescriptor {
src: string;
width: number;
height: number;
sourceWidth: number;
sourceHeight: number;
top: number;
left: number;
heading: string;
@@ -315,8 +317,14 @@ const VIDEO_SCAN_EXPR = `(() => {
if (!ariaLabel && wrapper) ariaLabel = wrapper.getAttribute('aria-label') || '';
return {
src: src,
// width/height are the DOM display box (what the page laid the element out
// at); sourceWidth/Height are the clip's intrinsic resolution. Size planners
// off the source dims, not the display box (a 1920x1080 clip can display at
// 904x613). 0 when metadata has not loaded yet.
width: Math.round(rect.width),
height: Math.round(rect.height),
sourceWidth: v.videoWidth || 0,
sourceHeight: v.videoHeight || 0,
top: Math.round(rect.top),
left: Math.round(rect.left),
heading: heading,
@@ -418,6 +426,8 @@ export async function captureVideoManifest(
filename: k,
width: 0,
height: 0,
sourceWidth: 0,
sourceHeight: 0,
top: 0,
left: 0,
heading: "",
@@ -441,6 +451,8 @@ export async function captureVideoManifest(
filename: string;
width: number;
height: number;
sourceWidth: number;
sourceHeight: number;
heading: string;
caption: string;
ariaLabel: string;
@@ -508,6 +520,8 @@ export async function captureVideoManifest(
filename: v.filename,
width: v.width,
height: v.height,
sourceWidth: v.sourceWidth,
sourceHeight: v.sourceHeight,
heading: v.heading,
caption: v.caption,
ariaLabel: v.ariaLabel,
+8 -3
View File
@@ -104,6 +104,11 @@ export interface ManifestEntry {
filename: string;
width: number;
height: number;
/** Intrinsic clip resolution (videoWidth/Height). Optional — absent in
* manifests written before source dims were recorded. Size off these, not
* the display box (width/height). */
sourceWidth?: number;
sourceHeight?: number;
heading: string;
caption: string;
ariaLabel: string;
@@ -213,7 +218,7 @@ export async function runVideoMode(args: VideoModeArgs): Promise<void> {
);
for (const e of manifest) {
console.log(
` ${c.bold(`[${e.index}]`)} ${e.filename}${e.width}×${e.height}` +
` ${c.bold(`[${e.index}]`)} ${e.filename}${e.sourceWidth || e.width}×${e.sourceHeight || e.height}` +
(e.heading ? `\n heading: "${e.heading}"` : "") +
`\n url: ${e.url}`,
);
@@ -253,7 +258,7 @@ export async function runVideoMode(args: VideoModeArgs): Promise<void> {
const relPath = isW2hLayout ? `capture/assets/videos/${fname}` : `assets/videos/${fname}`;
console.log(
`${c.accent("▸")} downloading [${entry.index}] ${entry.filename} (${entry.width}×${entry.height})`,
`${c.accent("▸")} downloading [${entry.index}] ${entry.filename} (${entry.sourceWidth || entry.width}×${entry.sourceHeight || entry.height})`,
);
console.log(` from: ${entry.url}`);
try {
@@ -264,7 +269,7 @@ export async function runVideoMode(args: VideoModeArgs): Promise<void> {
const snippetId = `video-${entry.index}`;
console.log(
` Reference it from a beat composition as:\n` +
` <video id="${snippetId}" src="${relPath}" data-start="0" data-duration="${entry.width === entry.height ? 5 : 4}" data-track-index="0" autoplay muted loop></video>`,
` <video id="${snippetId}" src="${relPath}" data-start="0" data-duration="${(entry.sourceWidth || entry.width) === (entry.sourceHeight || entry.height) ? 5 : 4}" data-track-index="0" autoplay muted loop></video>`,
);
} catch (e) {
if ((e as NodeJS.ErrnoException).code === "EEXIST") {
@@ -473,11 +473,37 @@
return a.contains(b) || b.contains(a);
}
function isInFlow(element) {
const position = getComputedStyle(element).position;
return position === "static" || position === "relative" || position === "sticky";
}
function nearestFlexGridAncestor(element) {
for (let parent = element.parentElement; parent; parent = parent.parentElement) {
const display = getComputedStyle(parent).display;
if (display.includes("flex") || display.includes("grid")) return parent;
}
return null;
}
// Two in-flow text blocks governed by the same flex/grid container are placed
// by the layout engine, which reserves space for each — they cannot visually
// collide. Any measured text-rect overlap between them is line-box / leading
// slop (tight stacks, number lockups, super/subscript units), not a collision.
// A real overlap bug needs free positioning (absolute/fixed), which keeps a
// different formatting context and is still flagged.
function isManagedFlowOverlap(a, b) {
if (!isInFlow(a) || !isInFlow(b)) return false;
const container = nearestFlexGridAncestor(a);
return !!container && container === nearestFlexGridAncestor(b);
}
// Two solid text blocks whose boxes overlap by more than a fifth of the
// smaller block read as a collision — unreadable, and invisible to the
// overflow checks, which only compare an element against its container.
function overlapIssue(a, b, time) {
if (isNested(a.element, b.element)) return null;
if (isManagedFlowOverlap(a.element, b.element)) return null;
const area = intersectionArea(a.rect, b.rect);
if (area <= Math.min(rectArea(a.rect), rectArea(b.rect)) * 0.2) return null;
return {
@@ -537,13 +563,30 @@
return !!hit && hit !== element && !element.contains(hit) && !hit.contains(element);
}
// During a scene-to-scene crossfade the incoming scene paints over the
// outgoing scene's still-visible text at >= 0.6 opacity — and `--at-transitions`
// samples exactly that midpoint. That overlap is the transition doing its job,
// not an occlusion bug. Detect it: the occluder lives in a DIFFERENT composition
// mount ([data-composition-id]) than the text, and at least one of the two scenes
// is mid-fade (effective opacity < 1). Two fully-settled scenes overlapping
// (both opacity 1) is NOT suppressed — that is a real layering bug.
function isCrossSceneTransitionOverlap(textEl, occluder) {
const textScene = textEl.closest("[data-composition-id]");
const occluderScene = occluder.closest("[data-composition-id]");
if (!textScene || !occluderScene || textScene === occluderScene) return false;
return Math.min(opacityChain(textScene), opacityChain(occluderScene)) < 0.999;
}
// The opaque element painted over (x, y), or null when the topmost element
// there is related to the text or non-opaque.
// there is related to the text, non-opaque, or a transient crossfade overlap.
// fallow-ignore-next-line complexity
function occluderAt(element, x, y) {
if (typeof document.elementFromPoint !== "function") return null;
const hit = document.elementFromPoint(x, y);
if (!isForeignElement(element, hit)) return null;
return isOpaqueOccluder(hit) ? hit : null;
if (!isOpaqueOccluder(hit)) return null;
if (isCrossSceneTransitionOverlap(element, hit)) return null;
return hit;
}
// Sweep a grid across the text box (three rows, not just the mid-line, so
+24
View File
@@ -661,6 +661,30 @@ export default defineCommand({
}
}
// ── Slideshow guard ───────────────────────────────────────────────────
// A slideshow deck is several top-level scene compositions with no master
// root. `render` captures only the FIRST composition, so a deck renders as a
// silently truncated MP4 (e.g. slide 1 of a 40s deck). Warn and point at the
// deck-native path. Best-effort — never block a render on this probe.
if (!quiet) {
try {
const renderTarget = entryFile ? resolve(project.dir, entryFile) : project.indexPath;
const { slideshowIslandRegex } = await import("@hyperframes/core/slideshow");
if (slideshowIslandRegex("i").test(readFileSync(renderTarget, "utf8"))) {
console.log(
c.warn("⚠") +
" This composition carries a slideshow island — `render` captures only the first" +
" scene, so the MP4 will be truncated to slide 1. Use " +
c.accent("hyperframes present") +
" for the deck; a linear main-line MP4 export is not yet available.",
);
console.log("");
}
} catch {
/* best-effort — a missing/unreadable target surfaces later in the real flow */
}
}
// ── Print render plan ─────────────────────────────────────────────────
if (!quiet && !batchPath) {
const workerLabel =
+45 -21
View File
@@ -3,7 +3,7 @@ import { spawn } from "node:child_process";
import { defineCommand } from "citty";
import { existsSync, mkdtempSync, readFileSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { resolve, join, relative, isAbsolute } from "node:path";
import { resolve, join, relative, isAbsolute, basename } from "node:path";
import { resolveProject } from "../utils/project.js";
import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport.js";
import { serveStaticProjectHtml } from "../utils/staticProjectServer.js";
@@ -94,7 +94,7 @@ export const examples: Example[] = [
*/
async function captureSnapshots(
projectDir: string,
opts: { frames?: number; timeout?: number; at?: number[] },
opts: { frames?: number; timeout?: number; at?: number[]; outputDir?: string },
): Promise<string[]> {
const { bundleToSingleHtml } = await import("@hyperframes/core/compiler");
const { ensureBrowser } = await import("../browser/manager.js");
@@ -176,26 +176,37 @@ async function captureSnapshots(
// Extra settle time for media and animations to initialize
await new Promise((r) => setTimeout(r, 1500));
// Font verification — report which fonts loaded vs fell back
// Font verification — split into loaded / errored / unused. Only status
// "error" is a real failure; a face still "unloaded"/"loading" after
// document.fonts.ready + the settle wait was simply never requested by any
// rendered text (an unused @font-face), so it is reported as "unused", not
// FAILED — printing it as FAILED alongside "loaded" read as a contradiction.
const fontReport = await page
.evaluate(() => {
const loaded: string[] = [];
const failed: string[] = [];
const errored: string[] = [];
const unused: string[] = [];
(document as any).fonts.forEach((f: any) => {
const entry = `${f.family} (${f.weight} ${f.style})`;
if (f.status === "loaded") loaded.push(entry);
else failed.push(entry + ` [${f.status}]`);
else if (f.status === "error") errored.push(entry);
else unused.push(entry);
});
return { loaded, failed };
return { loaded, errored, unused };
})
.catch(() => ({ loaded: [] as string[], failed: [] as string[] }));
.catch(() => ({ loaded: [] as string[], errored: [] as string[], unused: [] as string[] }));
if (fontReport.loaded.length > 0 || fontReport.failed.length > 0) {
console.log(
`\n ${c.dim("Fonts loaded:")} ${fontReport.loaded.length > 0 ? fontReport.loaded.join(", ") : "none"}`,
);
if (fontReport.failed.length > 0) {
console.log(` ${c.error("Fonts FAILED:")} ${fontReport.failed.join(", ")}`);
if (
fontReport.loaded.length > 0 ||
fontReport.errored.length > 0 ||
fontReport.unused.length > 0
) {
const parts = [`${fontReport.loaded.length} loaded`];
if (fontReport.errored.length > 0) parts.push(`${fontReport.errored.length} failed`);
if (fontReport.unused.length > 0) parts.push(`${fontReport.unused.length} unused`);
console.log(`\n ${c.dim("Fonts:")} ${parts.join(", ")}`);
if (fontReport.errored.length > 0) {
console.log(` ${c.error("Fonts FAILED:")} ${fontReport.errored.join(", ")}`);
}
}
@@ -221,7 +232,7 @@ async function captureSnapshots(
? [duration / 2]
: Array.from({ length: numFrames }, (_, i) => (i / (numFrames - 1)) * duration);
const snapshotDir = join(projectDir, "snapshots");
const snapshotDir = opts.outputDir ?? join(projectDir, "snapshots");
mkdirSync(snapshotDir, { recursive: true });
try {
const { readdirSync } = await import("node:fs");
@@ -387,7 +398,8 @@ async function captureSnapshots(
const framePath = join(snapshotDir, filename);
await page.screenshot({ path: framePath, type: "png" });
savedPaths.push(`snapshots/${filename}`);
const rel = relative(projectDir, framePath);
savedPaths.push(rel.startsWith("..") || isAbsolute(rel) ? framePath : rel);
}
} finally {
await chromeBrowser.close();
@@ -410,6 +422,11 @@ export default defineCommand({
description: "Project directory",
required: false,
},
output: {
type: "string",
alias: "o",
description: "Directory to write snapshots into (default: <project>/snapshots)",
},
frames: {
type: "string",
description: "Number of evenly-spaced frames to capture (default: 5)",
@@ -457,7 +474,15 @@ export default defineCommand({
console.log(`${c.accent("◆")} Capturing ${label} from ${c.accent(project.name)}`);
try {
const paths = await captureSnapshots(project.dir, { frames, timeout, at: atTimestamps });
const snapshotDir = args.output
? resolve(String(args.output))
: join(project.dir, "snapshots");
const paths = await captureSnapshots(project.dir, {
frames,
timeout,
at: atTimestamps,
outputDir: snapshotDir,
});
if (paths.length === 0) {
console.log(
@@ -466,7 +491,9 @@ export default defineCommand({
process.exit(1);
}
console.log(`\n${c.success("◇")} ${paths.length} snapshots saved to snapshots/`);
console.log(
`\n${c.success("◇")} ${paths.length} snapshots saved to ${args.output ? snapshotDir : "snapshots/"}`,
);
for (const p of paths) {
console.log(` ${p}`);
}
@@ -474,7 +501,6 @@ export default defineCommand({
// Generate contact sheet for quick AI review
try {
const { createSnapshotContactSheet } = await import("../capture/contactSheet.js");
const snapshotDir = join(project.dir, "snapshots");
const sheets = await createSnapshotContactSheet(
snapshotDir,
join(snapshotDir, "contact-sheet.jpg"),
@@ -501,8 +527,6 @@ export default defineCommand({
const { GoogleGenAI } = await import("@google/genai");
const ai = new GoogleGenAI({ apiKey: geminiKey });
const model = process.env.HYPERFRAMES_GEMINI_MODEL || "gemini-3.1-flash-lite-preview";
const snapshotDir = join(project.dir, "snapshots");
const customQuestion =
describeArg === "true"
? "Describe this video composition frame in 1-2 sentences. Be specific and factual: what elements are visible, what text appears, is the frame blank/black/loading, what is the composition. Flag any obvious problems."
@@ -533,7 +557,7 @@ export default defineCommand({
const results = await Promise.allSettled(
paths.map(async (p) => {
const filename = p.replace("snapshots/", "");
const filename = basename(p);
const filePath = join(snapshotDir, filename);
if (!existsSync(filePath)) return { filename, desc: "file not found" };
const raw = readFileSync(filePath);
+4
View File
@@ -33,6 +33,10 @@ const GROUPS: Group[] = [
title: "Project",
commands: [
["lint", "Validate a composition for common mistakes"],
[
"validate",
"Runtime-validate a composition in headless Chrome (JS errors, missing assets, contrast)",
],
["beats", "Detect beats in the music track and write beats/<audio>.json"],
["inspect", "Inspect rendered visual layout across the timeline"],
["snapshot", "Capture key frames as PNG screenshots for visual verification"],