From 51d46f6cd4662028e6046fc12c95559fff652930 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 19:28:15 +0000 Subject: [PATCH 1/3] perf(scripts): scope format and validate to given files format-yaml.ts and validate.ts always walked all 16,730 entries, so an import's edit-fix loop paid 68s of formatting and 34s of validation per cycle regardless of how many files it touched. Both now accept paths: formatting is per-file with no cross-file dependency, so it is a full win. Validation gains a --files pre-flight that reuses validateFile and keeps manufacturer-reference checks (slugs come from filenames), but skips duplicate-ID and supersedes checks and says so. The full run is unchanged and still guards the pre-commit hook and CI. Also documents the conditional io position rule in the generated JSON schema, which listed position as plainly optional while validate enforces it for everything outside the Instruments group. Co-Authored-By: Claude Opus 5 --- README.md | 8 +++ schema/json/hardware.json | 2 +- scripts/format-yaml.ts | 52 +++++++++++++---- scripts/generate-json-schemas.ts | 12 +++- scripts/validate.ts | 97 ++++++++++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 6b245b91a..6724dbf85 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,14 @@ pnpm install # Validate all YAML files pnpm validate +# Validate only the files you touched (fast pre-flight while editing). +# Skips cross-file checks, so still run the full `pnpm validate` first. +pnpm validate --files data/hardware/some-entry.yaml + +# Format all YAML files, or only the ones you touched +pnpm format +pnpm format data/hardware/some-entry.yaml + # Build SQLite database locally pnpm build diff --git a/schema/json/hardware.json b/schema/json/hardware.json index df140f5f2..bbbebaf01 100644 --- a/schema/json/hardware.json +++ b/schema/json/hardware.json @@ -2394,7 +2394,7 @@ "Left", "Right" ], - "description": "Physical position on the device. Defined in schema/io-positions.yaml." + "description": "Physical position on the device. Defined in schema/io-positions.yaml. Required on every io entry except on played instruments (the Instruments category group in schema/category-groups.yaml), whose single output jack has no panel position. Enforced by pnpm validate as E199, not by this schema." }, "columnPosition": { "type": "number" diff --git a/scripts/format-yaml.ts b/scripts/format-yaml.ts index 2b4b50668..ffafbaf0f 100644 --- a/scripts/format-yaml.ts +++ b/scripts/format-yaml.ts @@ -4,7 +4,12 @@ * Moves `id` field to the top of each YAML file, then runs Prettier. * Replaces plain `prettier --write` for YAML formatting. * - * Usage: tsx scripts/format-yaml.ts + * Usage: + * tsx scripts/format-yaml.ts # whole catalog + * tsx scripts/format-yaml.ts data/hardware/a.yaml # just these files + * + * Formatting has no cross-file dependency, so scoping it to the files an + * import touched turns a whole-catalog pass into an instant one. */ import { execSync } from "node:child_process"; @@ -54,14 +59,35 @@ function moveIdToTop(filePath: string): boolean { return true; } -let reordered = 0; - -for (const collection of COLLECTIONS) { - const files = getYamlFiles(path.join(DATA_DIR, collection)); - for (const file of files) { - if (moveIdToTop(file)) { - reordered++; +/** + * Files to format: the paths given on the command line, or every + * collection when none are. Formatting is per-file with no cross-file + * dependency, so scoping it to the handful of files an import touched + * turns a whole-catalog pass into an instant one. + */ +function filesToFormat(): { files: string[]; scoped: boolean } { + const args = process.argv.slice(2).filter((arg) => !arg.startsWith("-")); + if (args.length > 0) { + const resolved = args.map((arg) => path.resolve(process.cwd(), arg)); + const missing = resolved.filter((file) => !fs.existsSync(file)); + if (missing.length > 0) { + console.error(`No such file(s):\n${missing.map((f) => ` ${f}`).join("\n")}`); + process.exit(1); } + return { files: resolved, scoped: true }; + } + return { + files: COLLECTIONS.flatMap((collection) => getYamlFiles(path.join(DATA_DIR, collection))), + scoped: false, + }; +} + +const { files, scoped } = filesToFormat(); + +let reordered = 0; +for (const file of files) { + if (moveIdToTop(file)) { + reordered++; } } @@ -69,9 +95,13 @@ if (reordered > 0) { console.log(`Moved id to top in ${reordered} files`); } -// Run Prettier on all YAML files -console.log("Running Prettier..."); -execSync('prettier --write "data/**/*.yaml"', { +console.log(scoped ? `Running Prettier on ${files.length} file(s)...` : "Running Prettier..."); +// Prettier is given explicit paths when scoped so it never walks the +// whole data tree; the glob stays the default for a full run. +const targets = scoped + ? files.map((file) => JSON.stringify(path.relative(REPO_ROOT, file))).join(" ") + : '"data/**/*.yaml"'; +execSync(`prettier --write ${targets}`, { cwd: REPO_ROOT, stdio: "inherit", }); diff --git a/scripts/generate-json-schemas.ts b/scripts/generate-json-schemas.ts index f81da1795..e70f3f6f6 100644 --- a/scripts/generate-json-schemas.ts +++ b/scripts/generate-json-schemas.ts @@ -129,7 +129,17 @@ const ioSchema = { position: { type: "string", enum: ctx.ioPositions, - description: "Physical position on the device. Defined in schema/io-positions.yaml.", + // Not listed in `required`: the rule is conditional on the entry's + // primaryCategory, which JSON Schema cannot express here without + // duplicating the Instruments category group. `pnpm validate` + // enforces it (E199), so a file that omits position passes this + // schema and still fails validation ā€” say so rather than let an + // editor or an external contributor read it as optional. + description: + "Physical position on the device. Defined in schema/io-positions.yaml. " + + "Required on every io entry except on played instruments (the Instruments " + + "category group in schema/category-groups.yaml), whose single output jack " + + "has no panel position. Enforced by pnpm validate as E199, not by this schema.", }, columnPosition: { type: "number" }, rowPosition: { type: "number" }, diff --git a/scripts/validate.ts b/scripts/validate.ts index 4c1da6b5f..c0bd9941c 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -1941,10 +1941,107 @@ function writeConsoleOutput(result: ValidationResult): void { console.log(); } +// ============================================================================= +// SCOPED VALIDATION (--files) +// ============================================================================= + +const COLLECTION_SCHEMAS: Record = { + manufacturers: ManufacturerSchema, + software: SoftwareSchema, + content: ContentSchema, + hardware: HardwareSchema, + accessories: AccessorySchema, +}; + +/** + * Validate only the named files, for the edit-fix loop during an import. + * + * This is a pre-flight, not a replacement for the full run. Per-file rules + * (schema shape, enums, io, name hygiene) are identical, and manufacturer + * references still resolve because manufacturer slugs come from filenames + * rather than file contents. What it cannot see is anything cross-file: + * duplicate IDs, supersedes targets, and supersedes cycles. The full + * `pnpm validate` still runs in the pre-commit hook and in CI, so nothing + * reaches main unchecked. + */ +function validateScoped(paths: string[]): number { + const allManufacturers = new Set( + getYamlFiles(path.join(DATA_DIR, "manufacturers")).map((file) => + path.basename(file, path.extname(file)) + ) + ); + + const errors: ValidationError[] = []; + let checked = 0; + + for (const given of paths) { + const file = path.resolve(process.cwd(), given); + if (!fs.existsSync(file)) { + console.error(`āŒ No such file: ${given}`); + return 1; + } + const collection = path.basename(path.dirname(file)); + const schema = COLLECTION_SCHEMAS[collection]; + if (!schema) { + console.error( + `āŒ ${given} is not inside a known collection ` + + `(${Object.keys(COLLECTION_SCHEMAS).join(", ")}).` + ); + return 1; + } + // No validSupersedesIds: the id set is a cross-file fact this mode + // deliberately does not build, and validateFile skips the check. + const error = validateFile(file, schema, allManufacturers); + if (error) errors.push(error); + checked += 1; + } + + // Deliberately not writeConsoleOutput: its Stats block reports whole-catalog + // counts, which would read as an empty catalog here. + console.log(`\nšŸ“‹ Scoped validation ā€” ${checked} file(s)\n`); + console.log("ā”€".repeat(70)); + if (errors.length === 0) { + console.log("āœ… No errors in the checked files.\n"); + } else { + console.log("āŒ Validation failed!\n"); + for (const error of errors) { + console.log(`\nšŸ“„ ${error.file}`); + if (error.details && error.details.length > 0) { + for (const detail of error.details) { + const lineInfo = detail.line ? `:${detail.line}` : ""; + console.log(` ${detail.code ?? ""}${lineInfo}: ${detail.message}`); + console.log(` Path: ${detail.path}`); + if (detail.docsUrl) console.log(` Docs: ${detail.docsUrl}`); + } + } else { + for (const msg of error.errors) console.log(` āš ļø ${msg}`); + } + } + console.log(); + } + console.log("ā”€".repeat(70)); + console.log( + "Cross-file checks (duplicate IDs, supersedes targets and cycles) are skipped.\n" + + "Run 'pnpm validate' before committing." + ); + console.log(); + return errors.length === 0 ? 0 : 1; +} + // ============================================================================= // MAIN // ============================================================================= +const filesFlagIndex = process.argv.indexOf("--files"); +if (filesFlagIndex !== -1) { + const scopedPaths = process.argv.slice(filesFlagIndex + 1).filter((arg) => !arg.startsWith("-")); + if (scopedPaths.length === 0) { + console.error("āŒ --files needs at least one path."); + process.exit(1); + } + process.exit(validateScoped(scopedPaths)); +} + const result = validate(); const idResult = validateIds(); From 5f887a5462b85c770a6b48d3b8e763af90da1005 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 00:58:35 +0000 Subject: [PATCH 2/3] fix(scripts): reject non-YAML paths in scoped format moveIdToTop parses whatever it is handed, so passing a .md or .json path crashed inside the YAML document parser with a stack trace. The unscoped path never hit this because getYamlFiles() filters by extension; accepting arbitrary argv paths removed that guarantee. Co-Authored-By: Claude Opus 5 --- scripts/format-yaml.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/format-yaml.ts b/scripts/format-yaml.ts index ffafbaf0f..275330921 100644 --- a/scripts/format-yaml.ts +++ b/scripts/format-yaml.ts @@ -74,6 +74,17 @@ function filesToFormat(): { files: string[]; scoped: boolean } { console.error(`No such file(s):\n${missing.map((f) => ` ${f}`).join("\n")}`); process.exit(1); } + // Only YAML: moveIdToTop parses every file it is handed, so a stray + // .md or .json argument would crash inside the YAML document parser + // rather than report anything useful. The unscoped path cannot hit + // this because getYamlFiles() already filters by extension. + const notYaml = resolved.filter((file) => !/\.ya?ml$/.test(file)); + if (notYaml.length > 0) { + console.error( + `Not YAML (this script only formats .yaml/.yml):\n${notYaml.map((f) => ` ${f}`).join("\n")}` + ); + process.exit(1); + } return { files: resolved, scoped: true }; } return { From 4b1c44aba23f2635df8800f7e0289a7cf7758581 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 01:07:51 +0000 Subject: [PATCH 3/3] fix(scripts): pass prettier paths as argv, reject non-YAML Scoped paths came from argv and were interpolated into a shell command string. JSON.stringify is not shell quoting, so a filename containing $(...) or backticks would have executed. Prettier now receives the paths as an argument array, with no shell involved. Scoped validation also accepted any existing file inside a collection directory. JSON parses as YAML, so a stray .json would have validated and exited 0. Both entry points now require a .yaml/.yml extension. Raised by CodeQL and CodeRabbit on #627. Co-Authored-By: Claude Opus 5 --- README.md | 5 +++-- scripts/format-yaml.ts | 13 +++++++------ scripts/validate.ts | 7 +++++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 6724dbf85..b79b4a08f 100644 --- a/README.md +++ b/README.md @@ -152,8 +152,9 @@ pnpm install # Validate all YAML files pnpm validate -# Validate only the files you touched (fast pre-flight while editing). -# Skips cross-file checks, so still run the full `pnpm validate` first. +# Validate only the files you touched: a fast pre-flight while editing. +# It skips cross-file checks (duplicate IDs, supersedes targets and +# cycles), so run the full `pnpm validate` above before committing. pnpm validate --files data/hardware/some-entry.yaml # Format all YAML files, or only the ones you touched diff --git a/scripts/format-yaml.ts b/scripts/format-yaml.ts index 275330921..3039cac39 100644 --- a/scripts/format-yaml.ts +++ b/scripts/format-yaml.ts @@ -12,7 +12,7 @@ * import touched turns a whole-catalog pass into an instant one. */ -import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; import { parseDocument } from "yaml"; @@ -108,11 +108,12 @@ if (reordered > 0) { console.log(scoped ? `Running Prettier on ${files.length} file(s)...` : "Running Prettier..."); // Prettier is given explicit paths when scoped so it never walks the -// whole data tree; the glob stays the default for a full run. -const targets = scoped - ? files.map((file) => JSON.stringify(path.relative(REPO_ROOT, file))).join(" ") - : '"data/**/*.yaml"'; -execSync(`prettier --write ${targets}`, { +// whole data tree; the glob stays the default for a full run. These are +// passed as argv rather than interpolated into a command string: the +// scoped paths come from argv, and with no shell in between there is no +// quoting to get wrong. Prettier expands the glob itself. +const targets = scoped ? files.map((file) => path.relative(REPO_ROOT, file)) : ["data/**/*.yaml"]; +execFileSync("prettier", ["--write", ...targets], { cwd: REPO_ROOT, stdio: "inherit", }); diff --git a/scripts/validate.ts b/scripts/validate.ts index c0bd9941c..9d86b9820 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -1980,6 +1980,13 @@ function validateScoped(paths: string[]): number { console.error(`āŒ No such file: ${given}`); return 1; } + // JSON is valid YAML, so without this a stray .json inside a collection + // directory would parse, validate and exit 0 ā€” reporting success for a + // file the catalog never reads. + if (!/\.ya?ml$/.test(file)) { + console.error(`āŒ Not YAML (this mode only validates .yaml/.yml): ${given}`); + return 1; + } const collection = path.basename(path.dirname(file)); const schema = COLLECTION_SCHEMAS[collection]; if (!schema) {