fix(skills): search the component catalog before hand-building a look (#3829)

* fix(motion-graphics): make the catalog search fire before hand-authoring

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.

* fix(skills): search the component catalog before hand-building a look

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.

* refactor(scripts): reuse native recursive readdir and the shared registry 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.

* refactor(scripts): name the registry check's blind spots and stop self-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.

* chore(skills): regenerate skills manifest after catalog-search edits
This commit is contained in:
Miguel Ángel
2026-09-10 03:21:33 +00:00
committed by GitHub
parent db51d5479d
commit 0f8eb892e3
28 changed files with 309 additions and 43 deletions
+56 -1
View File
@@ -8,7 +8,7 @@
import test from "node:test";
import assert from "node:assert/strict";
import { lintFrontmatter } from "./lint-skills.ts";
import { lintFrontmatter, lintRegistryItemRefs } from "./lint-skills.ts";
const wrap = (frontmatter) => `---\n${frontmatter}\n---\n\n# body\n`;
@@ -125,3 +125,58 @@ test("invalid: top-level scalar (frontmatter is not a mapping)", () => {
// be non-empty.
assert.ok(violations.length > 0);
});
// ---------------------------------------------------------------------------
// Registry-snapshot drift guard
// ---------------------------------------------------------------------------
const KNOWN = new Set(["caption-glitch-rgb", "code-diff", "data-chart"]);
const MARKER = "<!-- registry-items: -->";
test("registry refs: unmarked file is never checked", () => {
// Opt-in is the whole design. Most kebab-case backticks in skill docs are CSS
// properties, data-* attributes or skill directory names, and a check that
// flags those gets switched off. null (not []) distinguishes "not a snapshot"
// from "a snapshot with nothing wrong", which is what the counter reports.
assert.equal(lintRegistryItemRefs("Use `not-a-real-item` here.\n", KNOWN), null);
});
test("registry refs: a marker inside a fenced block does not arm the check", () => {
// Otherwise a doc that documents this marker's own syntax arms the check on
// itself, and every identifier in it starts failing for no stated reason.
const doc = ["# Doc", "", "```md", MARKER, "```", "", "Use `not-a-real-item`."].join("\n");
assert.equal(lintRegistryItemRefs(doc, KNOWN), null);
});
test("registry refs: marked file passes when every id is real", () => {
const doc = `${MARKER}\n\nUse \`caption-glitch-rgb\` or \`code-diff\`.\n`;
assert.deepEqual(lintRegistryItemRefs(doc, KNOWN), []);
});
test("registry refs: marked file flags an id the registry does not have", () => {
const doc = `${MARKER}\n\nInstall \`text-wave-distort\` for the wobble.\n`;
const violations = lintRegistryItemRefs(doc, KNOWN);
assert.equal(violations.length, 1);
assert.equal(violations[0].line, 3);
assert.ok(violations[0].message.includes("text-wave-distort"));
});
test("registry refs: allow= exempts a legitimately non-item identifier", () => {
const doc = `<!-- registry-items: allow=dark-plus,pin-rollout -->\n\n\`dark-plus\` and \`pin-rollout\`.\n`;
assert.deepEqual(lintRegistryItemRefs(doc, KNOWN), []);
});
test("registry refs: non-id backticks are ignored", () => {
const doc = `${MARKER}\n\n\`--json\`, \`Foo-Bar\`, \`a b-c\`, \`UPPER-CASE\`.\n`;
assert.deepEqual(lintRegistryItemRefs(doc, KNOWN), []);
});
test("registry refs: single-word ids are a KNOWN blind spot, not an accident", () => {
// The pattern requires a hyphen, so real single-word registry items (glitch,
// flowchart, typewriter, confetti, separator, vignette, vignelli) are never
// checked. Pinned here so the tradeoff is visible in code, not just in a
// comment: dropping the hyphen would cost 46 allow= entries of prose nouns
// across the marked files to monitor 3 more items. See lint-skills.ts header.
const doc = `${MARKER}\n\n\`glitch\` was renamed and this doc was not updated.\n`;
assert.deepEqual(lintRegistryItemRefs(doc, KNOWN), []);
});
+95 -4
View File
@@ -12,6 +12,7 @@
import { readFileSync, readdirSync, statSync } from "node:fs";
import { join, relative } from "node:path";
import { parse as parseYaml, YAMLParseError } from "yaml";
import type { RegistryManifest } from "../packages/core/src/index.js";
const REPO_ROOT = join(import.meta.dirname, "..");
// Every location that ships SKILL.md files gets linted. `skills/` is the
@@ -256,6 +257,79 @@ function lintInlinePatterns(file: string, stripped: string): Violation[] {
.flatMap((line, i) => (line ? matchDangerousPatterns(file, line, i + 1) : []));
}
// ---------------------------------------------------------------------------
// Registry-item references in hand-maintained snapshots
// ---------------------------------------------------------------------------
//
// A skill doc that snapshots part of the component registry rots in silence:
// nothing fails when an item is renamed or dropped, and the agent that follows
// the doc runs `hyperframes add <gone>` and dies. A doc opts into this check
// with a marker line, after which every registry-item-shaped identifier in a
// backtick span must name a real item in registry/registry.json:
//
// <!-- registry-items: allow=some-suffix,another-suffix -->
//
// Opt-in rather than repo-wide on purpose. Kebab-case backticks are also CSS
// properties, `data-*` attributes, skill directory names, script names, and
// motion-graphics category names, and a check that flags those is a check
// people turn off. `allow=` carries the few non-item identifiers a snapshot
// legitimately names (bare suffixes under a spelled-out prefix, ids the doc
// itself marks as hand-authored).
//
// TWO KNOWN BLIND SPOTS, both deliberate, both false NEGATIVES (this check
// never invents a violation, it only misses some):
//
// 1. Identifiers outside a backtick span are not seen. A bare `bar-chart-race`
// in prose slipped past this check while it was a live defect elsewhere.
// 2. Single-word item names are not seen, because the pattern below requires a
// hyphen. Measured on the six currently-marked files: dropping the hyphen
// requirement would monitor 3 more real items (`glitch`, `flowchart`,
// `typewriter`) and force 46 new allow= entries for ordinary prose words
// ("add", "line", "name", "height", "text"). A 15:1 noise ratio is how a
// check gets switched off, so the hyphen requirement stays.
const REGISTRY_MARKER = /<!--\s*registry-items:\s*(?:allow=([^\s]*))?\s*-->/;
const REGISTRY_ITEM_ID = /^[a-z][a-z0-9]*(?:-[a-z0-9]+)+$/;
function registryItemNames(): Set<string> {
const raw = readFileSync(join(REPO_ROOT, "registry", "registry.json"), "utf-8");
// The cast describes the file; it does not validate it. The runtime filter and
// the throw below are what actually stop us linting against an empty set.
const parsed = JSON.parse(raw) as RegistryManifest;
const names = (parsed.items ?? []).map((item) => item.name).filter((name) => Boolean(name));
if (names.length === 0) {
throw new Error("registry/registry.json parsed to zero item names — refusing to lint blind.");
}
return new Set(names);
}
/** `null` when the file is not marked as a registry snapshot; otherwise its violations. */
export function lintRegistryItemRefs(content: string, known: Set<string>): LineViolation[] | null {
// Marker detection ignores fenced blocks so a doc that *documents* the marker
// syntax in an example does not arm the check on itself. The scan below still
// reads full content, so ids inside fenced examples stay covered.
const marker = stripFencedBlocks(content).match(REGISTRY_MARKER);
if (!marker) return null;
const allowed = new Set((marker[1] ?? "").split(",").filter(Boolean));
return content.split("\n").flatMap((line, index) => {
const dead = [...new Set([...line.matchAll(/`([^`\n]+)`/g)].map((m) => (m[1] ?? "").trim()))]
.filter((token) => REGISTRY_ITEM_ID.test(token))
.filter((token) => !known.has(token) && !allowed.has(token));
return dead.map((token) =>
violation(
index + 1,
`"${token}" is not an item in registry/registry.json, but this file is marked as a registry snapshot. Correct the name, remove it, or add it to the marker's allow= list if it is legitimately not an item.`,
line.trim(),
),
);
});
}
function collectMarkdownFiles(dir: string): string[] {
return readdirSync(dir, { withFileTypes: true, recursive: true })
.filter((entry) => entry.isFile() && entry.name.endsWith(".md"))
.map((entry) => join(entry.parentPath, entry.name));
}
function lintFile(filePath: string): Violation[] {
const raw = readFileSync(filePath, "utf-8");
const file = relative(process.cwd(), filePath);
@@ -281,18 +355,35 @@ if (files.length === 0) {
let totalViolations = 0;
for (const file of files) {
const violations = lintFile(file);
function report(file: string, violations: LineViolation[]): void {
for (const v of violations) {
console.error(`${v.file}:${v.line}: ${v.message}`);
console.error(`${file}:${v.line}: ${v.message}`);
console.error(` ${v.text}\n`);
totalViolations++;
}
}
for (const file of files) {
report(relative(process.cwd(), file), lintFile(file));
}
const knownItems = registryItemNames();
let snapshotsChecked = 0;
for (const dir of SKILLS_DIRS) {
if (!statSync(dir, { throwIfNoEntry: false })?.isDirectory()) continue;
for (const path of collectMarkdownFiles(dir)) {
const found = lintRegistryItemRefs(readFileSync(path, "utf-8"), knownItems);
if (found === null) continue;
snapshotsChecked++;
report(relative(process.cwd(), path), found);
}
}
if (totalViolations > 0) {
console.error(`\n${totalViolations} skill lint error(s) found.`);
process.exit(1);
} else {
console.log(`Checked ${files.length} skill file(s) — no issues found.`);
console.log(
`Checked ${files.length} skill file(s) and ${snapshotsChecked} registry snapshot(s) against ${knownItems.size} registry items — no issues found.`,
);
}