fix: storyboard-angle review follow-ups (M1 bg-on-clip, B3 slideshow, parser guard, CLI fixes)#1791
Conversation
…arser 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>
… 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>
jrusso1020
left a comment
There was a problem hiding this comment.
Posting before drafting context: critical clarification first, then the substantive review.
⚠️ Slack-thread mismatch worth flagging
On the Slack thread in C0ACCNHLG3U (1782752554), this PR was linked as the iterative-confirmation-UX PR — the "after generating the storyboard skeleton it pauses and asks the user to confirm + whether to open the preview" flow responding to the "too automated, make it more iterative" feedback.
That flow is already in master from HF#1635 (commit 1967901b5, ~Jun 19). It's the existing "After drafting, show a frame-by-frame summary. In that same message ask the user (a) to approve or request changes, and (b) whether they want a live preview of the storyboard scaffold... Iterate until approved" text in SKILL.md of faceless-explainer + pr-to-video + product-launch-video. This PR (#1791) does NOT add or modify that flow. It appears as CONTEXT in the diff (space prefix), not as added lines.
#1791 is genuinely fix: storyboard-angle review follow-ups as the title says — addressing M1 / B3 / parser drift / CLI/lint fixes. The substance is solid (see below), but if the routing question was "is this the iterative-confirmation PR?", the answer is no — that landed in #1635 already.
Substantive verification — review-item-by-review-item
Cross-checked each row of the body's "Findings addressed" table against the diff:
| Body claim | Verified in diff |
|---|---|
M1 — root-background trap → frame-worker.md (×3) + SKILL.md Step 5 (×3) require full-bleed background on a class="clip" layer, never #root |
✓ Confirmed in skills/product-launch-video/sub-agents/frame-worker.md — new bullet "Full-bleed background on a class='clip' layer, never #root" with rationale ("at assembly the frame root is clip-gated to its scene window"). |
B3 — slideshow renders only slide 1 → slideshow/SKILL.md gains Output section + render warns |
✓ Both prongs landed. SKILL.md adds an "Output — a navigable deck, not a linear MP4" section with the explicit "Do not hyperframes render a slideshow into a single MP4" guidance. render.ts:661-685 adds the slideshow-island detector that warns when rendering a deck — uses slideshowIslandRegex from @hyperframes/core/slideshow, wrapped in try/catch (best-effort, never blocks). |
Parser drift — vendoredParity.test.ts pins byte-identity + parse-identity |
✓ Confirmed. Test asserts 3 vendored storyboard.mjs copies are byte-identical to each other, AND that a vendored copy parses a representative storyboard identically to @hyperframes/core. Sample covers frontmatter, H2/H3 frame headings, meta keys + aliases, unknown status (warning), unparseable duration (warning), unknown extra keys, free-form narrative — the surfaces most likely to drift. Solid guard. |
lint — -apple-system / BlinkMacSystemFont added to GENERIC_FAMILIES |
✓ packages/lint/src/rules/fonts.ts:16 adds both vendor-prefixed system-font keywords to the set, with an inline comment explaining the engine resolves them to the OS UI font. Test in fonts.test.ts:261 verifies no findings on -apple-system, BlinkMacSystemFont, system-ui, sans-serif. |
validate missing from --help, snapshot -o ignored, snapshot contradictory font status, inspect text_occluded false-positive on crossfades, inspect content_overlap on stacked flex, capture stores display dims (not source res) |
The file-list confirms each surface is touched (help.ts +4, snapshot.ts +46/-22, layout-audit.browser.js +45/-2, mediaCapture.ts +14, contentExtractor.ts +5, capture/video.ts +8/-3). Body's per-item descriptions track the diff scopes. |
Body-vs-diff scope note (additive, non-blocking)
The frame-worker.md changes in all 3 skills go beyond just the M1 fix described in the body. Each file also restructures the prior effects-model section into the time-coded shot sequence + blueprint: + focal: + roles: model — aligning these older skills with the shot-sequence architecture #1745/#1778 landed on product-launch-video. This appears intentional (it shifts the worker's input contract), but isn't called out in the body's "Findings addressed" table.
Per REVIEW_DISCIPLINE rule 3, worth a one-line body note: "plus aligns faceless-explainer + pr-to-video frame-worker.md to the shot-sequence input contract (parity with product-launch-video)." Not a blocker — just clarity for archaeology + auto-changelog readers.
Smaller item
The frame_id example also got bumped from 03-mechanism → 03-compounds (faceless) / 04-the-fix (pr-to-video). Cosmetic, but worth knowing the example reads were updated to match the skill's own narrative shape.
CI + verdict
CI 45/45 deduped green (52 raw — 7 superseded runs filtered per the rollup-dedup gotcha). No blockers.
COMMENT only — external author (WaterrrForever + Miao are allowlisted but not in trusted-stampers tier). Routing through <@U08E7PV788Z> for any stamp/merge call per protocol. No remaining blockers from me.
— Jerrai
What
Follow-ups to the storyboard-angle review (jrusso1020). The storyboard core itself was solid; this PR fixes the blockers / majors / minors that surfaced when driving real videos and the CLI end-to-end, plus closes the systemic gaps the review flagged.
Two commits, by domain:
fix(skills)— docs + a parser-drift guard test.fix(cli)—@hyperframes/cli+@hyperframes/lintfixes.Findings addressed
frame-worker.md(×3) +SKILL.mdStep 5 (×3) now require a frame's full-bleed background on aclass="clip"layer, never the#root/data-composition-idelement. The root is clip-gated to its scene window, so a background on it is not a dependable ground. (The assembler already paintsframe.md'scanvasonto index#rootas the base ground.)slideshow/SKILL.mdgains an Output section: decks render viapresent;render index.htmlcaptures only the first composition → truncated MP4; linear export is deferred.rendernow warns when the target carries a slideshow island.vendoredParity.test.ts: asserts the three vendoredstoryboard.mjscopies are byte-identical and parse-identical to@hyperframes/core.-apple-systemeven with a generic fallback-apple-system/BlinkMacSystemFontadded toGENERIC_FAMILIES(+ test).validatemissing from--helphyperframes --help.-oignored-o/--outputnow exists and is honored (resolved once, threaded through capture + contact-sheet + Gemini).errorreads as FAILED (an unrequested@font-faceisunused, not a contradiction).text_occludedfalse-positive on crossfadesdata-composition-idmount and a scene is mid-fade; a same-scene / two-settled-scenes overlap still flags.content_overlapon stacked flex / number-lockupsvideo-manifest.jsonnow recordssourceWidth/Height(videoWidth/Height) alongside the DOM box; consumers size off the source dims.Verified separately, no change needed
<template>transport, host↔innerdata-composition-idmatch, and root-styled-by-class rules are already documented inhyperframes-core/references/sub-compositions.md(Pitfalls 1–3). Theid == filenameslug is a skills-pipeline convention, enforced byassemble-index.mjsand documented inframe-worker.md— correct placement.mainbefore this PR (WCAG now samples the composited render;assemble-index.mjsexits non-zero on an id mismatch). The M1 doc rule above is the remaining authoring-side guard.Verification
bun run typecheckgreen for@hyperframes/cli+@hyperframes/lint.layout-audit.browser(13),contentExtractor(3),capture/video(20),render(19),fonts(27),vendoredParity(2).oxlint+oxfmt --checkclean on all changed files;fallow audit --base origin/mainpasses (0 introduced findings).🤖 Generated with Claude Code