fix(skills): search the component catalog before hand-building a look - #3829
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
The workflow's only reuse instruction pointed at catalog-map.md, a
hand-maintained snapshot of ~60 registry items, and no file in the skill
ever named `hyperframes catalog --query`. An agent asked mid-build for
CRT scanlines and a glitch effect had no instruction to search, so it
hand-authored both while caption-glitch-rgb ("RGB chromatic aberration
with CRT scanline overlay") ranks first for that query on either tier.
The search reads the hosted registry and needs nothing installed, from
any directory with no project, so "the components were not installed"
was never the cause. Say that where the reader is, since the wrong
diagnosis is the intuitive one.
Director Part 2 and the Builder now run the search before naming a
block, and catalog-map.md is labelled a partial snapshot whose misses
prove nothing. Pinned by a content test in coreSkillContent.test.ts.
Authoring workflows never told the agent to search the component library,
so agents rebuilt effects the registry already shipped. A user reported
building an effect from scratch that the registry already contained; the
search that would have found it needs nothing installed, which is why the
usual self-diagnosis ("I forgot to install the components") is wrong.
All ten workflow skills carried zero mentions of `hyperframes catalog`.
The instruction lived only in hyperframes-cli and hyperframes-registry,
both loaded on demand, and the registry skill's own trigger named the
command rather than the symptom - circular, because an agent that never
thought to search could not reach the doc telling it to search.
- Eight workflows now run the search at the point they decide what to
build, before authoring. The two that compile through a closed
authoring vocabulary (embedded-captions, talking-head-recut) document
why they deliberately do not.
- hyperframes-registry triggers on the symptom (a named look, effect,
treatment or transition) instead of the command name; the router table
and the catalog surfaces carry the same framing.
- Fixes hand-maintained lists that had drifted: bar-chart-race was listed
as a hand-author gap in two files while shipping in the registry;
stat-motion was named as an installable block and is not one; the
caption-* family count was one high; the registry discovery tables
claimed to be the block list while covering 97 of 180.
- bun run lint:skills now fails when a doc marked as a registry snapshot
names an item the registry does not have.
…stry type
Simplify pass on the new registry-snapshot check, behaviour identical:
- collectMarkdownFiles uses readdirSync({ recursive: true }) instead of
hand-rolled recursion, matching scripts/generate-template-previews.ts.
- registryItemNames types registry.json with the exported RegistryManifest
instead of an ad hoc inline shape, matching scripts/catalog/build-local-vectors.ts.
The runtime guard stays: a cast describes the file, it does not validate it.
- One report() helper replaces the duplicated print-and-count block in both
lint passes.
…f-arming Applies the review findings on the new check, behaviour identical except where noted: - lintRegistryItemRefs returns null for an unmarked file instead of an empty array, so "not a snapshot" and "a clean snapshot" have one owner and the marker is matched once rather than twice. - Marker detection ignores fenced blocks, so a doc that documents the marker syntax in an example no longer arms the check on itself. The id scan still reads full content, so fenced examples stay covered. - The header comment and two tests now pin both known false negatives: identifiers outside backticks, and single-word item names. Measured on the six marked files, dropping the hyphen requirement would monitor 3 more items and force 46 allow= entries for ordinary prose words, so the requirement stays and the gap is stated instead of silent.
60ac53b to
f44651c
Compare
There was a problem hiding this comment.
APPROVE at f44651c294d4f4ed5f0131b7c19ac0b1d2e21789. Skills-catalog discipline for the 5th/last PR in the arc. Docs + one lint script + tests, 28 files, +309/−43. All 5 commits by miguel-heygen.
1. Every authoring workflow searches, or is documented as exempt.
Enumerated the 10 workflow-shaped skills in skills/ (excluding atomic hyperframes-*, media-use, figma): embedded-captions, faceless-explainer, general-video, motion-graphics, music-to-video, pr-to-video, product-launch-video, remotion-to-hyperframes, slideshow, talking-head-recut. Matches PR body claim of 10.
- 8 that now search — verified by
coreSkillContent.test.ts:150-166: each containsnpx hyperframes catalog --queryANDnothing installed. Motion-graphics is pinned viaagents/director.md(director picks blocks) +agents/builder.md:9(search mid-build too) +catalog-map.mdpreamble. - 2 exempt — verified genuinely can't mount a registry composition:
embedded-captions: grep of the entireskills/embedded-captions/tree finds ZERO occurrences ofdata-composition-src/hyperframes add/hyperframes catalog(outside the new exemption note). Engines are locked compilers overcinematic.json/theme.json. Real.talking-head-recut: the ONEdata-composition-srcmention (SKILL.md:246) is for chunking a card's OWN timeline into sub-compositions, not mounting registry blocks.data-animcard-kinds vocabulary is closed and card fragments have no place to mount a standalone composition. Real.
2. Drifted-list fixes (six defects, all verified against registry/registry.json):
bar-chart-racemoved from gap → reuse incatalog-map.mdandcharts/module.md— the item exists in registry (lint enforces this since it's now in a marked file withoutallow=).stat-motiondemoted from "installable block" to hand-authored incatalog-map.md:29— correctly listed in theallow=alongsidegeo-*/flag-borders/pin-rolloutunder an inline comment stating these are deliberately-named non-items.caption-*count18 → 17matches the family.discovery.md: "97 blocks" → "97 of 180" — registry actually has 180 blocks. Accurate.init-and-scaffold.md:vscode-theme-visualizeradded to the closed set of 10 templates — realhyperframes:exampleinregistry.json.
3. The lint (scripts/lint-skills.ts:207-271, new lintRegistryItemRefs):
- Opt-in via
<!-- registry-items: [allow=...] -->marker (6 files currently marked). - Fenced-block-aware marker detection prevents self-arming when the marker syntax is documented in an example.
registryItemNames()throws ifregistry.jsonparses to zero item names — refuses to lint blind.- CI-wired as required:
.github/workflows/ci.yml:431(Skills: project-native lint + mirrorjob runsbun scripts/lint-skills.ts) AND top-levelLintchainstsx scripts/lint-skills.tsviapackage.json:29. Both SUCCESS at head. - Tripwire proven by Miguel via
x-post → x-post-legacyrename in a marked file → lint fails with file, line, id.
4. Catalog-search patterns. Every skill's new block ends with "Hand-author a look only after a search for it came back with nothing that fits." Graceful fallback. No hard runtime dependency (these are doc directives, not code).
5. Test coverage.
scripts/lint-skills.test.mjsgains 7 tests (unmarked skipped, fenced-marker ignored, all-real passes, bad-id flagged with correct line,allow=exempts, non-id backticks ignored, single-word blind spot pinned as known-false-negative).coreSkillContent.test.tsgains 3 test blocks (73 lines) pinning: motion-graphics search wiring in 3 files, 8-workflow search + "nothing installed" clause, 2-exemption clause, registry skill's symptom-trigger description withCRT scanlinesexample.- Both known blind spots (out-of-backtick refs, single-word ids) documented in
lint-skills.ts:225-235header with the 15:1 noise-ratio measurement that justifies the tradeoff. Not silent.
6. Scope. Docs + one lint script + tests. No engine/runtime/studio touch. skills-manifest.json hashes regenerated in the last commit (f44651c). Zero test deletions.
7. CI at head. 31 SUCCESS, 8 SKIPPED (change-filter gated), 8 IN_PROGRESS, 1 QUEUED, 1 NEUTRAL (CodeQL summary). Zero FAILURE/CANCELLED. Skills-relevant checks GREEN: Lint, Skills: project-native lint + mirror, Skills: manifest in sync, Test: skills, Format, Test: skill bootstrap (Windows, Node 24). Pending checks are the hyperframes CI cascade — none touch the skills tree. Doc/lint PRs on this rebase (post-#3834's flake fixes) should settle green.
8. No concurrent review at head — reviews: [].
One follow-up nit (not a blocker).
skills/hyperframes-cli/references/init-and-scaffold.md:19 self-claims its Templates list "is checked by bun run lint:skills" — but the file has no <!-- registry-items: --> marker, so the lint silently skips it. Adding the marker would need allow=blank (blank is a CLI-hardcoded template, not a registry item). Minor doc/lint asymmetry — the exact drift class this PR aims to prevent — but a follow-up, not a hold.
Additional follow-ups (added post-review, not blocking).
FU-1. Doc-directive gap for judging the CLI's threshold-free fuzzy ranker. Read packages/cli/src/commands/catalog.ts: catalog is two-tier fuzzy (default word-match against name/title/description/tags, opt-in on-device meaning-search via a downloadable local model). Explicit at catalog.ts:547-551: "Deliberately not a threshold: it is the signal a caller needs to judge a ranker that ships without one." Discrimination burden is placed on the agent caller reading top_score. NO false-positive/lookalike test in catalog.test.ts — nothing pins "asks for X, catalog has X2 — top result isn't a confidently-labeled X2 masquerading as X." The doc directives this PR adds say "read the top results" and "hand-author only after a search came back with nothing that fits" — but never tell the agent HOW to judge a threshold-free fuzzy match. An agent that asks for "confetti burst" and gets "particle-burst" ranked #1 has no doc-level tripwire to reject the lookalike. Worth a next-iteration on directive wording (interpret top_score, fall through when the top result's name/description doesn't semantically match the query).
FU-2. Exempt-skill inverse invariant. The two exemptions (embedded-captions, talking-head-recut) are plain doc claims pinned only by a positive-presence test (coreSkillContent.test.ts:184 checks the string "does not search the HyperFrames component registry" appears). NO test asserts the exempt skills lack data-composition-src / hyperframes add — the capability claim that justifies the exemption. Verified true at head SHA (grep), but nothing catches drift: if a future PR adds registry mounting to embedded-captions without touching the exemption note, the doc's capability claim silently becomes false, and the "does not search" positive-presence test still passes. Fragile against future capability growth. The inverse invariant — grep for data-composition-src in each exempt skill and fail if found, or grep for hyperframes add outside a fenced example — would make the exemption self-limiting.
FU-3. Lint direction is one-way by intent; no alias primitive exists. lintRegistryItemRefs (lint-skills.ts:252) flags doc → registry (references to items registry lacks) but has NO mirror check for registry items no doc references. Aligned with the PR body's stated goal ("rot in silence when an item is renamed or dropped") — the reverse direction isn't a rot pattern that harms an agent — but the asymmetry isn't explicit in the diff. Also: registry/registry.json has no aliases/alias field on any item, so the lint necessarily demands canonical-name matches — not by design choice, by absence of the underlying primitive. If registry aliases are ever added, the lint needs an update to accept aliased references.
— tai
The problem
Our authoring workflows never tell the agent to search the component library, so agents rebuild things that already exist.
A user reported building an effect from scratch that the registry already contained, and put it down to not having installed the components first. That diagnosis is wrong, and the way it is wrong is the whole bug: the catalog search needs nothing installed. It ranks the entire hosted registry from any directory, with no project and no account. Reproduced from an empty
/tmpdir:First result out of 399, and the directory is still empty afterwards.
Root cause is routing, not knowledge. All ten workflow skills contained zero mentions of
hyperframes catalog. The instruction lived only inhyperframes-cliandhyperframes-registry, both loaded on demand, and the registry skill's own trigger read "use when runninghyperframes addorhyperframes catalog" — circular, because an agent that never thought to search could not reach the doc telling it to search.Where the search now fires
Eight workflows run it at the point they decide what to build, above the authoring instructions rather than appended after them.
motion-graphicscatalog-map.mdpreambleproduct-launch-videofaceless-explainerpr-to-videocode-vocabulary.mdalready covers code beats; this covers the rest. Step 5 pre-installs whatever Step 4 namedmusic-to-videogeneral-videoslideshowremotion-to-hyperframestransitions.mdalready took this route forsdf-irisTwo workflows deliberately do not search, and now say so in writing.
embedded-captionsandtalking-head-recutboth compile through a closed authoring vocabulary — locked caption engines consumingcinematic.json/theme.json, and a fixeddata-animcard-kind list. A registry item is a standalone composition with its own timeline and nowhere to mount;embedded-captionshas nodata-composition-srcorhyperframes addpath anywhere in it. Adding the instruction there would have been false, and an unexplained gap invites someone to close it later, so each carries a short exemption note.The circular trigger is gone.
hyperframes-registrynow triggers on the symptom — a brief naming a look, effect, treatment or transition, with examples — instead of on the command name. The router table,CLAUDE.md,README.mdand the setup table carry the same framing, per the skill-catalog sync rule.Drifted lists
Every candidate was first classified as a genuine registry snapshot or not, because reporting the others as stale would be a false positive:
cut-catalog.mdlists cut types,workflow-catalog.mdlists workflows,music-to-video's catalogs name its own local templates and primitives underreferences/, andapi-map.mdlists HTML attributes.Real defects found and fixed:
motion-graphics/categories/charts/module.mdbar-chart-racelisted as a gap to hand-author — it ships in the registrymotion-graphics/catalog-map.mdbar-chart-racegap claimmotion-graphics/catalog-map.mdstat-motionnamed as an installable block; no such item existsmotion-graphics/catalog-map.mdcaption-*"hyperframes-registry/references/discovery.mdhyperframes-cli/references/init-and-scaffold.md--examplelist missingvscode-theme-visualizerfrom a closed set of 9Six genuine snapshots gain a one-line preamble: the live search is the source of truth, the table under-covers by design, and only an empty search result is evidence the registry lacks something. The tables are deliberately not expanded toward full coverage — that recreates the same defect at greater length.
Two suspected dead references did not reproduce and were left alone:
text-wave-distortandtext-spectral-raysare real local primitive directories inmusic-to-video, not registry ids;domain-warpandchromatic-splitare shader names rather than block names, and both files using them say so and link the mapping.The check
bun run lint:skillsnow fails when a doc marked as a registry snapshot names a registry-item-shaped identifier thatregistry/registry.jsondoes not have. Opt-in via a marker with a per-file allowlist:Opt-in on purpose. A repo-wide scan would flag CSS properties,
data-*attributes, script names and skill directory names, and a noisy check gets switched off, leaving no check plus the belief that there is one.Both blind spots are stated in the header comment and pinned by tests rather than left silent. It only sees identifiers inside backtick spans —
bar-chart-raceappeared unbackticked in prose in one of the files fixed above and would not have been caught. And it requires a hyphen, so seven single-word items are unmonitored: measured across the six marked files, dropping that requirement would monitor 3 more items at the cost of 46 allowlist entries for ordinary prose words ("add", "line", "name", "height", "text"). A 15:1 noise ratio is how a check gets disabled, so the requirement stays.coreSkillContent.test.tsasserts all ten workflows: eight carry the instruction and the "nothing installed" clause, two carry the exemption, and the registry skill carries the symptom trigger.Verification
bun run lint(includeslint:skillsandcheck:skill-mirror),bun run format:check,scripts/lint-skills.test.mjs(25 pass), andcoreSkillContent.test.ts(13 pass) all green.Both new guards were proven non-vacuous by breaking them on purpose: removing the slideshow instruction fails the content test on
skills/slideshow/SKILL.md: expected … to contain 'npx hyperframes catalog --query', and renamingx-posttox-post-legacyin a marked file fails the lint with the exact file, line and identifier.typecheck:scriptsreports threeTS2307errors for generated runtime modules; verified identical with this branch's changes stashed, so it is a pre-existing unbuilt-workspace condition, not from this PR.