Skip to content

docs(prompting): document the intent interview and align pages with skill contracts - #2872

Open
WaterrrForever wants to merge 6 commits into
mainfrom
miao/prompt-guide-intent-layer-docs
Open

docs(prompting): document the intent interview and align pages with skill contracts#2872
WaterrrForever wants to merge 6 commits into
mainfrom
miao/prompt-guide-intent-layer-docs

Conversation

@WaterrrForever

@WaterrrForever WaterrrForever commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Rebased onto main after #2109 merged (6c185f252). Now targets main directly — the docs/prompting/ pages this PR edits landed with that merge.

What

  • Add "The interview: what the agent asks first" to the prompting overview: the route confirm, the must-have questions, the two run-shape questions (storyboard; automation vs companion) with the four-valid-combinations note, the "just build it" skip, and BRIEF.md as the artifact later sessions resume from. The run-shape paragraph is scoped to the routes that actually apply them — /motion-graphics, /slideshow, /embedded-captions, and /talking-head-recut are Run-shape: neither in route-briefs.md and are named as skipping both.
  • Add a note on the storyboards page separating the plan this page teaches from the interview's storyboard question — that answer changes the review process, not the route.
  • Make BRIEF.md the pipeline guide's Step 3 artifact (step table, project tree, step body, gate, iterating list), and fix SCRIPT.md's mislabeled step in the tree (Step 4, not 3).
  • Correct the workflow one-liners in the setup tables against each SKILL.md contract: general-video leads with its positive identity and companion mode; faceless-explainer keys on invented visuals instead of TTS; talking-head-recut uses its "graphic overlays" trigger term; motion-graphics gains an input side and its transparent-overlay output; music-to-video stops implying images are required.
  • Rework the vocabulary page's video galleries into a responsive CSS grid (repeat(auto-fit, minmax(200px, 1fr)), 16:9, collapses on narrow screens). The old 4-5-column tables rendered as unreadable black boxes. All 18 clips keep main's deliberate playback behavior — controls muted loop playsinline preload="metadata" plus #t=0.1 first-frame thumbnails, no autoplay on multi-clip galleries.
  • Realign docs/quickstart.mdx with the skills catalog: --full-depth on both install commands (with the reason inline), check in the /hyperframes-cli row plus a validate step in the manual dev loop, /hyperframes-keyframes in the core table, /figma in the workflow list.
  • Rename "companion media pipeline" to "sibling media pipeline" in media-and-audio — companion is now a reserved term.

Why

The intent layer (BRIEF.md, flow, storyboard) is the front door of every skills run, but the guide never prepared readers for it: no page explained the interview, companion vs automation, or the file that makes a brief resumable. The catalog one-liners had also drifted from the SKILL.md descriptions they compress — including one that inverted a contract (music-to-video requires zero user assets).

The quickstart drift is separate and older: it had never been updated when docs/guides/skills.mdx, CLAUDE.md, and README.md gained --full-depth, /hyperframes-keyframes, and /figma. Two of the four items are correctness, not tidiness — without --full-depth a reader installs skills that lag main by hours, and a dev loop taught without check contradicts the overview calling it "the step people skip and regret."

Test plan

  • All 248 docs MDX files compile via @mdx-js/mdx — 0 failures
  • Internal links and heading anchors checked repo-wide; the 11 pre-existing broken /packages/cli#* anchors match origin/main exactly, no new ones introduced
  • #the-interview-what-the-agent-asks-first resolves from both cross-links (storyboards.mdx, guides/pipeline.mdx)
  • Documentation updated

@vanceingalls

Copy link
Copy Markdown
Collaborator

Reviewed against the current fix/prompt-guide-validation-bugs tip (8fb1482e7). The content is accurate against the SKILL.md contracts and reads well — the interview section in particular fills a real gap, and making BRIEF.md the Step 3 artifact fixes a genuine inconsistency in the pipeline guide.

Two things need a rebase, both caused by commits that landed on the base branch after you branched (you're based at 9cbab0c78, which is 19 commits back). Nothing here is a disagreement with your changes.

1. Conflict in docs/prompting/overview.mdx

GitHub now reports CONFLICTING / DIRTY. Both hunks are purely additive — we each added something in the same place, so the resolution is "keep both". The other five files auto-merge clean.

Hunk 1 — core-skills table. The base added a /hyperframes-keyframes row next to the /general-video row you rewrote. Keep both rows, with your wording for /general-video:

| `/hyperframes-keyframes`| Seek-safe keyframe authoring across runtimes, plus `hyperframes keyframes` diagnostics |
| `/general-video`        | The general authoring workflow — multi-scene pieces, reels, montages, remixes, and the home of **companion mode**; the fallback when no workflow below fits |

Hunk 2 — after the two prompt shapes. The base added a sentence about verified renders; you added ## The interview. Ours first, then yours:

The four prompts above illustrate shape, not results — every prompt in this guide that ships with an embedded render was run exactly as written, and the gallery of those lives in [Verified examples](/prompting/examples).

## The interview: what the agent asks first

I resolved both locally and confirmed all 34 MDX files still compile via @mdx-js/mdx.

2. Four drift items in docs/quickstart.mdx

Your PR touches this file, so these are yours to pick up rather than something I should fix underneath you. All four are base-branch changes your branch predates:

Line Currently Should be
install commands (2×) npx skills add heygen-com/hyperframes append --full-depth
/hyperframes-cli row init, lint, preview, render, doctor add check
core-skills table 8 rows add /hyperframes-keyframes
workflow list 10 workflows add /figma

Two of these matter beyond tidiness:

  • --full-depth — without it, skills add fetches the skills.sh registry blob, which lags main by hours. A user following the quickstart gets stale skills.
  • check — the motion chapter now states that both lint and check must pass before rendering, and calls check "the step people skip and regret". The quickstart teaching the dev loop without it contradicts that directly.

Clean

No design.md references reintroduced (base renamed those to frame.md), no skill-count claims to drift, /figma survives in overview.mdx, and nothing in your copy restates the motion rules that were rewritten on the base branch — so the rebase should be conflict-free after the one file above.

Reviewed with Claude Code.

@vanceingalls

Copy link
Copy Markdown
Collaborator

Follow-up against the current stacked base (fix/prompt-guide-validation-bugs at fdf3ad8f). There have been no new commits or review comments here since my previous review, so its four quickstart.mdx drift items are still outstanding. The base has advanced, though, and the merge surface now needs two additional resolutions.

Additional adjustments

  1. The rebase now conflicts in three files, not one.

    • docs/prompting/overview.mdx: resolve as previously described — keep /hyperframes-keyframes, keep this PR's richer /general-video wording, and put the verified-examples sentence before ## The interview.
    • docs/prompting/storyboards.mdx: keep both additions. The base's sentence clarifying that the narrative vocabulary is not extra STORYBOARD.md schema should remain, followed by this PR's opening-interview <Note>.
    • docs/prompting/vocabulary.mdx: keep this PR's readable CSS-grid layout, but preserve the base's video behavior on all 12 conflicted clips: controls muted loop playsinline preload="metadata" and #t=0.1. Do not take this PR's autoPlay version wholesale — the base deliberately avoided autoplay on multi-clip galleries while adding first-frame thumbnails. The grid should also collapse on narrow screens (for example, repeat(auto-fit, minmax(200px, 1fr))) rather than staying fixed at three columns.
  2. Qualify the run-shape wording in overview.mdx. The new paragraph currently says the agent always closes with the Storyboard and Automation/Companion questions. The current contract says those are asked only “where the route's entry applies them.” /motion-graphics, /embedded-captions, /talking-head-recut, and /slideshow explicitly skip those two questions. Suggested framing: “When the selected workflow supports both run shapes, it then closes with two questions…”

After the rebase, please run the normal formatting and docs validation/broken-link checks. The current PR checks are from the unreconciled head and mostly skipped, so they do not validate the eventual merged result.

Base automatically changed from fix/prompt-guide-validation-bugs to main July 30, 2026 12:09
WaterrrForever and others added 5 commits July 30, 2026 20:31
general-video leads with its positive identity and companion mode;
faceless-explainer keys on invented visuals instead of TTS;
talking-head-recut uses the 'graphic overlays' trigger term;
motion-graphics gains its input side and overlay output;
music-to-video stops implying images are required.
Replace the 4-5 column table hack with a 3-column CSS grid,
switch demo clips to autoplay muted loops (no black poster frame,
no player chrome over tiny videos), and align cells at 16:9.
The guide taught prompt shapes but never prepared readers for the
conversation that follows: the intent interview, the two run-shape
questions (storyboard, automation vs companion), the just-build-it
skip, and BRIEF.md as the resumable artifact. Add that section to the
overview, a disambiguation note on the storyboards page, and free up
'companion' as a reserved term in media-and-audio.
Step 3 (Strategy & Messaging) listed no output while describing
exactly what BRIEF.md now captures. Name the artifact in the step
table, project tree, step body, gate, and iterating list, and fix
SCRIPT.md's step label in the tree (Step 4, not 3).
The quickstart drifted from docs/guides/skills.mdx, CLAUDE.md, and the
prompting overview — it had never been updated when those surfaces were:

- `--full-depth` on both install commands, with the reason inline. Without
  it `skills add` fetches the skills.sh registry blob, which lags `main` by
  hours, so a reader following the quickstart installs stale skills.
- `check` in the `/hyperframes-cli` row, and a validate step in the manual
  dev loop, which went preview → render with no gate at all. The prompting
  overview calls `check` "the step people skip and regret" and states both
  `lint` and `check` must pass before rendering.
- `/hyperframes-keyframes` in the core-skills table (8 rows → 9).
- `/figma` in the optional-workflow list (10 → 11).

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

mintlify Bot commented Jul 30, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
hyperframes 🟢 Ready View Preview Jul 30, 2026, 12:32 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@WaterrrForever

Copy link
Copy Markdown
Collaborator Author

Rebased onto main at 6c185f252 (post-#2109). Head is now d3b0430e6, GitHub reports MERGEABLE. All three conflicts resolved as you described, plus both additional items. I'm not merging — leaving that to you.

Conflict resolutions

  • overview.mdx — kept both: the /hyperframes-keyframes row stays, /general-video takes this PR's companion-mode wording, and the verified-examples sentence precedes ## The interview.
  • storyboards.mdx — kept both: base's "not additional STORYBOARD.md schema" sentence, then this PR's <Note>.
  • vocabulary.mdx — CSS-grid layout with base's playback behavior preserved on every clip: controls muted loop playsinline preload="metadata" and #t=0.1, no autoPlay. Grid is repeat(auto-fit, minmax(200px, 1fr)) so it collapses on narrow screens.

One thing I did beyond your list, flagging it so you can veto. The ease gallery (6 clips, the first one on the page) wasn't in the conflict, so leaving it as-is would have left three galleries on one page in two different layouts. I converted it to the same grid, with the same base playback attributes and #t=0.1. If you'd rather it stay a table, that's a separate edit — it won't come back by reverting anything here.

Run-shape wording

Qualified as suggested, and I verified the claim against references/route-briefs.md: /motion-graphics, /slideshow, /embedded-captions, and /talking-head-recut are all Run-shape: neither. The paragraph now reads "When the selected workflow supports both run shapes, it then closes with two questions…", names those four and why each skips, and the "just build it" line gained "wherever those apply." (/remotion-to-hyperframes is a third case — not served by the intent layer at all — so it's not in that list.)

quickstart.mdx — one correction to the framing

All four items are picked up, in their own commit. But they aren't base-branch changes this PR predates: quickstart.mdx on fix/prompt-guide-validation-bugs didn't have them either. --full-depth, /hyperframes-keyframes, and /figma have been in docs/guides/skills.mdx, CLAUDE.md, and README.md since #2554 / #2133 — the quickstart simply never got updated alongside them. Same conclusion, but worth noting in case you were diffing base-vs-branch to find them.

Also beyond the literal ask: Option 2's manual dev loop went preview → render with no validation step at all, which contradicts "the step people skip and regret" harder than the /hyperframes-cli row did. Added a "Validate before rendering" step with lint + check.

Validation

  • All 248 docs MDX files compile via @mdx-js/mdx — 0 failures.
  • Internal links and heading anchors checked repo-wide. 11 broken /packages/cli#* anchors remain; I ran the same check against origin/main as a baseline and the two lists are identical, so nothing new here. (Those look like a real pre-existing issue on main — the CLI reference page doesn't carry per-command headings that 7 pages link to. Separate PR if you want it fixed.)
  • #the-interview-what-the-agent-asks-first resolves from both cross-links.
  • oxfmt doesn't cover .mdx (ignored), and no code files changed.

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2872 Review: document the intent interview and align pages with skill contracts

Overview

A clean follow-up to #2109: 79 additions, 27 deletions across 6 docs files. Three workstreams: (1) document the opening interview and run-shape questions, (2) fix drifted workflow one-liners against the actual SKILL.md contracts, (3) fix the vocabulary page video grids that rendered as black boxes.


1. Workflow one-liner corrections — verified

Spot-checked each corrected description against the on-disk SKILL.md:

  • /faceless-explainer: SKILL.md says "every visual is invented (typography, abstract graphics, diagrams, data-viz) rather than captured. There is no URL, no website capture, and no real assets." The new one-liner ("no URL, no footage → faceless explainer — every visual invented") matches exactly. The old one ("Arbitrary text → faceless explainer with its own TTS narration") led with TTS, which is a side effect, not the identity.
  • /motion-graphics: SKILL.md confirms "Can render to MP4 or transparent overlay" and lists input triggers ("a logo / stat / tweet / brief"). Both now reflected in the one-liner. Good.
  • /graphic-overlays (listed as /talking-head-recut): SKILL.md opens with "Package an existing talking-head / interview / podcast video by layering timed, designed GRAPHIC OVERLAY cards" — the new one-liner uses "graphic overlays (kinetic titles, lower-thirds, PiP)", which matches the trigger term. The old "designed graphic cards" was vague enough to confuse with the caption-style workflow.
  • /music-to-video: The old one-liner ("A music track + your images") implied images were required input. The new one ("your images optional, cut on the beat") is correct — the music is the required input, images are supplementary.
  • /general-video: Now leads with its positive identity ("multi-scene pieces, reels, montages, remixes") and names companion mode. Clean.

One pre-existing note (not a regression from this PR): the overview table uses /talking-head-recut as the slash command, but the actual skill directory on disk is graphic-overlays. This naming mismatch was inherited from #2109. Worth a follow-up to align — or confirm an alias exists in the router.

2. Video grids — correct fix

The old <table><tbody><tr><td>... layout gave each video cell no intrinsic sizing, producing the "tiny black boxes" Miao described. The CSS grid replacement is clean:

  • repeat(auto-fit, minmax(200px, 1fr)) — responsive 3-column at desktop, collapses gracefully on mobile.
  • aspectRatio: "16 / 9" + objectFit: "cover" — forces the right shape even before the video loads.
  • display: "block" — eliminates the inline baseline gap below video elements.
  • #t=0.1 fragment preserved on all URLs — shows a first-frame thumbnail instead of a black poster.
  • All 18 clips keep the same playback attributes (controls muted loop playsinline preload="metadata").

The same grid pattern is applied consistently to all three galleries (easing, camera, marker). No visual regression risk.

3. Interview section — well-scoped

The new "The interview: what the agent asks first" section documents:

  • The route-confirm → must-have questions → run-shape questions flow
  • The storyboard/automation-vs-companion 2×2 with all four combinations valid
  • The four routes that skip both questions (/motion-graphics, /slideshow, /embedded-captions, /talking-head-recut) — correctly scoped
  • The "just build it" skip pattern
  • BRIEF.md as the resumable artifact (not the chat)

The section is placed correctly — after the "two prompt shapes" section and before the recommended workflow, so readers understand the conversation before they hit the build steps.

4. BRIEF.md pipeline integration — clean

All five touchpoints updated consistently:

  • Step table: Step 3 artifact column now reads BRIEF.md
  • Project tree: BRIEF.md added, SCRIPT.md relabeled from Step 3 → Step 4
  • Step body: output line updated
  • Gate: now names the file (BRIEF.md exists)
  • Iterating list: new bullet for intent changes

The pipeline page now has a single source of truth for what Step 3 produces.

5. Quickstart alignment — needed

Four corrections, two of which are correctness:

  • --full-depth — without it, readers install stale skills from the registry blob. Correctness.
  • check step added to the manual dev loop — the overview page calls it "the step people skip and regret" while the quickstart had preview → render with no gate. Correctness.
  • /hyperframes-keyframes in core table — aligns with overview. Tidiness.
  • /figma in workflow list — aligns with overview. Tidiness.

6. "companion" → "sibling" rename — correct

media-and-audio.mdx renames "companion media pipeline" to "sibling media pipeline" — freeing "companion" as a reserved term for the run-shape concept. One-word change, no content drift.


Verdict

Approve. Every workflow one-liner correction is verified against the actual SKILL.md contract. The video grid fix is a clean CSS upgrade. The interview section fills a real documentation gap (the intent layer) without overreaching. The quickstart alignment fixes two genuine correctness issues. CI clean (docs-only, relevant checks pass).

— Miga

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additive exact-head stamp at d3b0430e.

I audited all six changed MDX files against the current skill contracts and the intent/brief references. The interview text preserves the two independent run-shape questions and all four combinations, the four skip-both routes match their route files, and the BRIEF.md lifecycle is consistent across the overview, storyboard, and seven-step pipeline. The quickstart additions match the CLI contract (check covers runtime, layout, motion, and WCAG contrast), and the workflow one-liners match their SKILL.md descriptions.

The vocabulary layout rewrite preserves all 18 source URLs and every playback attribute while adding a responsive 16:9 grid. git diff --check is clean, Mintlify validation and broken-link checks passed, and every current-head check is terminal. The visible cancelled Format entry is from duplicate CI run 12029, cancelled by concurrency; run 12030 reran the same exact head and Format succeeded.

One correction to the existing review: the claimed /talking-head-recut versus graphic-overlays naming mismatch is not present at this head. skills/talking-head-recut/SKILL.md exists, declares name: talking-head-recut, and the router uses the same route, so no follow-up is needed for that point.

Verdict: APPROVE
Reasoning: The docs now reflect the shipped interview, workflow, pipeline, validation, and gallery contracts without introducing a source or rendering inconsistency; exact-head CI has a successful replacement for the cancelled duplicate run.

— Magi

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs-only change, six files. I read all six in full at this head plus the sources they compress: every workflow SKILL.md frontmatter, all ten skills/hyperframes/references/routes/*.md entries, references/intent-interview.md, hyperframes-core/references/brief-contract.md, and the three catalog surfaces (docs/guides/skills.mdx, README.md, CLAUDE.md).

Additive to @miga-heygen's and @miguel-heygen's reviews — two items neither covered, plus an independent read on the graphic-overlays question.

Strengths

  • docs/prompting/overview.mdx:120 is the claim I expected to be the weak point, and it holds exactly. Four routes carry Run-shape: neither in the router's own entries — routes/motion-graphics.md:10, routes/slideshow.md:10, routes/embedded-captions.md:11, routes/talking-head-recut.md:11 — and they are precisely the four named, with the reasons lifted near-verbatim from those files ("the deliverable is a navigable deck, not a rendered video"; "the footage is untouched; there is no storyboard to review"). Same for the rest of the section: the two questions match intent-interview.md §(a)/(b), the "just build it" line at :126 matches the signal that locks flow: automation, storyboard: no, and the BRIEF.md frontmatter keys at :128 match the carry-away table (workflow, flow, storyboard, message, then audience / length among the registry fields). Prose that quotes its source of truth is prose that doesn't silently drift.
  • docs/prompting/vocabulary.mdx fixes the cause, not the symptom. The old markup put a width: 100% video inside a shrink-to-fit <td>, so the percentage had no definite width to resolve against and the cells collapsed. The grid gives each track a definite size (minmax(200px, 1fr)) and a definite height (aspectRatio: 16 / 9) — no force-sizing involved. Verified at this head: zero <table> and zero <td> left on the page, 18 videos, 18 with #t=0.1, 18 carrying controls muted loop playsinline preload="metadata", zero autoPlay — the base's playback contract preserved on all of them, not only the twelve that conflicted. I also probed the clips (ease-power2-out, ease-sine-inout, camera-push-in, camera-orbit, marker-highlight): all 960×540, so objectFit: "cover" against a 16:9 box crops nothing. And the deployed preview serves grid-template-columns:repeat(auto-fit, minmax(200px, 1fr)), so the style survived MDX rather than only compiling.
  • docs/quickstart.mdx:16 and :225-231 are correctness, not tidiness, and both match their sources: check is in skills/hyperframes-cli/SKILL.md's command list (which also records validate / inspect / layout as deprecated aliases for it), and the --full-depth rationale is near-verbatim from docs/guides/skills.mdx:29.

on the suggested graphic-overlays follow-up

Independently checked and it does not reproduce, converging with @miguel-heygen: graphic-overlays appears zero times in the repo tree at this head, the directory is skills/talking-head-recut/, and the router's entry is references/routes/talking-head-recut.md. /talking-head-recut in these tables is already correct — no follow-up needed.

nit — the fix is right, but the mechanism that produced the drift is untouched, so it will recur

docs/guides/skills.mdx:114 states the contract: the one-liners come from each SKILL.md frontmatter description, and "the same catalog lives in the README's ## Skills section and the repo CLAUDE.md; all three surfaces are kept in sync when a skill is added or renamed."

I checked all three at this head, and each already carried the corrected wording for every one of the five — /faceless-explainer, /talking-head-recut, /motion-graphics, /music-to-video, /general-video (docs/guides/skills.mdx:87-94, README.md:83-90, CLAUDE.md:23-30). That is why the drift was isolated to the two tables this PR fixes: docs/prompting/overview.mdx:51-63 and docs/quickstart.mdx:26-32 carry the same catalog content but are not named in that sync set, so nothing brings them along when a description changes.

One clause added to that paragraph naming the two prompting-guide tables would close it. Cheap, and it is the difference between fixing this instance and fixing the class.

nit — one input dropped from /music-to-video

The corrected cell at docs/prompting/overview.mdx:63 reads "A music track (or a video's audio)". The skill's own description carries a third input — a track generated from a mood brief — which is the one that lets someone with no audio at all take this route, and the same clause that makes "a complete video needs zero assets" true. README.md:88 and docs/guides/skills.mdx:92 compress it out the same way, so this is consistency with the catalog rather than a new gap; the page that just fixed a contract inversion in this same cell is the natural place to gain it.

note on re-verifying the run-shape claim

The discussion here cites references/route-briefs.md. At this head that file is an eleven-line "moved" pointer and the Run-shape lines live in references/routes/<workflow>.md. The claim is correct — I verified it in the per-route files — only the citation is stale, and it will send the next reader to a stub.

CI

Terminal-green at d3b0430e6a7ab18e1a06a68a0a86e71896c0d710: 23 successes, 48 skipped by the docs-only path filters, zero failures. The single non-success entry is a cancelled CI run superseded by a later CI run on the same SHA that concluded success. Docs, CodeQL, regression, preview-regression, Player perf, and Windows render verification all concluded success. The block on this PR is the reviewer gate, not a check.

Verdict: APPROVE
Reasoning: Every load-bearing claim in the diff checks out against the skill sources it compresses, the gallery fix addresses the cause and preserves the playback contract on all 18 clips, and CI is terminal-clean at this exact head. Both nits are non-blocking, and the more valuable one is about preventing recurrence rather than about this diff.

— Rames Jusso

WaterrrForever added a commit that referenced this pull request Jul 30, 2026
…-video input

Follow-up on the two review nits from #2872.

`/music-to-video`'s SKILL.md names three inputs — an audio file, a video to
pull audio from, or a track generated from a mood brief. Every compressed copy
of that description carried only the first two, and the third is the one that
makes "a complete video needs zero assets" true. Fixed on all eight surfaces
that state it, so no surface is now more correct than its siblings: the
prompting overview and quickstart setup tables, docs/guides/skills.mdx, the
README catalog, root CLAUDE.md + AGENTS.md, both CLI project templates, and the
router's own routes/music-to-video.md Input line (whose Interview must-haves
already listed all three).

The drift was structural, not accidental: the sync set declared in
docs/guides/skills.mdx and in CLAUDE.md's "Skill catalog maintenance" named
four surfaces and never the two setup tables, so those two were free to rot
while the declared four stayed correct. Both declarations now name them, and
both say the set applies to a *changed contract* — a reworded description —
not only to an added or renamed skill.

skills-manifest.json regenerated for the touched route file.

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

Copy link
Copy Markdown
Collaborator Author

Both non-blocking nits from @jrusso1020's review are now fixed in 56f23574a. New commit on top of the approved head — nothing earlier was rewritten, so d3b0430e6 is still in the history. Heads-up that this reset reviewDecision to REVIEW_REQUIRED; the repo dismisses approvals on a new head, so this needs a re-stamp.

Nit 2 — /music-to-video's third input

Fixed, but the omission wasn't confined to the overview row. The SKILL.md names three inputs; eight surfaces stated only the first two:

  • docs/prompting/overview.mdx and docs/quickstart.mdx setup tables
  • docs/guides/skills.mdx and the README.md catalog
  • root CLAUDE.md + AGENTS.md
  • both CLI project templates (packages/cli/src/templates/_shared/)
  • skills/hyperframes/references/routes/music-to-video.md — its Input line, even though the Interview → must-haves line two lines below already listed all three ("a track file, a video to pull audio from, or generate one from a mood description")

Fixing only the overview row would have left it more correct than its siblings, which is the same drift in the other direction, so all eight now match. skills-manifest.json regenerated for the touched route file.

Nit 1 — the drift class

You were right that the fix was instance-level. Both declarations now name the two setup tables: docs/guides/skills.mdx and CLAUDE.md's "Skill catalog maintenance" item 1.

One thing I'd add to your diagnosis: the declarations were also scoped to the wrong event. Both said "when a skill is added or renamed" — but /music-to-video was neither. Its description: was reworded, which is exactly how a compressed copy ends up asserting the opposite of the contract. Both now state the set applies to a changed contract too.

On the graphic-overlays point

Converging with you and @miga-heygen that there's nothing to fix, but the "zero occurrences in the tree" reasoning doesn't hold — there are 12, in 5 files:

packages/cli/src/commands/skills.test.ts   8
packages/cli/src/commands/skills.ts        1
packages/cli/src/utils/skillsManifest.ts   1
docs/changelog.mdx                         1
releases/v0.7.8.md                         1

They're all deliberate references to the rename: skills.ts:719 reads // (e.g. graphic-overlays → talking-head-recut) would linger forever in the prune logic, skillsManifest.ts:422 says the same, and the test fixtures use the old name as their canonical renamed-skill case. So the conclusion stands — directory is skills/talking-head-recut/, frontmatter is name: talking-head-recut, /talking-head-recut in the table is correct — but the occurrences do exist and they're load-bearing.

Separately: one stale pointer I did not touch

CLAUDE.md's maintenance item 3 still points at skills/hyperframes/references/workflow-catalog.md for "the input/output/trigger contract the router reads." That file is now a stub reading # Workflow catalog (moved) — the contracts live in references/routes/<workflow>.md. Same failure class as nit 1 (a maintenance rule outliving the layout it describes), but it belongs to the routes/ split rather than this PR, so I left it. Happy to fold it in if you'd rather it not wait.

Validation on the new head

  • bun packages/cli/scripts/gen-skills-manifest.ts --check → in sync (19 skills)
  • Skills test suite: 454 pass, 0 fail (all skills/**/*.test.mjs)
  • scripts/lint-skills.ts → 31 skill files, no issues
  • Both CLI templates still byte-identical
  • 248 docs MDX compile, 0 failures; link/anchor check matches the origin/main baseline exactly (same 11 pre-existing /packages/cli#*)
  • oxfmt --check clean on skills-manifest.json

…-video input

Follow-up on the two review nits from #2872.

`/music-to-video`'s SKILL.md names three inputs — an audio file, a video to
pull audio from, or a track generated from a mood brief. Every compressed copy
of that description carried only the first two, and the third is the one that
makes "a complete video needs zero assets" true. Fixed on all eight surfaces
that state it, so no surface is now more correct than its siblings: the
prompting overview and quickstart setup tables, docs/guides/skills.mdx, the
README catalog, root CLAUDE.md + AGENTS.md, both CLI project templates, and the
router's own routes/music-to-video.md Input line (whose Interview must-haves
already listed all three).

The drift was structural, not accidental: the sync set declared in
docs/guides/skills.mdx and in CLAUDE.md's "Skill catalog maintenance" named
four surfaces and never the two setup tables, so those two were free to rot
while the declared four stayed correct. Both declarations now name them, and
both say the set applies to a *changed contract* — a reworded description —
not only to an added or renamed skill.

skills-manifest.json regenerated for the touched route file.

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

Copy link
Copy Markdown
Collaborator Author

CI is green on 7608fa899. Correcting my own previous comment: the checks were not clean when I posted it — my first push of the nit fixes failed four checks. Both causes were mine to own:

1. README.md failed oxfmt --check, and it cascaded. Widening the /music-to-video table cell left the row's trailing padding out of alignment. That one file took down four checks, because preview-parity has needs: [changes, preflight] and preflight runs format:check — so the format failure skipped parity, and the preview-regression gate treats a skipped parity as a failure:

README.md oxfmt → Preflight ✗ → Preview parity skipped → preview-regression ✗
                → Format ✗ (standalone job)

Note also why this PR reached those jobs at all: packages/cli/** is in the preview-regression path filter, and the nit-2 fix touched packages/cli/src/templates/_shared/. The earlier docs-only heads never triggered them.

Amended rather than stacked a fixup, since it was a defect in that commit. The whitespace-only fix is the entire delta between 56f23574a and 7608fa899.

Why I missed it locally: I'd run oxfmt --check on only the JSON I regenerated, and my worktree had no node_modules, so bunx oxfmt fetched latest instead of the pinned 0.41.0. The wrong version reported 26 unrelated files failing, which is noise that hides the real one. Running the repo's pinned binary reproduces CI exactly — one file, README.md. Full-repo oxfmt --check . now passes on the pinned version.

2. Validate docs was cancelled — a flake, not content. npx mint validate ran 4m20s against the job's timeout-minutes: 5 and got killed. The same job passed on 56f23574a, whose docs/ tree is byte-identical to this head (the only delta is README whitespace, and README isn't under docs/). Re-ran the job; it passed in well under the limit. Worth noting the headroom here is thin — that job cold-installs mint@4.2.762 inside a 5-minute budget, so this will recur on unrelated PRs.

Green on 7608fa899

Format · Preflight (lint + format) · preview-regression · Skills: manifest in sync · Skills: project-native lint + mirror · Validate docs · Mintlify Deployment · CodeQL — no FAILURE, TIMED_OUT, or CANCELLED entries remain.

Also re-verified locally against the pinned toolchain: manifest in sync (19 skills), skills suite 454/454, lint-skills.ts clean over 31 files, both CLI templates byte-identical, 248 docs MDX compile, link/anchor check matching the origin/main baseline.

Still needs a re-stamp — reviewDecision is REVIEW_REQUIRED since the head moved. @jrusso1020 @miguel-heygen when you get a chance.

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: v2 — catalog drift fix

The new commit closes the music-to-video description drift across all 8 surfaces that state it:

Surface Updated
AGENTS.md
CLAUDE.md
README.md
docs/guides/skills.mdx
docs/prompting/overview.mdx
packages/cli/src/templates/_shared/AGENTS.md
packages/cli/src/templates/_shared/CLAUDE.md
skills/hyperframes/references/routes/music-to-video.md

All surfaces now include the third input: "one generated from a mood brief." Wording varies by context (the setup tables compress, the route file is detailed) but the semantic is consistent — a complete video needs zero supplied assets.

The real fix is structural, not editorial: the sync set declaration in CLAUDE.md's "Skill catalog maintenance" section AND docs/guides/skills.mdx's "Source of truth" now name the two setup tables (overview + quickstart) as part of the sync set, and both state the set applies to changed contracts — not just added or renamed skills. This is what prevents the next drift.

skills-manifest.json regenerated for the touched route file. Hash changed only for hyperframes (the skill that owns the route). Consistent.

Still approve. Clean SSOT follow-through.

— Miga

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head v2 stamp at 7608fa899.

The previously approved head d3b0430e6 remains an ancestor, so I audited the new nine-file delta. The contract text now matches skills/music-to-video/SKILL.md:3,31 across all eight surfaces that state the input: root AGENTS.md:22, root CLAUDE.md:28, README.md:88, docs/guides/skills.mdx:92, docs/prompting/overview.mdx:63, both CLI templates at line 15, and skills/hyperframes/references/routes/music-to-video.md:3. All include file/video/mood-brief generation, preserving the zero-supplied-assets route.

The recurrence fix is correctly structural: CLAUDE.md:50 and docs/guides/skills.mdx:114 now include the prompting setup surfaces in the sync set and explicitly cover changed contracts, not only add/rename events. The CLI templates remain byte-identical, skills-manifest.json regenerates in sync with only the owning hyperframes hash changed, and git diff --check is clean.

Current-head CI is terminal: 41 successes, 5 intentional skips, zero pending or failed checks. The remaining GitHub block is the reviewer gate.

Verdict: APPROVE
Reasoning: The v2 delta closes both the specific music-input omission and the maintenance-rule gap that allowed the catalog surfaces to drift, with exact-head validation clean.

— Magi

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of the delta since the previously approved head — nine files, both nits. I read the delta in full and re-derived the surface population from a fresh checkout of this head rather than from the change list.

Additive to @miga-heygen's and @miguel-heygen's re-reviews: a correction I owe on my own prior comment, the sweep that actually licenses the "all eight" claim, one completeness check neither stated, and one new finding.

Correction to my previous review

My graphic-overlays note said the string appears zero times in the tree. That was wrong, and you're right to push on it — there are 12 occurrences across 5 files: packages/cli/src/commands/skills.test.ts (8), packages/cli/src/commands/skills.ts:719, packages/cli/src/utils/skillsManifest.ts:422, docs/changelog.mdx:2524, releases/v0.7.8.md:25. I confirmed each is a deliberate reference to the rename: the two source hits are prune-logic comments explaining why a renamed skill's stale bundle would otherwise linger, the test hits use the old name as the canonical renamed/unknown-skill fixture, and the last two are the historical changelog and release entries.

The conclusion is unchanged — skills/talking-head-recut/ is the directory, its frontmatter is name: talking-head-recut, there is no skills/graphic-overlays/, so /talking-head-recut in the tables is correct — but it stands on the directory and frontmatter, not on an absence I asserted without measuring.

What I verified on the delta

  • The completeness claim is real, and here is what licenses it. Rather than confirm the eight changed rows, I swept the tree for the old two-input shape (audio file, or video to pull audio from, or a video's audio, music track (or a): zero remnants. Every surface that states the input now states three, and the propagated wording matches skills/music-to-video/SKILL.md's description: in both directions, including the "zero supplied assets" clause that the third input is what makes true.
  • The amended contract has no unamended sibling. This is the check that decides whether nit 1 actually closed: AGENTS.md carries the workflow list but no maintenance/sync declaration of its own, so the contract exists in exactly two places — CLAUDE.md item 1 and docs/guides/skills.mdx "Source of truth" — and both were amended. Had AGENTS.md held a third copy, this would have reproduced the original bug inside its own fix.
  • The new cross-link resolves. /prompting/overview#one-time-setup matches the ## One-time setup heading at docs/prompting/overview.mdx:29.
  • Both templates are still byte-identical (verified by hash, not by reading).
  • Scoping the sync set to a changed contract, not just add/rename, is the better half of this fix. A reworded description: was exactly the event both declarations failed to cover, and it is the event most likely to recur.

One factual note on the nit-2 accounting

The eight fixed surfaces don't include docs/quickstart.mdx, though the comment lists it. Its workflow skills are a bare inline enumeration at :32 with no per-workflow descriptions, so there was nothing there to correct — the count is right, the attribution is off by one file. Worth saying only because it cuts the other way for nit 1: quickstart.mdx does belong in the sync set, since its core-skill table at :22-31 compresses those descriptions and can drift on its own.

Also worth noting for the record that docs/prompting/music-and-slideshows.mdx:31 already carried all three inputs before this PR. The prose page that explains the workflow was correct; only the compressed one-liners were wrong. That is the drift direction your diagnosis predicts, and it is the strongest evidence the diagnosis is right.

New finding — the same failure class survives two items below the one you fixed

CLAUDE.md maintenance item 3 has two stale pointers, not one. You flagged skills/hyperframes/references/workflow-catalog.md; item 3 also sends readers to references/route-briefs.md "when the route asks questions." Both are now # … (moved) stubs, and both contracts live under references/routes/<workflow>.md.

So the numbered list this PR just corrected at item 1 still misdirects at item 3, on both of its pointers, in the same failure class — a maintenance rule outliving the layout it describes. Since you offered: yes, please fold it in. It is two path corrections in a file this PR already edits, and it closes the item that would otherwise send the next person doing exactly this task to two stubs. Non-blocking either way — if you'd rather land this as-is, the follow-up is equally fine, and a re-stamp after one more push is cheap.

CI

Terminal-green at 7608fa89904a856ca3433fbd8134b253fce49a2f, read at this head rather than carried over: 41 success, 5 skipped, and zero non-success entries. All seven workflow runs at this SHA are completed/success. Docs is on attempt 2, which matches the account of the re-run after the timed-out validate job — the passing attempt is the one attached to this head, and no cancelled or timed-out entry remains. The block is the reviewer gate, not a check.

Thanks for handling the CI self-correction the way you did. Retracting your own "green" claim and naming the cascade — one misaligned table cell taking down four checks through a skipped-parity gate — is more useful than a clean report would have been, and the unpinned-formatter detail is the part that will save someone else the same hour.

Verdict: APPROVE
Reasoning: Both nits are addressed at the class level rather than the instance, the completeness claim survives an independent old-shape sweep with zero remnants, the amended contract has no unamended sibling copy, and CI is terminal-green at this exact head. The item-3 finding is non-blocking and the author has already offered to take it.

— Rames Jusso

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants