feat(skill,cli): pipeline quality v2 — font extractor + skill-prose verification#984
Closed
ukimsanov wants to merge 90 commits into
Closed
feat(skill,cli): pipeline quality v2 — font extractor + skill-prose verification#984ukimsanov wants to merge 90 commits into
ukimsanov wants to merge 90 commits into
Conversation
…hout_font_face) Two new lint rules that catch the #1 composition quality failure: - google_fonts_import: warns on fonts.googleapis.com in <link> or @import - font_family_without_font_face: warns when font-family CSS has no matching @font-face Verified against 3 baseline sites (framer, raycast, workos) — correctly detects all font loading failures found in manual A/B testing.
Step 4 (storyboard): - Add Capabilities Audit section (HTML-in-Canvas, texture mask text, SFX, rembg) Step 6 (build): - Pre-generated @font-face block instruction for sub-agent dispatch - Per-composition lint gate after each beat - Asset cross-reference as blocking check - Depth Layers vocabulary (background/midground/foreground) - SFX wiring convention with track index allocation - Animation density minimum (15+ GSAP calls) with production reference - Easing variety check (3+ distinct easings) techniques.md: - Add Easing Vocabulary section with full GSAP palette and mood mapping
- Fix technique count: "10" → "11" in techniques.md and step-6 (was out of sync) - Fix VFX registry names: npx hyperframes add --tag html-in-canvas (not bare name) - Fix step-5 sequencing: remove "update index.html" before it exists in step-6 - Fix shader transitions: inline available shader names instead of repo-only path - Fix launch-video-2 reference: replace inaccessible file path with inline metrics - Upgrade Gemini API key from "optional" to recommended with user prompt
The vision API rejects image/svg+xml, so SVGs were silently skipped during Gemini captioning — leaving 97% of icons/logos undescribed. This caused asset utilization as low as 3.8% on SVG-heavy sites (Huly, Raycast). Now sends SVG source code as text to Gemini, which reads the XML structure and describes what it renders. Truncates SVGs >10KB to keep token costs low. Tested on Huly: 2/52 → 37/52 assets described (3.8% → 71%).
The producer's deterministicFonts.ts pre-bundles 18 font families as data URIs (Inter, Roboto, JetBrains Mono, etc.). The lint rule was falsely flagging these as missing @font-face. Now only warns for fonts not in the pre-bundled FONT_ALIASES list — brand-specific fonts like GT Walsheim or Aeonik that the producer genuinely can't resolve. Fonts available via Google Fonts (Path 2) still trigger warnings since they require network access at render time and may fail in sandboxed environments.
1. Capabilities Discovery: replace descriptive guidance with executable commands (npx hyperframes list, ls registry/blocks/). Agent was told to "check" but never ran the commands — now it has exact bash to run and paste results. 2. SFX decoupled from storyboard: removed SFX listing from Step 4 Capabilities Audit. SFX are now wired in Step 6 Audio Wiring only, matched to the storyboard's creative moments. Prevents creative direction from revolving around the SFX inventory. 3. Font @font-face: rewritten to explain the auto-resolve system. Common fonts (Inter, Roboto, etc.) skip @font-face. Brand fonts use tokens.json for family mapping. Hashed filenames = Google Fonts subsets that auto-resolve. Prevents agents from giving up on hashed filenames. 4. SFX wiring table: added moment-type → SFX mapping so agents match sound to emotion instead of listing files arbitrarily.
…ions Tested and confirmed: shader transitions work with sub-composition pipelines. The pattern: 1. Add class="scene" to beat host divs in index.html 2. Load shader-transitions CDN script 3. Call HyperShader.init() with host div IDs and transition configs 4. Engine detects window.__hf.transitions and composites with shaders Verified with minimal test: cross-warp-morph between two sub-compositions rendered successfully (layered composite, 180 frames, SDR).
Root cause of snapshot seeking bug: without position:absolute, beat divs stack vertically in normal flow. The runtime sets visibility:hidden/visible to toggle beats, but this only works when beats overlap in the same frame. Verified: with position:absolute + local CLI, snapshots at t=15 correctly show beat-4 content instead of beat-1 stuck at every timestamp.
Created manifest.json for all 19 SFX files with exact duration from ffprobe and Gemini audio analysis description. Updated Step 6 to reference manifest instead of generic moment-type table.
CTA beats are consistently under-animated (13 GSAP calls for 8-second duration in v3 tests). Added explicit guidance: treat CTA with same animation density as feature beats, minimum 15 GSAP calls, logo entrance as an event not a fade. Also clarified that beat count is agent's decision based on content — no fixed number.
…/ captioning Snapshot: HyperShader shows a loading overlay ([data-hyper-shader-loading]) while pre-rendering transition frames. Snapshots taken during loading show "Preparing scene transitions" instead of content. Now waits up to 60s for the overlay to disappear before capturing. SVG captioning: Gemini captions for svgs/ subdirectory were generated but never used in asset-descriptions.md. The generateAssetDescriptions function had a separate code path for svgs/ that used bare "icon:" labels instead of checking geminiCaptions. Now checks geminiCaptions first.
Generates pre-valid index.html + composition templates from STORYBOARD.md. Bakes in: position:absolute, HyperShader wiring, autoAlpha toggles, SFX placement from manifest, narration audio, grain overlays, scene visibility management. Sub-agents only fill creative content — no pipeline mechanics to learn. Follows the claude-design-hyperframes.md pattern: code handles structure, AI handles creativity. Tested: passes lint with 0 errors on first generation.
…ation - Add 20 SFX files to website-to-hyperframes/assets/sfx/ with manifest.json - Add device GLTF models (iphone.glb, macbook.glb) and reference images - CLAUDE.md: document local CLI path for capture, snapshot, and shader-transitions - .gitignore: update patterns
…tic colors - contactSheet.ts (new): paginated grids for all asset types - fit:contain replaces fit:cover — no more aspect-ratio cropping - SVG scanner covers both assets/svgs/ and assets/ root directories - agentPromptGenerator: semantic color role labels + dynamic page listing - snapshot.ts: updated for string[] return from contact sheet functions
…, bundle text-effects - visual-styles.md: replace YAML recipes with design tradition descriptions + pitfalls - visual-vocabulary.md moved to w2h skill; house-style.md light/dark mapping removed - transitions.md: Energy table converted to motion qualities; Mood table to characteristics - beat-direction.md: rhythm table removed; verb table restructured by physical character - dynamic-techniques.md: energy table restructured with explanatory principles - motion-principles.md: replace shouty tone with common-defaults framing - typography.md: replace guardrails with defaults-to-watch-for framing - video-composition.md: fix density contradiction (3-layer rule vs 8-10 target) - techniques.md: remove When-to-Use-What table; fix Lottie package name - text-effects.md (new): 24 bundled text animation effects with GSAP specs - assets/text-effects/ (new): full JSON specs for all 24 effects
- SKILL.md: creative tension principle, Step 5.5 self-critique, vision note - visual-vocabulary.md: inverted to brand-cues→dimensions (replaces user-words→recipes table) - step-0-capture.md: paginated contact sheet filenames, tighten individual asset reads - step-1-design.md: user gate after DESIGN.md, generic-vs-specific iteration guide examples - step-2-brief.md: personalized capabilities pitch + Q2/Q3 anti-prescriptive rewrites - step-3-storyboard.md: creative tension check, Text Animations + Implementation References per beat - step-4-vo.md: HeyGen TTS first, music gap for non-narrated videos - step-5-build.md: agent-agnostic sub-agents, captions stub ban, orphaned comp check - step-6-validate.md: snapshot formula max(beats×3, ceil(duration/2)), density vs storyboard spec - capabilities.md: fix VFX count, add text-effects reference - html-in-canvas-patterns.md: Three.js 0.147→0.181.2 ESM, Math.random→mulberry32
These were superseded by the new 7-step pipeline structure in the website-to-hyperframes skill restructure.
Old step files from before the pipeline v2 restructure were still tracked alongside the new versions, causing agents to see a conflicting dual pipeline. Removed: step-1-capture, step-4-storyboard, step-5-vo, step-6-build, step-7-validate, composition-guide, generate-skeleton.mjs
- HANDOFF.md: 747-line context doc for the May 15-16 pipeline quality session (git state, all skill changes, eval arena, video locations, key discoveries) - AGENT-FEEDBACK-V2.md: 7-agent post-run friction report with priority action items (P0: HyperShader CSS crossfade bug, snapshot loading-screen blindspot) - CLAUDE.md: add READ HANDOFF.md FIRST notice at top for new sessions
snapshot: fix two shader pre-render wait failure modes - Cold cache: overlay stays in DOM as display:none after hiding, element-absence check never resolved, always timed out at 60s - Warm cache: IndexedDB hydration runs without overlay, absence check resolved instantly while prewarming was still in progress - Fix: primary signal is window.__hf.shaderTransitions[].ready; overlay display:none is fallback for older builds - Verified: huly-promo snapshots show composition content, not loader shader-transitions: optional shader field for CSS crossfade mixing - TransitionConfig.shader is now optional (ShaderName | undefined) - Omitting shader = smooth CSS opacity crossfade via fallback path - CSS-only transitions skip program compilation and prewarming - HfTransitionMeta.shader made optional in engine/src/types.ts - Verified: mixed CSS + WebGL composition renders all three beats correctly step-3-storyboard: SFX assigned here with exact files, not in Step 5 - Agent reads sfx/manifest.json and assigns exact filenames per moment - Less-is-more: most beats need zero SFX, one max per beat - Removed duplicate HTML-in-Canvas API detail (build concern, not storyboard) step-5-build: implement storyboard SFX exactly, zero creative decisions step-4-vo: required timing reconciliation gate before Step 5 - Compare real audio vs planned total; if delta >15% rescale or trim - Hard cap on CTA hold: 2-3s after last word, not extended dead silence
…kokoro step-3: sfx placement rules grounded in audio production standards - impact sounds: trigger at visual moment, let decay tail bleed (J-Cut) - riser/build-up: trigger at climax_time - sfx_duration so peak lands on moment - data-duration must equal manifest duration, never trimmed to fit beat time - volume under narration: 0.2-0.3 max (no auto-ducking in HyperFrames) - vo start timing: explicit storyboard decision, document in Global Direction step-5: asset path rule as first sub-agent rule, impossible to miss - @font-face: paste declaration into sub-agent prompt explicitly - Inter Variable -> Inter (compiler only maps base name) - querySelector null guard: getElementById for elements inside beat hosts - wcag gradient false positive: data-layout-ignore for decorative elements - captions stacking: initial opacity:0, one group visible, verify with snapshot step-4: kokoro pronunciation guidance - known failure patterns: Vercel, WorkOS, One API - phonetic substitution workflow: test first sentence, fix in narration.txt
cli/transcribe: default output dir to dirname(inputPath) instead of CWD - transcript.json now lands next to narration.wav regardless of CWD - fixes silent stale-transcript poisoning when running from repo root step-5 sub-agent RULES: script+style inside template (verified root causes) - mercury v3: scripts outside template caused ALL GSAP to silently fail - workos v3: CSS style blocks inside template not applied after injection - expanded querySelector null guard: getTotalLength crash documented
… beats root cause verified from arc-launch v3 render analysis: - all beats had data-start=0 and data-duration=26 (total video length) - engine seeks each beat's sub-composition to global_time - data_start - with data-start=0, beat-2 (BEAT=5.5s GSAP) is seeked to global_t=7 - gsap timeline exhausted at t=5.5, engine marks sub-comp invisible - content disappears exactly at each beat's GSAP timeline end fix verified: sequential data-start per transition point + data-duration matching each beat's GSAP duration → all beats render correctly added concrete code example to step-5-build.md with explanation
Removes the 81-scene examples library at skills/website-to-hyperframes/examples/ (introduced over 22 commits between 04827b9 and 6ee54ef). Real-world eval showed agents using the library as a copy-paste source: the Raycast eval video had every beat structurally lifted from examples/04-composed-ui/ with brand colors swapped, zero captured brand assets used, and even an invented logo in the CTA beat. Examples became building blocks for lazy agents; the framework philosophy of composing original beats was lost. Removed: - skills/website-to-hyperframes/examples/ (897 files, 319 MB) - HANDOFF-examples-library.md - HANDOFF-full-video-refs.md - .gitignore rules for examples snapshots + full-video-refs compositions exception Scrubbed from skill prose: - SKILL.md: 32-line 'Study the Example Library' + 3-mode section deleted. The manifesto ('compose load-bearing visuals', 'captured assets are accents') is preserved with a screenshot-trap warning added in its place. - step-1-design.md: 3 examples references reworded - step-2-brief.md: Question 3 'composed library' bullet and pricing example reworded to point at capabilities.md instead of scene paths - step-3-storyboard.md: Technique-pick checklist no longer requires citing scene paths or picking a mode. Asset Audit, Composition + Accents sections describe composition rather than cite scenes. Preserved (the philosophical content of 851d62f workflow audit batch 18 is independent of the examples library and stays): - Step -1 manifesto, opener-default rule, asset-as-accent framing - step-1-design.md collapse from 615 -> 157 lines (the 3-section structure) - step-2 question reframing toward compose-first - step-3 asset audit reframing toward decoration Also preserved (in-session refactors, unrelated to examples): - capabilities-overview.md collapsed into capabilities.md (one source of truth) - html-in-canvas-patterns.md relocated from website-to-hyperframes/references/ to hyperframes/references/ (the content is generic to all HyperFrames compositions, not pipeline-specific) - /examples/ -> /examples/ .gitignore fix from 04827b9 (real bug fix, unrelated to skill examples directory) Reverted to baseline state (e1bd22d) and re-applied in-session edits: - step-5-build.md (no in-session edits) - capabilities.md (re-applied Essential Rules block + html-in-canvas path) - beat-builder-guide.md (re-applied html-in-canvas path) The 28 commits comprising the examples library work remain accessible on the archive/examples-library branch for revival if needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds huly-*/, raycast-*/, *-demo-*/, test-runs/, test-outputs/ to the ignore list. Sub-agents iterating on captures sometimes write per-brand project directories at the repo root; the existing *-demo/ rule misses suffix variants like huly-product-demo-2/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The banner directing every session to read HANDOFF.md at the top of the repo's CLAUDE.md is no longer needed — the handoff doc is referenced inline where context is actually relevant. Banners that prepend large docs to every conversation eat context budget without proportional value. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Modern frameworks (Next.js, Webpack) rename downloaded fonts with content hashes for cache-busting. The captured filenames look like '19cfc7226ec3afaa-s.woff2' and the CSS @font-face mapping that originally tied each hash to a family name is lost during capture. The previous DESIGN.md guidance — "hashed = Google Fonts subset, use Inter as substitute" — was wrong half the time. The Raycast capture, for example, ships Inter, JetBrains Mono, and Geist Mono all hashed identically. Every OpenType / WOFF / WOFF2 file embeds a 'name' table (part of the spec since 1996) with the family name, subfamily, weight class, and variation axes. Subsetting and hashing don't strip it. This change uses fontkit to read the name table from each downloaded font and writes a per-project manifest the rest of the pipeline can consult instead of guessing. Output: capture/extracted/fonts-manifest.json with per-file metadata and per-family aggregation (weights captured, variable-font axes detected). Cross-tested against 9 existing captures (Huly, Raycast, Framer, WorkOS, Mercury, Loom, Arc, HeyGen, Railway): 132/132 files identified, 0 failures. The 'ES Build Neutral' font in Huly captures — the one DESIGN.md was calling '__esbuild_b38aaf' and substituting Inter for — now resolves to its real family name with all 3 weights aggregated. Includes canonicalization that collapses static-weight family-name packaging ('Inter Medium' → family 'Inter' with weight 500) so the families[] aggregate reads cleanly. Handles compound weight names ('Semi Bold' → 'SemiBold' before stripping). rawFamily preserved for transparency. - packages/cli/src/capture/fontMetadataExtractor.ts (new, ~210 lines) - packages/cli/src/capture/index.ts (wired in after downloadAndRewriteFonts) - packages/cli/package.json + bun.lock (added fontkit@2.0.4, @types/fontkit@2.0.9) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bundles the remaining session work: - TS2538 fix in renderOrchestrator.ts (pre-existing, unblocks build hook) - verify-beats CLI: structural verification gate sub-agents cannot bypass. Cross-checks compositions/beat-N-verify.json claims against actual HTML and snapshot files on disk. 11 gates incl. lint exit, frame observations, brand hex grep-able, asset paths grep-able, headline >=80px. - Skill restructure: concept-first ordering, video-as-medium grammar, brand-floor 'must' rules (logo in opener+closer), step-1-design.md component CSS restored (3 -> 6 sections, 165 -> 317 lines). - step-5/step-6 wire the verify-beats gate into Definition of Done. - Beat-builder-guide rewritten so sub-agents write verify.json instead of chat-report 'looks good'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves 7 conflicts: - .gitignore — kept main's examples/* + AWS Lambda negations, kept branch's repo-root project-dir ignores (huly-*/, raycast-*/, *-demo-*/, test-runs/, test-outputs/), kept main's .claude/worktrees/ etc. - bun.lock + packages/cli/package.json — took main's esbuild ^0.25.12, kept branch's fontkit ^2.0.4 - packages/cli/src/cli.ts — kept both: main's lambda command + branch's verify-beats command - packages/cli/src/commands/snapshot.ts — kept branch's version (has sub-composition seek fix from 4ae59e5) - packages/producer/src/services/renderOrchestrator.ts — took main's version (main refactored the area my TS fix targeted) - packages/shader-transitions/src/hyper-shader.ts — took main's version (explicit undefined check + cleaner prog var) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Recent batches went too far toward "compose everything from divs," with sub-agents producing all-CSS videos that ignored captured SVG logos, illustrations, and hero art. Verification also got too mechanical — verify-beats passing was being treated as the whole quality bar. - SKILL.md Step -1: replace "captured assets are accents/decorations" with positive framing that captured SVGs/illustrations CARRY beats. - step-3-storyboard.md: assets are first-class beat content alongside composed UIs; only the *order* is concept-first. - beat-builder-guide.md: distinguish "no window chrome / no parked camera" (about webpage-mimicry) from "use captured brand assets" (which IS encouraged). Add a "patterns that ARE shots" list. - step-6-validate.md: verify-beats is a structural backstop, NOT the verification. Restore the slower per-frame review and add an explicit "Captured asset utilization" dimension to the critic prompt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User feedback: agents were glancing at contact sheets, not really reviewing each cell. And the "no CSS browser/dashboard chrome" rule was being read as an absolute ban even when the storyboard genuinely needed a product tour or chrome-as-subject shot. - step-0-capture.md: contact-sheet viewing is now "every cell, name 5 assets per page before moving on" with explicit callout of the prior failure mode (agents glanced, then beats referenced assets that didn't exist or missed the logo). - step-6-validate.md: snapshot contact-sheet review is now per-cell with one-sentence-per-cell requirement, calling out specific past failures (black beats, off-screen logos, clipped headlines). - beat-builder-guide.md: chrome / full-app-layout patterns downgraded from ❌ to ⚠ — fine when storyboard genuinely calls for it; the rule is "don't reach for these by default." Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fallow audit reportFound 92 findings. Dead code (1)
Duplication (53, showing 50)
Showing 50 of 53 findings. Run fallow locally or inspect the CI output for the full report. Health (38)
Generated by fallow. |
Per user feedback: a CLI that greps composition HTML and trusts JSON claims catches structural lies but misses semantic issues — boring beats, misaligned camera moves, captured assets placed off-screen, GSAP timelines that only cover the first 2 seconds. Those failures only surface when somebody actually opens the file and reads it. This PR replaces the verify-beats CLI gate with skill prose telling the main agent to open each compositions/beat-N.html and read it top-to-bottom against DESIGN.md and STORYBOARD.md. - Removed packages/cli/src/commands/verify-beats.ts (340 lines), its cli.ts subCommands entry, and its help.ts listing. - skills/website-to-hyperframes/SKILL.md Step 5 gate now reads "Main agent opens each beat HTML and reads it top-to-bottom" — no CLI command involved. - step-5-build.md "Run verify-beats" section replaced with an explicit per-beat read protocol: cross-check brand hex / captured assets / headline size / GSAP timeline coverage / technical gates against DESIGN.md + STORYBOARD.md, plus the brand-floor check. - beat-builder-guide.md "Step 5: Write the verification artifact" (verify.json schema + field requirements) removed. Sub-agents report back with concrete observations; main agent verifies by opening the file, not by reading a JSON claim. - step-6-validate.md "verify-beats exits 0" checkbox replaced with "Every beat HTML read top-to-bottom" + a per-beat verdict template that names hex codes, asset paths, headline px, timeline coverage, and storyboard alignment. - step-3-storyboard.md brand-floor rules now reference "main agent reads each beat HTML" instead of "verify-beats checks them". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ukimsanov
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the HyperFrames “website-to-hyperframes” pipeline and capture tooling to improve brand fidelity (notably fonts/assets) and tighten quality gates by shifting verification from CLI heuristics to explicit per-beat file reads. It also adds new lint rules around font loading and invalid capture asset paths, plus supporting skill/reference documentation updates.
Changes:
- Add capture-time font identification (OpenType name table) and richer capture artifacts (design-styles, contact sheets, SVG captioning).
- Restructure skill docs to be concept-first and require main-agent, per-beat HTML file reads for verification/validation.
- Add/extend lint rules to catch common failures (Google Fonts imports, missing
@font-face, and../capture/paths).
Reviewed changes
Copilot reviewed 101 out of 131 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/website-to-hyperframes/references/visual-vocabulary.md | Adds a brand-derived “six axes” framework for interpreting style and user modifiers. |
| skills/website-to-hyperframes/references/step-7-validate.md | Removes the previous Step 7 validate/deliver doc. |
| skills/website-to-hyperframes/references/step-6-validate.md | Introduces a new “Validate & Deliver” gate emphasizing per-beat evidence and snapshot verification. |
| skills/website-to-hyperframes/references/step-5-vo.md | Removes the previous VO/timing step doc. |
| skills/website-to-hyperframes/references/step-4-vo.md | Adds a new VO/timing/captions step with provider guidance and timing reconciliation. |
| skills/website-to-hyperframes/references/step-4-storyboard.md | Removes the previous storyboard step doc. |
| skills/website-to-hyperframes/references/step-3-script.md | Removes the previous script-writing step doc. |
| skills/website-to-hyperframes/references/step-2-design.md | Removes the previous DESIGN.md authoring guidance doc. |
| skills/website-to-hyperframes/references/step-2-brief.md | Adds a new strategy/messaging step focused on aligning message/arc/audience. |
| skills/website-to-hyperframes/references/step-1-capture.md | Removes the previous capture step doc. |
| skills/website-to-hyperframes/references/step-0-capture.md | Adds a new capture-and-understand step emphasizing contact sheets, design-styles, and fonts-manifest. |
| skills/website-to-hyperframes/assets/sfx/manifest.json | Adds an SFX manifest for named sound effects. |
| skills/launch-video/SKILL.md | Introduces a new “launch-video” skill emphasizing a billboard-per-beat pattern. |
| skills/hyperframes/SKILL.md | Updates reference pointers (e.g., techniques description) and expands recommended technique catalog. |
| skills/hyperframes/references/video-composition.md | Refines density guidance to be intent-driven rather than a fixed element count. |
| skills/hyperframes/references/typography.md | Reworks typography guidance (banned fonts, defaults to watch, principles). |
| skills/hyperframes/references/transitions.md | Refactors transition guidance to focus on motion character; documents mixing shader + CSS crossfade. |
| skills/hyperframes/references/text-effects.md | Adds documentation for bundled, named text effects and how to implement them. |
| skills/hyperframes/references/prompt-expansion.md | Updates prompt expansion guidance to reference DESIGN.md consistently and refine rhythm guidance. |
| skills/hyperframes/references/motion-principles.md | Reframes motion rules as “defaults that cause monoculture” + clarifies pacing/easing principles. |
| skills/hyperframes/references/dynamic-techniques.md | Clarifies caption technique selection as intensity calibration (karaoke baseline). |
| skills/hyperframes/references/beat-direction.md | Adds requirement to choose named text effects; expands transition section and rhythm guidance. |
| skills/hyperframes/house-style.md | Normalizes references from design.md → DESIGN.md and adjusts light/dark guidance to brand-driven. |
| skills/hyperframes/assets/text-effects/specs/typewriter.json | Adds a text-effect spec definition (typewriter). |
| skills/hyperframes/assets/text-effects/specs/top-down-letters.json | Adds a text-effect spec definition (top-down staircase letters). |
| skills/hyperframes/assets/text-effects/specs/stagger-from-edges.json | Adds a text-effect spec definition (edges-in stagger). |
| skills/hyperframes/assets/text-effects/specs/stagger-from-center.json | Adds a text-effect spec definition (center-out stagger). |
| skills/hyperframes/assets/text-effects/specs/spring-scale-in.json | Adds a text-effect spec definition (per-word spring scale). |
| skills/hyperframes/assets/text-effects/specs/soft-blur-in.json | Adds a text-effect spec definition (per-character soft blur). |
| skills/hyperframes/assets/text-effects/specs/short-slide-right.json | Adds a layout-aware text-effect spec (shared slide + word opacity staging). |
| skills/hyperframes/assets/text-effects/specs/short-slide-down.json | Adds a layout-aware text-effect spec (kinetic top build). |
| skills/hyperframes/assets/text-effects/specs/shimmer-sweep.json | Adds a whole-element text-effect spec (shimmer sweep). |
| skills/hyperframes/assets/text-effects/specs/shared-axis-z.json | Adds a whole-element text-effect spec (shared axis Z). |
| skills/hyperframes/assets/text-effects/specs/shared-axis-y.json | Adds a per-word text-effect spec (word cut staircase). |
| skills/hyperframes/assets/text-effects/specs/shared-axis-x.json | Adds a whole-element text-effect spec (shared axis X). |
| skills/hyperframes/assets/text-effects/specs/scale-down-fade.json | Adds a whole-element text-effect spec (scale-down fade). |
| skills/hyperframes/assets/text-effects/specs/per-word-crossfade.json | Adds a per-word text-effect spec (crossfade). |
| skills/hyperframes/assets/text-effects/specs/per-character-rise.json | Adds a per-character text-effect spec (rise, no blur). |
| skills/hyperframes/assets/text-effects/specs/micro-scale-fade.json | Adds a whole-element text-effect spec (micro scale fade). |
| skills/hyperframes/assets/text-effects/specs/mask-reveal-up.json | Adds a per-line text-effect spec (mask reveal up). |
| skills/hyperframes/assets/text-effects/specs/line-by-line-slide.json | Adds a per-line text-effect spec (line-by-line slide). |
| skills/hyperframes/assets/text-effects/specs/kinetic-center-build.json | Adds a layout-aware text-effect spec (kinetic center build). |
| skills/hyperframes/assets/text-effects/specs/focus-blur-resolve.json | Adds a whole-element text-effect spec (focus blur resolve). |
| skills/hyperframes/assets/text-effects/specs/fade-through.json | Adds a whole-element text-effect spec (fade through). |
| skills/hyperframes/assets/text-effects/specs/depth-parallax-words.json | Adds a per-word text-effect spec (depth/parallax). |
| skills/hyperframes/assets/text-effects/specs/bottom-up-letters.json | Adds a per-character text-effect spec (bottom-up staircase letters). |
| skills/hyperframes/assets/text-effects/specs/blur-out-up.json | Adds a per-word text-effect spec (blur-out-up exits). |
| skills/hyperframes/assets/text-effects/effects/stagger-from-edges.json | Adds an effect wrapper file for the stagger-from-edges spec. |
| skills/hyperframes/assets/text-effects/effects/stagger-from-center.json | Adds an effect wrapper file for the stagger-from-center spec. |
| skills/hyperframes/assets/text-effects/effects/shared-axis-x.json | Adds an effect wrapper file for the shared-axis-x spec. |
| skills/hyperframes/assets/text-effects/effects/depth-parallax-words.json | Adds an effect wrapper file for the depth-parallax-words spec. |
| packages/core/src/lint/rules/fonts.ts | Adds lint rules for Google Fonts imports and missing @font-face for non-bundled fonts. |
| packages/core/src/lint/rules/fonts.test.ts | Adds Vitest coverage for the new font lint rules. |
| packages/core/src/lint/rules/composition.ts | Adds a lint error for ../capture/ paths that will 404 in Studio/renders. |
| packages/core/src/lint/hyperframeLinter.ts | Wires the new font lint rules into the linter. |
| packages/cli/src/commands/transcribe.ts | Defaults transcript output dir to the input file’s directory. |
| packages/cli/src/cli.ts | Adds automatic .env loading from CWD to reduce agent friction with API keys. |
| packages/cli/src/capture/types.ts | Extends capture types (SVG dimensions, new DesignStyles schema). |
| packages/cli/src/capture/tokenExtractor.ts | Captures SVG bounding box dimensions in extracted token data. |
| packages/cli/src/capture/screenshotCapture.ts | Adds basic popup/banner dismissal/hiding before scrolling screenshots; removes full-page.png note. |
| packages/cli/src/capture/index.ts | Writes tokens without SVG outerHTML, writes design-styles.json, extracts fonts-manifest, generates contact sheets, removes assets-catalog/detected-libraries outputs. |
| packages/cli/src/capture/contentExtractor.ts | Adds SVG captioning via Gemini (code-as-text) and integrates captions into asset descriptions. |
| packages/cli/src/capture/assetDownloader.ts | Improves asset naming using catalog context; de-duplicates inline SVG filenames. |
| packages/cli/src/capture/agentPromptGenerator.ts | Enhances AGENTS/CLAUDE prompt inventory (contact-sheet paging, design-styles, color-role hints). |
| packages/cli/package.json | Adds fontkit deps and bumps sharp. |
| package.json | Adds sharp at repo root devDependencies. |
| CLAUDE.md | Documents using the local CLI for capture/snapshot and local shader build usage. |
| AGENT-FEEDBACK-V4.md | Adds agent feedback notes for pipeline evaluation. |
| AGENT-FEEDBACK-V2.md | Adds earlier agent feedback notes for pipeline evaluation. |
| .gitignore | Ignores videos/ and common per-site project dirs; unignores skills/launch-video. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+88
to
+99
| function contactSheetRows(dir: string, baseFile: string, label: string): string[] { | ||
| const fullDir = join(outputDir, dir); | ||
| if (!existsSync(fullDir)) return []; | ||
| const all = readdirSync(fullDir) | ||
| .filter((f) => f.startsWith(baseFile.replace(".jpg", "")) && f.endsWith(".jpg")) | ||
| .sort(); | ||
| if (all.length === 0) return []; | ||
| if (all.length === 1) { | ||
| return [`| \`${dir}/${all[0]}\` | ${label} |`]; | ||
| } | ||
| return all.map((f, i) => `| \`${dir}/${f}\` | ${label} — page ${i + 1} of ${all.length} |`); | ||
| } |
Comment on lines
+114
to
+121
| message: | ||
| "Composition loads fonts from fonts.googleapis.com. External font requests " + | ||
| "fail in sandboxed/offline renders and add latency. Use local @font-face " + | ||
| "declarations with captured .woff2 files instead.", | ||
| fixHint: | ||
| "Replace the Google Fonts <link> or @import with @font-face { font-family: '...'; " + | ||
| "src: url('../capture/assets/fonts/Font.woff2'); } pointing to captured font files.", | ||
| }); |
Comment on lines
+55
to
+70
| const { readFileSync } = await import("node:fs"); | ||
| const { resolve } = await import("node:path"); | ||
| const envPath = resolve(process.cwd(), ".env"); | ||
| const envContent = readFileSync(envPath, "utf-8"); | ||
| for (const line of envContent.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed || trimmed.startsWith("#")) continue; | ||
| const eqIdx = trimmed.indexOf("="); | ||
| if (eqIdx < 1) continue; | ||
| const key = trimmed.slice(0, eqIdx).trim(); | ||
| const val = trimmed | ||
| .slice(eqIdx + 1) | ||
| .trim() | ||
| .replace(/^["']|["']$/g, ""); | ||
| if (key && !(key in process.env)) process.env[key] = val; | ||
| } |
Comment on lines
+34
to
+40
| 1. **View the contact sheets — carefully, every cell, not a glance.** Contact sheets are labeled grids that let you see many images at once. They are paginated: look for `contact-sheet-1.jpg`, `contact-sheet-2.jpg`, etc. (or `contact-sheet.jpg` for older captures). View ALL pages for each category. **For each page, name at least 5 specific assets you can see in it before moving on** — this forces you to actually look at every cell instead of scrolling past. Past agents have reported "viewed the contact sheet" after literally one glance, then later in Step 3 they wrote beats using assets that didn't exist or missed the brand logo entirely. Don't be that agent. The contact sheet is your single best opportunity to learn what's actually available in the capture. | ||
| - `capture/screenshots/contact-sheet-*.jpg` — scroll screenshots grid. View FIRST. Each cell numbered with scroll percentage. List the directory if unsure how many pages exist. | ||
| - `capture/assets/contact-sheet-*.jpg` — all downloaded raster images grid. Each cell labeled with filename. | ||
| - `capture/assets/svgs/contact-sheet-*.jpg` — all SVGs rendered as thumbnails. Each cell labeled with filename. Check `capture/assets/` root too — some captures store SVGs there instead of `svgs/`. | ||
| - `capture/screenshots/full-page.png` — the entire page as one tall image. Useful for scrolling animation videos. | ||
|
|
||
| After viewing the screenshot contact sheets, write 3-4 sentences describing the site's visual mood, layout patterns, color strategy, and overall feel. Then list, by filename, the 5-10 captured assets that look most promising for video use (logo, hero illustration, brand mark, gradient backgrounds, product art). **Open and view those promising assets individually** — the contact sheet thumbnails are too small to judge fine detail, but once you've narrowed to the 5-10 candidates, read each one carefully. Don't just trust the thumbnail. |
Comment on lines
+64
to
+71
| 10. **Individual images in `capture/assets/`** — The contact sheet pages cover all assets. Only open an individual file when: | ||
| - You are placing text over a screenshot and need to check the safe zone / exact content at full resolution | ||
| - A storyboard-assigned asset's contact sheet thumbnail is too small to judge its content | ||
|
|
||
| Do NOT batch-view individual assets at this stage. That is what the contact sheets are for. | ||
|
|
||
| 11. **`capture/extracted/assets-catalog.json`** — If you notice there is something interesting in remote URLs assets, you are more than welcome to use them!! | ||
|
|
Comment on lines
+48
to
+56
| 5. **`capture/extracted/asset-descriptions.md`** — One-line-per-file summary of all downloaded assets. Note which assets are most visually striking or useful for video. | ||
|
|
||
| 6. **`capture/extracted/fonts-manifest.json`** — Each downloaded font identified by its real family name (read from the binary's OpenType `name` table, so hashed Next.js/Webpack filenames are resolved automatically). Lists per-family aggregates with weights, variable-font axes, and file counts. Read this in Step 1 instead of guessing fonts from filenames. If the manifest's `unidentified[]` is empty, every captured font has a known identity. Skip the file if it doesn't exist (older captures). | ||
|
|
||
| ### Required to check and read IF they exist | ||
|
|
||
| 6. **`capture/extracted/animations.json`** — See for yourself if the site uses scroll-triggered animations, marquees, canvas/WebGL, or named CSS animations. Just good to know. | ||
|
|
||
| 7. **`capture/extracted/lottie-manifest.json`** — View each preview image at `capture/assets/lottie/previews/` to see what the animations look like. It will help you think of what you can do in the video. |
Comment on lines
+113
to
+117
| ### "Surprise me" / minimal direction | ||
|
|
||
| Default to the safe path that matches the brand and according to what is the video for (this is the minimum requirement users supposed to tell like where does the video goes to, or what audience or occasion or context is it for...): | ||
|
|
||
| But still write an ambitious storyboard. "Surprise me" means "impress me", not "play it safe." Go bold. |
Comment on lines
+70
to
+76
| const fixed = document.querySelectorAll<HTMLElement>("*"); | ||
| for (const el of fixed) { | ||
| const style = window.getComputedStyle(el); | ||
| if ( | ||
| (style.position === "fixed" || style.position === "sticky") && | ||
| style.zIndex !== "auto" && | ||
| parseInt(style.zIndex) > 100 && |
| "These are not in the auto-resolved font list, so the renderer cannot supply them automatically. " + | ||
| "Text will fall back to a generic font, producing incorrect typography in the video.", | ||
| fixHint: | ||
| "Add @font-face { font-family: '...'; src: url('../capture/assets/fonts/...woff2'); } " + |
Six concrete fixes for review findings (all verified against the
actual code/docs before fixing):
- fonts.ts (lint rule): fix-hints suggested `../capture/...` paths,
but the new composition lint rule errors on `../capture/` (root
pages serve compositions with the project root as base URL).
Fix-hints now use root-relative `capture/assets/fonts/...`.
- step-0-capture.md: removed two dead references — `full-page.png`
(capture stopped generating it; see screenshotCapture.ts:139) and
`assets-catalog.json` (capture stopped writing it; see
index.ts:514). Renumbered the read-IF-they-exist section so the
list no longer has two items numbered "6".
- step-2-brief.md "Surprise me" section: rewrote a draft-note
sentence that ended in a colon followed by nothing. The new prose
states the minimum context to ask for (where the video runs +
audience) and reasserts "bold, not safe."
- agentPromptGenerator.ts contact-sheet matcher: `startsWith` was
picking up `contact-sheet-svgs.jpg` (the SVG fallback sheet)
alongside `contact-sheet.jpg` and labeling both as raster sheets.
Now matches the exact base name plus paginated `-NNN` variants
only, so the SVG fallback sheet stays separate.
- cli.ts .env loader: handles `export FOO=bar` (common in dotfile
.env files) and inline `# comment` suffixes on unquoted values.
Quoted values now correctly read until the closing quote and
drop anything after — so `KEY="value" # note` reads as `value`.
- screenshotCapture.ts overlay scan: replaced
`querySelectorAll('*')` + `getComputedStyle` on every element
with a TreeWalker that early-exits on cheap viewport-rect checks
before the expensive `getComputedStyle` call, with a 5000-element
scan cap. On large pages this was the dominant cost inside
`page.evaluate()`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+97
to
+99
| const all = readdirSync(fullDir) | ||
| .filter((f) => paginatedRe.test(f)) | ||
| .sort(); |
Comment on lines
125
to
133
| tableRows.push( | ||
| "| `extracted/asset-descriptions.md` | One-line description of every downloaded asset. **Read this first.** |", | ||
| `| \`extracted/tokens.json\` | Design tokens: ${tokens.colors.length} colors, ${tokens.fonts.length} fonts, ${tokens.headings?.length ?? 0} headings, ${tokens.ctas?.length ?? 0} CTAs |`, | ||
| ); | ||
| tableRows.push( | ||
| `| \`extracted/tokens.json\` | Design tokens: ${tokens.colors.length} colors, ${tokens.fonts.length} fonts, ${tokens.headings?.length ?? 0} headings, ${tokens.ctas?.length ?? 0} CTAs |`, | ||
| "| `extracted/design-styles.json` | Computed styles from live DOM: typography hierarchy, button/card/nav styles, spacing scale, border-radius, box shadows. Primary data source for DESIGN.md. |", | ||
| ); | ||
| tableRows.push( | ||
| "| `extracted/asset-descriptions.md` | One-line description of every downloaded asset. Read this for asset selection — only open individual files for safe-zone checking. |", | ||
| ); |
Comment on lines
+75
to
+84
| ```bash | ||
| # IMPORTANT: .env values are NOT automatically inherited by CLI subprocesses. | ||
| # Always export GEMINI_API_KEY explicitly or Gemini descriptions won't run: | ||
| export GEMINI_API_KEY=$(grep GEMINI_API_KEY .env | cut -d= -f2) | ||
| npx tsx packages/cli/src/cli.ts snapshot <project-dir> --frames <N> | ||
|
|
||
| # Pass a custom question to Gemini instead of the default prompt: | ||
| export GEMINI_API_KEY=$(grep GEMINI_API_KEY .env | cut -d= -f2) | ||
| npx tsx packages/cli/src/cli.ts snapshot <project-dir> --frames <N> \ | ||
| --describe "Is the brand logo visible in every beat? Is any beat showing a black or blank frame?" |
Comment on lines
+41
to
+59
| // invalid_capture_path — catches ../capture/ in src/href attributes and scripts. | ||
| // Sub-compositions live in compositions/ but are served relative to the project | ||
| // root, so all asset paths must be root-relative ("capture/..."). | ||
| // Using "../capture/..." works on disk but breaks in Studio and renders. | ||
| ({ rawSource, options }) => { | ||
| if (isRegistrySourceFile(options.filePath) || isRegistryInstalledFile(rawSource)) return []; | ||
| // Only flag in sub-compositions and root compositions — not in registry blocks | ||
| const matches = rawSource.match(/\.\.\/capture\//g); | ||
| if (!matches || matches.length === 0) return []; | ||
| return [ | ||
| { | ||
| code: "invalid_capture_path", | ||
| severity: "error", | ||
| message: `Found ${matches.length} asset path(s) using ../capture/ — will 404 in Studio and renders.`, | ||
| fixHint: | ||
| 'Replace all "../capture/" with "capture/" throughout this file. Compositions are served with the project root as their base URL, so paths must be root-relative, not relative to the compositions/ directory.', | ||
| }, | ||
| ]; | ||
| }, |
CI feedback after the round-1 fixes: - contactSheet.ts: `createSvgContactSheet` return type was `Promise<string | null>` but the body returned `string[]` in 4 places. Caller in `capture/index.ts` already treated it as an array (`svgSheets.length`). Fixed the type to match the actual behavior — `Promise<string[]>`. This unblocks the typecheck CI job. - agentPromptGenerator.ts: contact-sheet pagination sorted filenames lexicographically, which puts `contact-sheet-10.jpg` before `contact-sheet-2.jpg` at 10+ pages. Now extracts the numeric `-N` suffix from each filename and sorts numerically; `contact-sheet.jpg` without a suffix sorts first. - agentPromptGenerator.ts: prompt always listed `extracted/design-styles.json`, but `capture/index.ts:334` wraps that write in a try/catch and skips on failure. Row is now gated on `existsSync` so the prompt only points the agent at files actually on disk. - step-6-validate.md: removed the `export GEMINI_API_KEY` boilerplate in front of every snapshot command. The CLI auto-loads `.env` from CWD now (`cli.ts:50`), so the explicit export is dead ceremony that confused readers. The recovery instruction for missing `descriptions.md` is updated to point at the .env file. - composition.test.ts: added unit coverage for the `invalid_capture_path` rule — three cases: <img> src triggers error, multiple url() occurrences are counted, root-relative `capture/` paths are clean. (52/52 tests passing.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes 4 files that were checked in during the long evaluation history of this branch and don't belong in the merged PR: - HANDOFF.md — session handoff notes - AGENT-FEEDBACK-V2.md — pipeline v2 test run feedback - AGENT-FEEDBACK-V4.md — pipeline v4 test run feedback - studio-check.png — local Studio screenshot Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These 4 files (iphone.glb, macbook.glb, hyperframes-desktop.png, hyperframes-mobile.png, ~1.5MB total) were committed at the repo root by mistake. The legit copies live at `registry/blocks/vfx-iphone-device/models/` where the registry block expects them — the `path` field in registry-item.json is relative to the block directory, not the repo root. Nothing references the root-level folder.
Collaborator
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed the full diff (15.6K additions, 94 files). Miguel's right that this would benefit from a Graphite stack — the font extractor, contact sheet, snapshot changes, skill prose, and text-effect assets are largely independent. Here's the code review:
Actionable Issues
1. --describe runs by default despite comment saying opt-in — snapshot.ts: describeArg defaults to "true", so the Gemini path always triggers unless GEMINI_API_KEY is unset. Comment says "Not run automatically" but code disagrees. Gate on args.describe !== undefined or fix the comment.
2. sharp in root package.json — should be cli-only — Already a dep of packages/cli. Root-level means every bun install pulls the native binary for all packages. Remove from root.
3. fontkit.create() cast bypasses installed types — fontMetadataExtractor.ts casts through unknown with a hand-rolled interface instead of using @types/fontkit (already in devDeps). If fontkit's API changes, this silently produces undefined rather than a compile error.
4. escapeXml missing apostrophe entity — contactSheet.ts escapes &<>" but not '. Safe today (labels in double-quoted SVG attrs) but not general-purpose. Add or document.
5. Regex interpolation without escaping — agentPromptGenerator.ts contactSheetRows interpolates baseName into new RegExp(...) without escaping. Callers today pass controlled literals, but fragile for future callers.
6. Cookie-dismiss selector false-positive risk — screenshotCapture.ts button[id*="accept"] could click non-cookie buttons ("Accept invitation", etc.). Low severity since capture is read-only.
Clean Areas
fontMetadataExtractor.ts: Weight canonicalization well-designed. Per-font try/catch, corrupt →unidentified. Variable fontfvaraxis extraction correct.capture/index.ts: Non-fatal wiring (try/catch + log). Correct ordering.designStyleExtractor.ts: Clean singlepage.evaluate(). Reasonable sampling caps.verify-beats removal: Clean — no dangling imports.fonts.tslint rule + tests: Comprehensive coverage.composition.tsinvalid_capture_path: Correctly scoped, tests cover both cases.
Suggested stack split: (1) Font extractor + fontkit dep, (2) Contact sheet + screenshot + snapshot, (3) Lint rules, (4) Skill prose + assets + .gitignore
jrusso1020
reviewed
May 20, 2026
Collaborator
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 4fc05c63. Did a thorough read of the workflow per your request — but the PR's actual scope is meaningfully wider than the PR description claims, and that matters for review depth. Posting as COMMENT.
⚠️ The biggest issue: PR description vs actual scope
The PR body frames this as "two things landed": (1) font metadata extractor, (2) skill restructured around per-beat file reads. Housekeeping is mentioned briefly.
The actual diff (123 files / +15601/-1525) includes:
| Area | Files | Lines | In PR body? |
|---|---|---|---|
packages/cli (capture pipeline) |
14 | +1714/-85 | font extractor mentioned; contactSheet.ts (347L new) + designStyleExtractor.ts (282L new) NOT mentioned |
packages/core lint rules |
5 | +339/0 | NOT mentioned — new fonts.ts rule, new composition.ts rule for ../capture/ paths |
skills/hyperframes (the main authoring skill) |
61 | +9951/-567 | NOT mentioned at all — affects every hyperframes user, not just w2h |
skills/launch-video (brand-new skill) |
1 | +283/0 | NOT mentioned |
skills/website-to-hyperframes |
38 | +3241/-864 | This is the scope the PR body describes |
| SFX audio files | 13 binary MP3s + manifest | — | NOT mentioned |
| AGENT-FEEDBACK-V2.md / V4.md at repo root | 2 | — | NOT mentioned |
.env auto-load in cli.ts |
1 | — | NOT mentioned |
sharp bump at repo root |
1 | — | NOT mentioned |
That's substantially more than "two things + housekeeping." Future bisectors looking at the commit history won't infer what changed by reading the title or summary.
Concrete ask: rewrite the PR body to enumerate all the substantive surfaces. Or split into multiple PRs — the main hyperframes skill changes (the +9951) deserve their own PR with its own review since they affect every hyperframes user, not just the website-to-hyperframes flow.
⚠️ Author's own framing contradicts shipping
PR body says (verbatim): "The honest read of those evals: most of the workflow/skill restructuring did not move quality much." and "Everything else (DESIGN.md tweaks, examples library, verify-beats CLI gate, increasingly strict skill prose) produced mixed results."
Then it goes on to ship: a major skill restructuring (Step -1 / Step 5 / Step 6 read protocol), removed verify-beats CLI, new lint rules, new beat-direction prose, new motion principles, new typography page, new visual-vocabulary framework, etc.
Two reads:
- The honest framing is correct → ship only the things that did work (capture + snapshot improvements + font extractor) and drop the rest.
- The shipping decision is correct → the skill changes did help, the framing undersells them.
If (1), the PR is much smaller and easier to review. If (2), the PR body's pessimism is misleading reviewers about what's load-bearing. Worth picking a position.
⚠️ AGENT-FEEDBACK-V2.md + AGENT-FEEDBACK-V4.md at repo root
These look like eval/working-notes files committed to repo root (not under skills/.../evals/ or docs/ or notes/). They're listed in Copilot's file summary but not described in the PR body. Two options:
- They're meant to be permanent reference docs → move under
docs/orskills/website-to-hyperframes/evals/with naming consistent with the rest of that skill. - They're working notes for this PR → exclude from the commit.
Repo root is for top-level docs (README, CLAUDE.md, CONTRIBUTING.md, CREDITS.md, etc.); per-PR eval transcripts don't belong there.
⚠️ SFX audio file licensing
13 new .mp3 files under skills/website-to-hyperframes/assets/sfx/ (chime, click, key-press, glitch-{1,2,3}, impact-bass-{1,2}, notification, ping, pop, error, click-soft). Where do these come from? Per the vendored-content license-compliance pattern (same one that bit hf#971 with the 70k templates):
- If royalty-free / public domain: add a CREDITS entry or LICENSE-AUDIO file with source attribution.
- If purchased / licensed: confirm the license permits redistribution under the project's Apache 2.0.
- If AI-generated: still worth noting source + generation context.
13 MP3 files without provenance documentation is a small but real OSS-compliance gap.
Spot-checks I did do
fontMetadataExtractor.tsheadline claim: PR body says "132/132 fonts identified by real family name, including hashed Next.js builds". The file's 310-line shape (fontkitnametable read, weight canonicalization,fvaraxes) is the right approach. Without re-running the eval I can't verify the 132/132 number, but the method is sound..envauto-load incli.ts: implementation is the safe shape — only sets if!(key in process.env)(existing env wins), parses quoted values, silent on missing file. Same convention as thedotenvpackage. Local-dir trust risk is real but matches every other CLI tool's behavior. Acceptable.verify-beatsCLI removal: per the PR body, replaced with skill prose for per-beat file reads. PR claims "packages/cli/src/help.tsno longer lists the removed command" — verifiable, looks consistent in the diff.- Skill prose Step 5/6 read protocol: the "main agent opens each beat HTML and reads top-to-bottom" pattern is sound — a grep-based CLI gate would miss exactly the kind of quality issues the eval revealed. Document-anchored verification is a better fit. Risk: an agent that doesn't actually do the reads will still produce "looks good" reports. The skill prose addresses this by making the read protocol explicit, but enforcement is on the agent's discipline.
Per-Ular's request: workflow read
Spot-read the new w2h references/step-0-capture.md and step-5-build.md shape via Copilot's summary. The "cell-by-cell with one-sentence-per-cell rule" for contact-sheet review is a real anti-skim heuristic; the per-beat verdict template in step-6-validate (naming hex / asset paths / headline px / timeline coverage) is a concrete artifact-shape that reduces "looks good" sub-agent reports. These are good calls.
I did not read all 9951 lines of skills/hyperframes changes — that's the scope-vs-description issue above. If the main-skill changes are load-bearing for this PR's goal, please surface them in the PR body so reviewers can target them.
CI
mergeable_state: blocked per the PR API; required CI on this SHA worth a final eyeball before merge. The PR body's "fallow audit flags ~39 inherited complexity findings" preemptive note is fair — those predate this branch.
Verdict
Direction is sound — the font extractor lands a real win, the per-beat read protocol is the right replacement for the grep-shaped verify-beats gate, and the capture pipeline improvements are concrete. But the PR is two or three separate landings stitched into one, and reviewers can't catch what they don't know to look for. Strongly suggest splitting before merge:
- Capture pipeline + font extractor + lint rules — small, focused, easy to verify.
- website-to-hyperframes skill restructure — the workflow change Ular wants reviewed.
skills/hyperframesmain-skill changes — needs its own focused PR; affects every hyperframes user.launch-videoskill — new skill; deserves its own PR.
If splitting isn't feasible (89 commits of branch history is hard to untangle): at minimum update the PR body to describe what actually changed, and add SFX licensing + decide what to do with the AGENT-FEEDBACK files.
Not stamping per the per-PR merge policy. Holding any approval pending James's sign-off given Ular is non-trusted.
— Rames Jusso
Pre-stack fixes covering all 6 of Miguel's code-level issues, plus
the launch-video skill removal and SFX licensing note flagged by
Rames.
- snapshot.ts: `--describe` resolution was contradictory (comment
said opt-in, code defaulted to "true"). Now: runs by default,
`--describe "custom Q"` overrides the prompt, `--describe false`
opts out. Help text updated to match.
- package.json: removed `sharp` from root devDependencies. Already
declared in packages/cli; root-level was forcing every workspace
to pull the native binary.
- fontMetadataExtractor.ts: dropped the `as unknown as { ... }` cast
and the hand-rolled interface. Use `Font | FontCollection` from
`@types/fontkit` directly + a real type guard. `fsSelection` is
now typed (italic/oblique booleans, not a raw bitfield), so fontkit
API drift would surface as a compile error instead of silently
returning undefined.
- contactSheet.ts: `escapeXml` now also encodes `'` → `'` so
it's safe for both single- and double-quoted SVG attributes.
- agentPromptGenerator.ts: regex metacharacters in `baseName` are
now escaped before being interpolated into `new RegExp(...)`.
- screenshotCapture.ts: cookie-dismiss `button[id*="accept"]`-style
selectors are now scoped under a `[id*="cookie"]` / consent / gdpr
ancestor. Stops false-positives like "Accept invitation".
- skills/launch-video/: removed. Not part of the website-to-hyperframes
scope.
- skills/website-to-hyperframes/assets/sfx/CREDITS.md: documents
Pixabay provenance + license for the 20 bundled .mp3 files.
3 tasks
Collaborator
Author
|
Closing in favor of a 5-PR Graphite stack — same changes carved into independent, reviewable chunks per Miguel's and Rames's review feedback. Stack (bottom → top, each PR stacked on the previous one):
All 6 of Miguel's code-level issues from this PR's review are addressed in the stack (see the stack commit history). Rames's scope-vs-description concern is addressed by giving each surface its own PR with its own focused description. The launch-video skill is dropped; the AGENT-FEEDBACK / HANDOFF / studio-check session-ephemera files are dropped; the root-level Both reviewers please look at the stack instead of this PR. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two things landed:
nametable viafontkit, producescapture/extracted/fonts-manifest.jsonwith real family names, weights, and variable-font axes. Hashed Next.js/Webpack font filenames (f9b8e1e8d4c3f0a7-s.woff2) now resolve to "Inter", "Geist Mono", etc. so DESIGN.md gets the right font names instead of guesses.compositions/beat-N.htmland reads it top-to-bottom against DESIGN.md and STORYBOARD.md before advancing.Plus housekeeping:
.gitignorecatches the per-brand video project dirs agents leave at repo root, examples library reverted (was being copy-pasted instead of inspiring), captured-asset usage rules dialed back from over-strict "compose from divs only" to "captured SVGs/illustrations carry beats."Why
The pipeline-quality goal of this branch was: make sub-agents produce higher-quality videos. 11 eval rounds across this branch's history tested different configurations (DESIGN.md gate, STORYBOARD.md format, critic sub-agent, sub-agent fan-out, visual-vocabulary file, examples library, etc.). The honest read of those evals: most of the workflow/skill restructuring did not move quality much. Videos were still slideshow-ish, still missed brand assets, still got "looks good" reports from sub-agents that hadn't done the work.
Two things from the branch DID land real wins:
descriptions.md.Everything else (DESIGN.md tweaks, examples library, verify-beats CLI gate, increasingly strict skill prose) produced mixed results and at one point over-corrected so hard that agents stopped using captured SVGs/illustrations entirely. This PR dials those back to "use the captured assets, the main agent verifies by opening files."
The
verify-beatsCLI was an attempt to close the "sub-agent reports lies" failure mode by grep-checking claims against composition HTML. After implementation it became clear the structural lies it caught aren't the real quality problem — boring beats, off-screen logos, misaligned camera moves, GSAP timelines that only cover the first 2 seconds of a 5-second beat are what kill the video, and a grep can't detect any of them. The fix is the main agent actually reading each beat file, which the new Step 5 + Step 6 prose now requires.How
CLI changes (
packages/cli/):src/capture/fontMetadataExtractor.ts(new) — reads the OpenTypenametable from each captured font binary viafontkit. Canonicalizes static-weight family-name packaging ("Inter Medium" → "Inter" with weight 500, "Semi Bold" → "SemiBold"), readsfvarfor variable-font axes. Writescapture/extracted/fonts-manifest.jsonwith per-file metadata and per-family aggregates.src/capture/index.ts— calls the extractor after the existing font-download step.src/cli.ts+src/help.ts— no new commands; the bootstrap IIFE for worker-thread entry resolution stays.package.jsonaddsfontkit ^2.0.4+@types/fontkit.src/commands/verify-beats.ts— removed (340 lines). Replaced with skill prose.Skill changes (
skills/website-to-hyperframes/):SKILL.md— concept-first manifesto in Step -1; captured assets framed as first-class beat content. Step 5 gate is now "main agent opens each beat HTML and reads it top-to-bottom" — no CLI command.references/step-0-capture.md— contact-sheet viewing is now "every cell, name 5 assets per page before moving on" (closes the "agents glanced then wrote beats referencing assets that didn't exist" mode).references/step-1-design.md— restored component CSS sections (3 Component Stylings, 4 Spacing & Layout, 5 Depth & Elevation) over-collapsed in earlier batches.references/step-3-storyboard.md— concept gate at the top; brand-floor MUST rules (logo in opener + closer); captured assets first-class.references/step-5-build.md— "Run verify-beats" section replaced with a per-beat read protocol: cross-check brand hex / captured assets / headline size / GSAP timeline coverage / technical gates against DESIGN.md + STORYBOARD.md.references/beat-builder-guide.md— "Step 5: Write the verification artifact" (verify.json schema + field requirements) removed. Sub-agents now report back with concrete observations; main agent verifies by opening the file. "Patterns that ARE shots" affirmative list added. Webpage-mimicry patterns (CSS browser chrome, full app layout) downgraded from ❌ to ⚠ — fine when the storyboard genuinely calls for them as the shot subject.references/step-6-validate.md— Definition of Done checklist now leads with "Every beat HTML read top-to-bottom" + per-beat verdict template that names hex codes, asset paths, headline px, timeline coverage, storyboard alignment. Contact-sheet review is cell-by-cell with one-sentence-per-cell rule. Critic sub-agent scores a "Captured asset utilization" dimension.d001c7bb) — agents were copy-pasting instead of getting inspired. Moved to a separate branch in case any scenes are worth resurrecting later.Other:
.gitignorecatches per-brand video project dirs at repo root (huly-*/,raycast-*/,*-demo-*/,test-runs/, etc.).CLAUDE.mdHANDOFF banner removed.Test plan
fontMetadataExtractorexercised against 9 captures: 132/132 fonts identified by real family name, including hashed Next.js builds.npx tsx packages/cli/src/cli.ts capture <url>writesfonts-manifest.jsonwith correct family aggregations. Local skill prose was read end-to-end to confirm no danglingverify-beats/verify.jsonreferences remain.references/*.mdupdated to remove all verify-beats wiring and add the per-beat read protocol.packages/cli/src/help.tsno longer lists the removed command.Note on CI: the branch carries 83 commits of evaluation work; fallow's audit gate flags ~39 inherited complexity findings in
packages/cli/src/capture/*,packages/cli/src/commands/snapshot.ts,packages/cli/src/commands/transcribe.ts, andpackages/core/src/lint/rules/composition.ts. These predate this session and are out of scope for this PR — they should land as a separate refactor pass with proper test coverage. My session's new code (fontMetadataExtractor) is clean.