Merge pull request #1672 from heygen-com/fix/skill-authoring-fixes

fix(skills): harden music-to-video Step 3 gate, align product-launch-video with pr-to-video

Author: WaterrrForever

skills/music-to-video/SKILL.md

@@ -63,7 +63,7 @@ python3 <SKILL_DIR>/scripts/analyze-beatgrid.py "$PROJECT_DIR/assets/bgm.mp3" \
 
 ---
 
-## Step 2: Frame skeleton
+## Step 2: Frame skeleton (structure only)
 
 Goal: Read the music and lay out the frames — the skeleton of `STORYBOARD.md`.
 
@@ -73,7 +73,7 @@ Read [`references/frame-skeleton.md`](references/frame-skeleton.md). Turn `audio
 
 ---
 
-## Step 3: Plan (user-gated)
+## Step 3: Fill the plan (user-gated)
 
 Goal: Turn the skeleton into an approved, complete `STORYBOARD.md`.
 
@@ -93,7 +93,7 @@ Fix every `✗` (hard errors: duration mismatch, frames not tiling the track, a
 
 ---
 
-## Step 4: Build frames
+## Step 4: Build frames from the plan
 
 Goal: Build every frame as a self-contained composition file.
 

skills/music-to-video/scripts/validate-plan.mjs

@@ -4,7 +4,10 @@
 // not exist yet), so it checks fields, not on-disk html.
 //
 // HARD (exit 1): frontmatter duration_s == audiomap duration; >=1 frame; each frame
-//   has src + positive duration; frames tile the track gap-free (sum == duration_s).
+//   has src + positive duration; frames tile the track gap-free (sum == duration_s);
+//   every frame has >=1 filled group (a frame whose `### Groups` is still
+//   `TBD (Step 3)`/empty is a Step-2 skeleton, not an approved plan — fail it so a
+//   skipped Step 3 can never reach the build step).
 // WARN (exit 0): best-effort group checks — each group exactly one of
 //   template/free_design/asset; a template id exists under the --templates dir;
 //   phrase_flow frame has no beat_cut asset treatment.
@@ -114,12 +117,18 @@ for (const ln of raw.split(/\r?\n/)) {
   if (cur) cur.lines.push(ln);
 }
 
+// HARD: every frame needs >=1 filled group. A frame with no parseable group head
+// is still a Step-2 skeleton (`### Groups` = `TBD (Step 3)`/empty) — erroring here
+// is what stops a skipped Step 3 from reaching the build step.
 const framesWithGroups = new Set(blocks.map((b) => b.frameLabel));
 for (const f of frames) {
+  const title = String(f.title ?? "").trim();
   const lbl = `Frame ${f.number ?? ""} — ${f.title ?? f.index}`.replace(/\s+—\s+$/, "");
-  if (![...framesWithGroups].some((s) => s.includes(String(f.title ?? "")))) {
-    warns.push(
-      `${lbl}: no parseable groups (expected \`- **gN** — template|free_design|asset …\`)`,
+  const hasGroup = title !== "" && [...framesWithGroups].some((s) => s.includes(title));
+  if (!hasGroup) {
+    errors.push(
+      `${lbl}: no filled groups — still a Step-2 skeleton (\`### Groups\` is TBD/empty). ` +
+        `Run Step 3: fill its groups (\`- **gN** — template|free_design|asset …\`) before building.`,
     );
   }
 }

skills/music-to-video/sub-agents/frame-worker.md

@@ -34,6 +34,7 @@ If your dispatch carries lint / validate feedback from a prior pass, address eac
 
 ## What comes fixed — realize it as given
 
+- **No plan = stop.** If your `### Groups` is `TBD`/empty (Step 3 was skipped), report back and write nothing — never invent groups, templates, or copy.
 - **The plan** is set in your `## Frame` block: the groups, templates / primitives, copy, brand,
   and anchors. Build it as written. If a plan is genuinely wrong (wrong template or copy), stop
   and report — the orchestrator re-plans at Step 3.

skills/product-launch-video/SKILL.md

@@ -155,12 +155,14 @@ Inject transitions, run checks, pause for review, then render.
 
 `npx hyperframes validate`
 
-`npx hyperframes inspect --strict-layout`
+`npx hyperframes inspect`
 
 `npx hyperframes snapshot --at <frame-midpoints>`
 
 If a command fails, surface stderr and stop. Do not pile on recovery commands. If a gate names a frame, fix `compositions/frames/NN-*.html` with the cheapest safe fix: edit the frame HTML for a local issue; re-dispatch the frame worker only when the whole shot must be rebuilt.
 
+**Known false-positive — do not chase it.** `inspect` may report a handful of `text_box_overflow` errors of ~1–4px on the **caption** highlight words (selector `#caption-word-*` / `.caption-line`). The caption pill uses a deliberately snug `line-height` (set once in `scripts/captions.mjs`) and has **no `overflow:hidden`**, so a heavy display glyph's ink spills a few px into the pill's own padding — nothing is actually clipped. Treat these as expected and proceed. Do **not** inflate the caption `line-height` (it balloons the pill, which is worse) and do **not** re-dispatch a frame for them. Only act on a `text_box_overflow` when it names a **frame** element (`#el-NN-*`), not a caption word.
+
 After checks pass, pause for user review. The video is assembled, viewable, and editable in Studio. Manage preview only once across Step 3 and Step 6: open it if the user asked earlier, offer it if they declined earlier, and do not ask again if they are already reviewing in Studio.
 
 Preview: `npx hyperframes preview`

skills/product-launch-video/sub-agents/frame-worker.md

@@ -19,7 +19,7 @@
 
 **Retry** — if your context carries lint / validate feedback from a prior pass, read it first and re-author so none of those findings recur; treat each as a hard constraint.
 
-**OUTPUT** — `compositions/frames/<frame_id>.html`, one self-contained sub-composition. Writing it (past the self-check) is your **terminal action** — you do not edit `STORYBOARD.md`, mint audio, assemble the index, or report back. The orchestrator picks up the file and marks the frame's `status`.
+**OUTPUT** — `compositions/frames/<frame_id>.html`, one self-contained sub-composition. Writing it (past the self-check below) is your **terminal action** — you do not edit `STORYBOARD.md`, mint audio, assemble the index, run the CLI, or report back. The orchestrator picks up the file and marks the frame's `status`.
 
 ## You do NOT decide
 
@@ -48,15 +48,16 @@ Generic seek-safety + structure live in `hyperframes-core` (read it; not restate
 
 1. **Read** — `hyperframes-core`'s composition contract (the structural law), then `frame.md` (the look) and your `## Frame N` block (content + effects / blueprint / assets). **Then open the recipe body of every id the block cites** — `ANIM_DIR/rules/<id>.md` per effect and `ANIM_DIR/blueprints/<id>.md` for the blueprint (plus its linked `examples/<id>.html` when the recipe is unclear): you reproduce these, not improvise them. Internalize the self-check codes below before you write — most lethal is **template transport**: every `<style>` + `<script>` (including the gsap load) must live INSIDE `<template>`, because the runtime only clones template contents and `lint` / `validate` / `inspect` can miss the resulting blank sub-composition.
 2. **Design** — translate `scene` + the (sometimes shot-by-shot) narrative + the recipes you just read into a visual plan using `frame.md`'s components and type ramp. Honor the note's phases shot-by-shot per the whole-shot rule above, and find a visual idea that reinforces the beat, not a literal restyle of the words. Place the named assets.
-3. **Author** — write the full sub-composition to `compositions/frames/<frame_id>.html` (rewrite to iterate; last write wins). `<template>`-wrapped root carrying `data-composition-id="<frame_id>"`, exactly one `gsap.timeline({ paused: true })` registered at `window.__timelines["<frame_id>"]`, built synchronously — per the core contract.
-4. **Self-check, then finish** — run the checklist below and fix in place. Writing the passing file is your terminal action.
+3. **Author** — write the full sub-composition to `compositions/frames/<frame_id>.html` (rewrite to iterate; last write wins). `<template>`-wrapped root carrying `data-composition-id="<frame_id>"` and styled via `#root` (not a class on that element — see the self-check below), exactly one `gsap.timeline({ paused: true })` registered at `window.__timelines["<frame_id>"]`, built synchronously — per the core contract.
+4. **Self-check, then finish** — re-read your file against the checklist below and fix in place. Writing the file is your terminal action; you do **not** run the CLI.
 
-## Self-check (fix before finishing)
+## Self-check before finishing (you do NOT run the CLI)
 
-The orchestrator runs `lint` / `validate` / `inspect`; catch these yourself first (codes are `hyperframes lint`'s; the rules behind them are in `hyperframes-core`):
+You **can't** meaningfully run `hyperframes lint` / `validate` / `inspect` here: they operate on the **assembled project** (the `index.html` graph / bundle), and your frame isn't wired in yet — so they report on _other_ files, not yours (a false green). The **orchestrator** runs them at **Step 6, after assembly** (the correct unit), and **re-dispatches you with the finding** if your frame fails (see **Retry** above). So get it right on write: re-read your file against this checklist before finishing — the codes in parens are `hyperframes lint`'s and what the orchestrator may cite back (the rules behind them live in `hyperframes-core`):
 
 - `missing_template_wrapper` / `missing_composition_id` — root is `<template>`-wrapped and carries `data-composition-id="<frame_id>"`.
 - **Template transport** — every `<style>` and `<script>` block, including the GSAP load, lives inside `<template>`.
+- `subcomposition_root_styled_by_class` — **style the frame root via `#root`, never a class on the `data-composition-id` element**: at render a class on the root gets scoped to a descendant selector that can't match it, so the **whole scene renders unstyled** (Studio preview still looks right — trust this rule, not the preview). Descendants use plain selectors.
 - `clip_missing_data_attrs` — every `class="clip"` element has `data-start` / `data-duration` / `data-track-index`.
 - `timeline_not_paused` / `timeline_not_registered` — one paused timeline, registered at `window.__timelines["<frame_id>"]`.
 - `css_transition_used` + repeat / yoyo / non-deterministic logic — none present (the renderer seeks frame-by-frame).