From a7706b5659211e82bd84b47478d38990cb157deb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:54:29 +0000 Subject: [PATCH 1/6] feat(validate): close id, connection and jack rules E129 pins id to the 21-char nanoid shape and E207 finds an id shared across collections. W121 and W128 are promoted to E130 and E131 now the corpus is clean (sc, the optical MADI connector, joins io-connections). W134/W135 warn on a name or url shared within a collection, from one grouping in lib/duplicates.ts that dataset-audit reuses in place of the always-zero broken-compatible check. W136 warns on hardware and accessories with no description, and prose fields are strings only. The audit also reports stale prices per maker. Every code has a VALIDATION_ERRORS.md entry; E304 and W124 to W127 gain the entries they lacked. Part of AUREO-1483 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- .coderabbit.yaml | 7 +- .github/workflows/dataset-audit.yml | 29 +- CLAUDE.md | 10 +- docs/VALIDATION_ERRORS.md | 176 ++++++++++-- schema/CONTEXT.md | 1 + schema/io-connections.yaml | 8 +- schema/json/hardware.json | 2 +- scripts/__tests__/dataset-audit.test.ts | 60 +++- scripts/__tests__/duplicates.test.ts | 62 +++++ scripts/__tests__/identifier-fallback.test.ts | 4 +- scripts/__tests__/validate.test.ts | 103 +++++-- scripts/dataset-audit.ts | 203 +++++++++----- scripts/generate-json-schemas.ts | 2 +- scripts/io-quality.ts | 4 +- scripts/lib/duplicates.ts | 111 ++++++++ scripts/lib/entry-id.ts | 13 + scripts/lib/error-codes.ts | 50 +++- scripts/lib/io-heuristics.ts | 2 +- scripts/validate.ts | 261 +++++++++++++----- 19 files changed, 882 insertions(+), 226 deletions(-) create mode 100644 scripts/__tests__/duplicates.test.ts create mode 100644 scripts/lib/duplicates.ts create mode 100644 scripts/lib/entry-id.ts diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 3860048db..4967ae776 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -51,9 +51,10 @@ reviews: - Distinct concurrent SKUs (different I/O or capabilities) must each be their own top-level entry — never a nested `versions` entry. - Check for potential duplicates: - - Flag if product name is very similar to an existing entry - - Flag if url matches another entry in the same collection + Check for potential duplicates (an exact name or url collision within + a collection is mechanised: `pnpm validate` warns W134/W135 and the + monthly dataset audit lists the groups, so flag only what a + normalised exact match cannot see): - Flag if the same product might exist under a different slug/name - Software products should NOT be in manufacturers/ (they belong in software/) diff --git a/.github/workflows/dataset-audit.yml b/.github/workflows/dataset-audit.yml index 35fbaf37c..cee42e85e 100644 --- a/.github/workflows/dataset-audit.yml +++ b/.github/workflows/dataset-audit.yml @@ -59,10 +59,12 @@ jobs: const title = '🔎 Dataset Audit'; const byCheck = audit.summary.byCheck; - // Actionable = needs a data fix. Orphan manufacturers and thin - // descriptions are informational and large; they don't gate the - // issue open/closed (otherwise it would never close). - const actionable = byCheck.duplicateName + byCheck.brokenCompatible; + // Actionable = needs a data fix. Orphan manufacturers, thin + // descriptions and stale prices are informational and large; + // they don't gate the issue open/closed (otherwise it would + // never close). Broken compatibleWith used to be counted here + // and was always zero, since E206 fails validation on it. + const actionable = byCheck.duplicateName + byCheck.duplicateUrl; const { data: issues } = await github.rest.issues.listForRepo({ owner: context.repo.owner, @@ -94,7 +96,7 @@ jobs: } const dupes = audit.findings.filter(f => f.check === 'duplicate-name'); - const broken = audit.findings.filter(f => f.check === 'broken-compatible'); + const dupeUrls = audit.findings.filter(f => f.check === 'duplicate-url'); const coverageRows = Object.entries(audit.coverage).map(([name, c]) => { const p = (n) => c.total ? `${Math.round((n / c.total) * 100)}%` : '—'; @@ -113,9 +115,10 @@ jobs: | Check | Count | Action | |-------|-------|--------| | Duplicate-name groups | ${byCheck.duplicateName} | Merge, fold into \`variants\`, or disambiguate | - | Broken \`compatibleWith\` | ${byCheck.brokenCompatible} | Fix the slug or remove the reference | + | Duplicate-url groups | ${byCheck.duplicateUrl} | Merge, or give each entry its own product page | | Orphan manufacturers | ${byCheck.orphanManufacturer} | Informational — manufacturers with no products | | Thin descriptions | ${byCheck.thinDescription} | Informational — descriptions under 40 chars | + | Makers with stale prices | ${byCheck.priceStaleness} | Informational — prices with no \`asOf\` or older than 18 months | | Flagged for LLM review | ${audit.summary.flagged} | \`/dataset-review\` | ### Coverage @@ -144,16 +147,18 @@ jobs: `); } - if (broken.length > 0) { - body += dedent(`### Broken \`compatibleWith\` references (${broken.length}) + if (dupeUrls.length > 0) { + body += dedent(`### Duplicate-url groups (${dupeUrls.length}) + + Same collection + normalized \`url\`. Usually one product filed twice; otherwise each entry needs its own product page.
Click to expand - | File | Detail | - |------|------| - ${broken.slice(0, 100).map(f => `| \`${cell(f.files[0])}\` | ${cell(f.detail)} |`).join('\n')} - ${broken.length > 100 ? `\n*... and ${broken.length - 100} more*` : ''} + | Collection | Files | Detail | + |------|------|------| + ${dupeUrls.slice(0, 100).map(f => `| ${cell(f.collection)} | ${f.files.map(x => `\`${cell(x)}\``).join(' + ')} | ${cell(f.detail)} |`).join('\n')} + ${dupeUrls.length > 100 ? `\n*... and ${dupeUrls.length - 100} more*` : ''}
diff --git a/CLAUDE.md b/CLAUDE.md index 838c71b89..950d9128f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -409,14 +409,16 @@ io: **IO field validation** uses a two-tier system: -- **Strict (errors, blocks CI):** `signalFlow`, `category`, `type`, `position`, `price.currency` +- **Strict (errors, blocks CI):** `signalFlow`, `category`, `type`, `connection`, `position`, `price.currency` - `signalFlow`: input, output, bidirectional - `category`: audio, midi, digital, power - `type`: closed vocabulary in `schema/io-types.yaml` — unknown values fail with E117 + - `connection`: closed vocabulary in `schema/io-connections.yaml` — unknown values fail + with E130 (the advisory W121 until the corpus carried one unknown value) - `position`: Top, Bottom, Left, Right - `currency`: ISO 4217 codes (USD, EUR, GBP, etc.) — see `schema/currencies.yaml` -- **Advisory (warnings, non-blocking):** `connection`, `link.type` - - Known values listed in `schema/io-connections.yaml`, `schema/link-types.yaml` +- **Advisory (warnings, non-blocking):** `link.type` + - Known values listed in `schema/link-types.yaml` - Unknown values produce warnings in `pnpm validate` output - Add new values to schema files via PR when they're confirmed valid @@ -508,7 +510,7 @@ two — bulk imports get this wrong. E.g. the Eventide H90 has 14 discrete jacks **Patchbays are the exception.** A `patch-bay` entry models each row as one `io` entry with `maxConnections` set to the point count (a 48-point TRS row, not 48 -entries), so W128 skips that category. TT/Bantam patch points use +entries), so E131 skips that category. TT/Bantam patch points use `connection: tt`. ## I/O Modelling Conventions diff --git a/docs/VALIDATION_ERRORS.md b/docs/VALIDATION_ERRORS.md index 0f833fec6..d3c54948d 100644 --- a/docs/VALIDATION_ERRORS.md +++ b/docs/VALIDATION_ERRORS.md @@ -526,6 +526,58 @@ over another scheme has no place in `links`; mention it in `details` instead. --- +### E129: Id Is Not a 21-Character Nanoid + +The `id` field is not the 21-character nanoid `pnpm assign-ids` writes +(letters, digits, `-` and `_`). Nothing else is meant to write the field: a +hand-typed `id: 1`, a slug pasted into it or a truncated paste used to +validate, build and ship, and Studio keys `supersedes` and product images on +the value. + +**Fix:** Remove the hand-written `id` and let CI assign one. An entry may +leave the field out entirely; `assign-ids.yml` fills it on the pull request. + +--- + +### E130: Unknown IO Connection + +The `connection` field in an IO entry is not in the known connections list. +This was the advisory W121 until the corpus carried a single unknown value +(`sc`, the duplex fibre connector optical MADI uses, now in the schema), so +an unknown value is the same hard error an unknown `type` is (E117). + +**Known connections are listed in:** `schema/io-connections.yaml` + +**Fix:** If the value is a real connector, add it to +`schema/io-connections.yaml` in the same PR, verified against the manual or a +photo. If it names a signal (`line`, `midi`), it belongs in `type`. `trs` is +not a connection: the schema models the jack size (`1/4-inch`, `1/8-inch`) +and a stereo jack is one entry. + +--- + +### E131: IO Entry Collapses Several Physical Jacks + +An IO entry sets `maxConnections` greater than 1 on a single-jack connection +(`1/4-inch`, `1/8-inch`, `2.5mm`, `xlr`, `mini-xlr`, `rca`, `5-pin din`, +`7-pin din`). Those connectors carry one physical link each, so a value above +1 is several jacks collapsed into one entry, the mistake bulk imports made +most. This was the advisory W128 until the corpus carried no hit. + +**Fix:** Model each physical jack as its own `io` entry with +`maxConnections: 1` (the Eventide H90 is the worked example: fourteen +discrete jacks). If the entry really is an aggregate on a connector that +carries several links (a D-sub snake, ADAT), use that `connection` instead. + +Not flagged: names describing an intentional aggregate ("All Slots", "Bank +A"), and entries whose `primaryCategory` is `patch-bay`, where a 48-point row +is one `io` entry by design. + +**Triage tip:** `pnpm io-quality` lists the wider, lower-precision worklist +(collapsed-pair names, missing I/O, layout coverage gaps). + +--- + ### E199: Validation Error A generic validation error that doesn't fall into a more specific category. @@ -663,6 +715,19 @@ reference. --- +### E207: Id Claimed by Two Entries + +The same `id` appears on two entries, in one collection or across two. +`pnpm assign-ids` draws every collection's ids from one pool, and product +images are served from R2 keyed by id alone, so an id shared by a hardware +entry and a manufacturer is a collision even though the two tables never +meet. The check used to be per collection. + +**Fix:** Keep the id on the entry that had it first (check `git log`) and +remove it from the other; CI assigns that one a fresh id. + +--- + ## Content Errors (E3xx) ### E300: Invalid Markdown @@ -732,6 +797,19 @@ description: Use the `foo` command to run it --- +### E304: Truncated Content + +The `description` ends with an ellipsis (`...`), the way a scraped page +excerpt is cut: "featuring 12 algorithms, a looper and..." is not a +description, and nothing downstream can finish it. A trailing "and more...", +"etc...", "others..." or "so on..." is a phrase rather than a cut and is not +flagged. + +**Fix:** Replace the field with the complete text from the maker's page, or +end it at the last full sentence. + +--- + ### E305: Raw HTML in Prose A prose field (`description`, `details`, `specs`, or the same under @@ -853,37 +931,59 @@ pnpm format Warnings are non-blocking — they appear in `pnpm validate` output but do not affect the exit code or CI status. They flag values that are not in the known schema but may be valid. -### W120: Unknown IO Type +Retired codes: W120 (unknown io `type`) became E117, W121 (unknown io +`connection`) became E130, W123 (unknown `compatibleWith`) became E206, W128 +(collapsed jacks) became E131 and W132 (unknown key) became E121. A code is +never reused. -The `type` field in an IO entry is not in the known types list. +### W124: Duplicate URL -**Known types are listed in:** `schema/io-types.yaml` +A `links[].url` repeats the entry's own `url` or an earlier link in the same +entry. Studio renders every link, so the reader sees the same page twice. -**Fix:** If the value is valid, add it to `schema/io-types.yaml`. If it's a mistake, correct it in the data file. Note: `type` describes the signal characteristic (e.g., `line`, `instrument`, `headphone`), not the physical connector. +**Fix:** Remove the duplicate link entry. If the two differ only in a fragment +or a tracking parameter, keep the plain one. --- -### W121: Unknown IO Connection +### W125: Link Matches Manufacturer Homepage -The `connection` field in an IO entry is not in the known connections list. +The entry's `url`, or one of its `links[].url`, is the manufacturer's homepage +(compared without scheme, `www.` and trailing slashes). The manufacturer file +already carries that page, and Studio shows the maker beside every product, so +the link adds nothing and a product `url` that is a homepage is not a product +page. -**Known connections are listed in:** `schema/io-connections.yaml` - -**Fix:** If the value is valid, add it to `schema/io-connections.yaml`. If it's a mistake, correct it in the data file. Note: `connection` describes the physical connector (e.g., `1/4-inch`, `xlr`, `usb-c`), not the signal type. +**Fix:** Point `url` at the product's own page, or remove it so the field +reads as unknown. Drop the redundant link. --- -### W128: IO Entry May Combine Multiple Physical Jacks +### W126: Specs Line Overlaps With Structured Field -An IO entry sets `maxConnections` greater than 1 on a single-jack connection type (e.g., `1/4-inch`, `xlr`, `rca`, `5-pin din`). These connectors carry one physical link each, so a value above 1 usually means several jacks were collapsed into one entry during import. +A `specs` bullet only restates what `formats` or `platforms` already declare +("- VST3, AU and AAX", "- macOS and Windows"). Studio renders both, so the +reader sees the same fact twice, and a bullet that drifts from the structured +field misleads where the field is what search filters on. -**Fix:** Model each physical jack as its own `io` entry with `maxConnections: 1` (see the Eventide H90 for an example). If the entry really is an intentional aggregate for a connector that carries multiple links (e.g., a D-sub snake), use the correct `connection` type instead. Names containing words like "all", "slots", or "bank" are treated as aggregates and not flagged. +**Fix:** Delete the bullet; keep the structured field as the single source. +A bullet that says more than the field (a version requirement, a host it is +tested in) stays. -Not flagged: entries whose `primaryCategory` is `patch-bay`. A patchbay row is a -run of identical points that users patch into as one group, so a 48-point row -stays a single `io` entry rather than 48 near-identical ones. +--- -**Triage tip:** Run `pnpm io-quality` to see the full I/O data-quality worklist (combine candidates, collapsed-pair names, missing I/O, and column/row coverage gaps). +### W127: Entry Would Benefit From searchTerms + +The name is a short all-caps acronym (`MPC`, `SM7B`, `OB-Xa`) and the entry +carries no `searchTerms`. Search composes manufacturer and name, so a reader +typing the expansion or a spaced form finds nothing. + +Not flagged: ordinary words that happen to be capitalised (`BOOM`, `HALO`), +listed in `scripts/lib/acronym-exclusions.ts`. + +**Fix:** Add `searchTerms` with the expansion and the forms people type +(`SM-7B`, `SM 7B`). Do not add the product's own name or the manufacturer's +display name; both are already indexed. --- @@ -945,6 +1045,50 @@ the `prices` block: absent means "unknown", which is honest, where zero asserts --- +### W134: Name Shared With Another Entry of the Same Maker + +Two or more entries in one collection share a manufacturer and a name, compared +after lowercasing and dropping everything but letters and digits (`Pro-Q 3` and +`ProQ3`; a trailing `+` survives as `plus`, so `Prime 4+` is not `Prime 4`). That +is the same product filed twice, a cosmetic variant that belongs in one entry's +`variants`, or two distinct products whose names need disambiguating. Reported +once per group, on the first file, naming the others. + +Reviewers used to be asked to spot this by eye on every import PR. It is +advisory while `main` carries 66 groups; the monthly dataset audit lists the +same groups for review. + +**Fix:** Merge the entries (keep the older id, see E207), fold a colour or +finish into `variants`, or give distinct products distinct names. + +--- + +### W135: URL Shared With Another Entry in the Collection + +Two or more entries in one collection carry the same `url`, compared without +scheme, `www.` and trailing slashes. A product page belongs to one product, so +this is usually one product filed twice, and occasionally a vendor whose entries +all point at the storefront, which is a `url` to fix rather than entries to +merge. Reported once per group. Advisory while `main` carries 150 groups. + +**Fix:** Merge duplicates, or point each entry at its own product page. A +product with no page of its own leaves `url` out. + +--- + +### W136: Hardware or Accessory Entry Has No Description + +A hardware or accessory entry carries no `description`, or an empty one. +CLAUDE.md has always required the field on both collections ("Hardware +requires: name, manufacturer, primaryCategory, description"), and the schema +never did; 33 entries on `main` carry none, so the check is staged as a warning +rather than E100 until those are filled. + +**Fix:** Write one or two sentences from the maker's page in flow scalar +format. Do not paste the maker's tagline as the description. + +--- + ## Getting Help If you encounter an error not listed here, or need clarification: diff --git a/schema/CONTEXT.md b/schema/CONTEXT.md index 22cf49fbf..6f9dc9904 100644 --- a/schema/CONTEXT.md +++ b/schema/CONTEXT.md @@ -870,6 +870,7 @@ rear-panel diagram — assign them with `pnpm enrich-io `, not during bulk - `lightning` - `aes/ebu` - `digilink` +- `sc` - `2-pin` - `db9` - `db15` diff --git a/schema/io-connections.yaml b/schema/io-connections.yaml index dfb8fbb4d..af420c04e 100644 --- a/schema/io-connections.yaml +++ b/schema/io-connections.yaml @@ -1,8 +1,9 @@ -# Valid IO connection/connector types (advisory - warning if not matched) +# Valid IO connection/connector types (closed vocabulary: E130 if not matched) # # The `connection` field describes the physical connector of an IO port. -# Add new values here as they appear in data. Unknown values generate warnings -# in `pnpm validate` and are surfaced during data review. +# An unknown value fails `pnpm validate`, the same as an unknown `type` +# (E117), so a genuinely new connector lands here in the same PR as the +# entry that needs it, verified against the manual or a photo. connections: # Quarter-inch jacks - 1/4-inch @@ -53,6 +54,7 @@ connections: - lightning - aes/ebu - digilink # Avid DigiLink (Pro Tools HD/HDX interface ports) + - sc # SC duplex fibre (optical MADI, e.g. sonible ml:mio) # Multi-pin connectors - 2-pin diff --git a/schema/json/hardware.json b/schema/json/hardware.json index db569c2ba..c325aa0d1 100644 --- a/schema/json/hardware.json +++ b/schema/json/hardware.json @@ -2481,7 +2481,7 @@ }, "connection": { "type": "string", - "description": "Physical connector (e.g., 1/4-inch, xlr, usb-c). Describes the plug/jack, not the signal type. Known values are in schema/io-connections.yaml — unknown values produce warnings." + "description": "Physical connector (e.g., 1/4-inch, xlr, usb-c). Describes the plug/jack, not the signal type. Known values are in schema/io-connections.yaml — an unknown value fails validation (E130)." }, "connectorDetail": { "type": "array", diff --git a/scripts/__tests__/dataset-audit.test.ts b/scripts/__tests__/dataset-audit.test.ts index 3e7c48b4e..9304c18ec 100644 --- a/scripts/__tests__/dataset-audit.test.ts +++ b/scripts/__tests__/dataset-audit.test.ts @@ -4,11 +4,12 @@ import { checkCvGateCategory, checkExpressionTypedCv, checkModularMissingHp, + checkPriceStaleness, computeHintCoverage, computeHpCoverage, type LoadedProduct, - normalizeName, } from "../dataset-audit.js"; +import { normalizeName } from "../lib/duplicates.js"; import type { Hardware } from "../lib/types.js"; import { DATA_DIR } from "../lib/utils.js"; @@ -179,3 +180,60 @@ describe("computeHpCoverage", () => { ).toEqual({ modular: 2, withHp: 1 }); }); }); + +describe("checkPriceStaleness", () => { + const now = new Date("2026-10-16T00:00:00Z"); + const priced = ( + slug: string, + maker: string, + prices: Record[] + ): LoadedProduct => ({ + type: "software", + slug, + file: path.join(DATA_DIR, "software", `${slug}.yaml`), + entry: { name: slug, manufacturer: maker, prices } as unknown as Hardware, + }); + + it("groups missing and stale asOf by manufacturer and leaves fresh prices alone", () => { + const findings = checkPriceStaleness( + [ + priced("a", "acme", [{ amount: 1, currency: "USD" }]), + priced("b", "acme", [{ amount: 1, currency: "USD", asOf: "2024-01-01" }]), + priced("c", "acme", [{ amount: 1, currency: "USD", asOf: "2026-09-01" }]), + priced("d", "other", [{ amount: 1, currency: "USD", asOf: "2025-04-16" }]), + priced("e", "other", [{ amount: 1, currency: "USD", asOf: "2025-04-15" }]), + ], + now + ); + expect(findings.map((f) => [f.name, f.files, f.detail])).toEqual([ + [ + "acme", + ["software/a.yaml", "software/b.yaml"], + "2 entries carry prices that need a re-check (1 with no asOf, 1 checked over 18 months ago).", + ], + [ + "other", + ["software/d.yaml", "software/e.yaml"], + "2 entries carry prices that need a re-check (2 checked over 18 months ago).", + ], + ]); + expect(findings.every((f) => f.check === "price-staleness" && !f.needsLlmReview)).toBe(true); + }); + + it("reads nested prices and caps the files it names", () => { + const many = Array.from({ length: 12 }, (_, i) => + priced(`p${i}`, "acme", [{ amount: 1, currency: "USD" }]) + ); + const nested: LoadedProduct = { + ...priced("v", "acme", []), + entry: { + name: "v", + manufacturer: "acme", + versions: [{ name: "1", prices: [{ amount: 1, currency: "USD" }] }], + } as unknown as Hardware, + }; + const [finding] = checkPriceStaleness([...many, nested], now); + expect(finding.files).toHaveLength(10); + expect(finding.detail).toMatch(/^13 entries .* the first 10 files are listed\.$/); + }); +}); diff --git a/scripts/__tests__/duplicates.test.ts b/scripts/__tests__/duplicates.test.ts new file mode 100644 index 000000000..08f7282af --- /dev/null +++ b/scripts/__tests__/duplicates.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; +import { + findDuplicateNameGroups, + findDuplicateUrlGroups, + normalizeName, + normalizeUrl, +} from "../lib/duplicates.js"; + +describe("normalizeUrl", () => { + it("drops scheme, www and trailing slashes and lowercases the rest", () => { + expect(normalizeUrl("HTTPS://www.Example.com/Serum/")).toBe("example.com/serum"); + expect(normalizeUrl(" http://example.com ")).toBe("example.com"); + }); +}); + +describe("findDuplicateNameGroups", () => { + it("groups by collection, manufacturer and normalized name, once per group", () => { + const groups = findDuplicateNameGroups([ + { collection: "software", file: "a.yaml", name: "Pro-Q 3", manufacturer: "fabfilter" }, + { collection: "software", file: "b.yaml", name: "ProQ3", manufacturer: "fabfilter" }, + { collection: "software", file: "c.yaml", name: "ProQ3", manufacturer: "other" }, + { collection: "hardware", file: "d.yaml", name: "ProQ3", manufacturer: "fabfilter" }, + { collection: "software", file: "e.yaml", name: "ProQ3+", manufacturer: "fabfilter" }, + ]); + expect(groups).toEqual([ + { + collection: "software", + value: "Pro-Q 3", + entries: [ + { collection: "software", file: "a.yaml", name: "Pro-Q 3", manufacturer: "fabfilter" }, + { collection: "software", file: "b.yaml", name: "ProQ3", manufacturer: "fabfilter" }, + ], + }, + ]); + }); + + it("never groups an entry without a manufacturer or a name", () => { + expect( + findDuplicateNameGroups([ + { collection: "manufacturers", file: "a.yaml", name: "Acme" }, + { collection: "manufacturers", file: "b.yaml", name: "Acme" }, + { collection: "software", file: "c.yaml", manufacturer: "acme" }, + { collection: "software", file: "d.yaml", manufacturer: "acme" }, + ]) + ).toEqual([]); + expect(normalizeName("")).toBe(""); + }); +}); + +describe("findDuplicateUrlGroups", () => { + it("groups by collection and normalized url", () => { + const groups = findDuplicateUrlGroups([ + { collection: "software", file: "a.yaml", url: "https://x.example/serum" }, + { collection: "software", file: "b.yaml", url: "http://www.x.example/serum/" }, + { collection: "hardware", file: "c.yaml", url: "https://x.example/serum" }, + { collection: "software", file: "d.yaml" }, + ]); + expect(groups.map((g) => [g.collection, g.value, g.entries.map((e) => e.file)])).toEqual([ + ["software", "https://x.example/serum", ["a.yaml", "b.yaml"]], + ]); + }); +}); diff --git a/scripts/__tests__/identifier-fallback.test.ts b/scripts/__tests__/identifier-fallback.test.ts index 032f938d6..9d18cdac0 100644 --- a/scripts/__tests__/identifier-fallback.test.ts +++ b/scripts/__tests__/identifier-fallback.test.ts @@ -104,9 +104,9 @@ describe("buildDatabase with fallback identifiers", () => { const ids = { acme: "MFR00000000000000acme", shared: "SW0000000000000shared", - app: "SW00000000000000000app", + app: "SW0000000000000000app", mixed: "SW00000000000000mixed", - collection: "SW00000000collection", + collection: "SW000000000collection", }; let tmp: string; diff --git a/scripts/__tests__/validate.test.ts b/scripts/__tests__/validate.test.ts index 4c4498a8a..846d76cf2 100644 --- a/scripts/__tests__/validate.test.ts +++ b/scripts/__tests__/validate.test.ts @@ -11,7 +11,6 @@ import { detectSupersedeCycle, getErrorCodeFromZodIssue, isValidCategory, - normalizeToMarkdownString, validateFile, validateMarkdown, type WarningContext, @@ -286,17 +285,6 @@ describe("validateMarkdown", () => { }); }); -describe("normalizeToMarkdownString", () => { - it("returns a string unchanged", () => { - expect(normalizeToMarkdownString("one\ntwo")).toBe("one\ntwo"); - }); - - it("joins an array with blank lines as paragraph breaks", () => { - expect(normalizeToMarkdownString(["one", "two"])).toBe("one\n\ntwo"); - expect(normalizeToMarkdownString([])).toBe(""); - }); -}); - describe("detectSupersedeCycle", () => { const slugs = new Map([ ["a", "alpha"], @@ -399,6 +387,71 @@ describe("validateFile", () => { expect(validateFile(file, COLLECTION_SCHEMAS.hardware, MAKERS)).toBeNull(); }); + it("rejects an id that is not a 21-character nanoid as E129, on every collection", () => { + for (const bad of ["1", "serum", "aaaaaaaaaaaaaaaaaaaa", "aaaaaaaaaaaaaaaaaaaa!"]) { + const file = writeEntry("hardware", HARDWARE_OK.replace("aaaaaaaaaaaaaaaaaaaaa", bad)); + const result = validateFile(file, COLLECTION_SCHEMAS.hardware, MAKERS); + expect(result?.details, bad).toHaveLength(1); + expect(result?.details?.[0], bad).toMatchObject({ + code: ValidationErrorCode.E129_INVALID_ID_FORMAT, + path: "id", + line: 1, + }); + } + const maker = writeEntry("manufacturers", "id: nope\nname: Acme\n"); + expect( + validateFile(maker, COLLECTION_SCHEMAS.manufacturers, MAKERS)?.details?.[0] + ).toMatchObject({ code: ValidationErrorCode.E129_INVALID_ID_FORMAT, path: "id" }); + // Optional: assign-ids writes it after validate.yml's first run. + const none = writeEntry("hardware", HARDWARE_OK.replace("id: aaaaaaaaaaaaaaaaaaaaa\n", "")); + expect(validateFile(none, COLLECTION_SCHEMAS.hardware, MAKERS)).toBeNull(); + }); + + it("rejects a YAML array in a prose field as E101", () => { + // CLAUDE.md has always asked for a block scalar; the union that accepted + // an array was dropped once the corpus carried none. + const file = writeEntry("hardware", `${HARDWARE_OK}specs:\n - one\n - two\n`); + expect(validateFile(file, COLLECTION_SCHEMAS.hardware, MAKERS)?.details?.[0]).toMatchObject({ + code: ValidationErrorCode.E101_INVALID_FIELD_TYPE, + path: "specs", + }); + }); + + it("rejects an unknown io connection as E130, keyed to the connection line", () => { + // The advisory W121 until the vocabulary was clean; `trs` is the + // mistake it meets most (the schema models the jack size instead). + const file = writeEntry( + "hardware", + HARDWARE_OK.replace("connection: 1/4-inch", "connection: trs") + ); + const result = validateFile(file, COLLECTION_SCHEMAS.hardware, MAKERS); + expect(result?.details).toHaveLength(1); + expect(result?.details?.[0]).toMatchObject({ + code: ValidationErrorCode.E130_UNKNOWN_IO_CONNECTION, + path: "io.0.connection", + line: 13, + }); + }); + + it("rejects several jacks collapsed into one io entry as E131, unless the entry is a patchbay", () => { + const collapsed = HARDWARE_OK.replace("maxConnections: 1", "maxConnections: 2"); + const result = validateFile( + writeEntry("hardware", collapsed), + COLLECTION_SCHEMAS.hardware, + MAKERS + ); + expect(result?.details).toHaveLength(1); + expect(result?.details?.[0]).toMatchObject({ + code: ValidationErrorCode.E131_IO_COLLAPSED_JACKS, + path: "io.0.maxConnections", + line: 14, + }); + const patchbay = collapsed.replace("primaryCategory: pedal", "primaryCategory: patch-bay"); + expect( + validateFile(writeEntry("hardware", patchbay), COLLECTION_SCHEMAS.hardware, MAKERS) + ).toBeNull(); + }); + it("rejects a negative or non-finite price amount as E127", () => { // `.nan` and `.inf` are reachable from YAML, and z.number() refuses // both at the base type check, so they never reach the `.check()`. @@ -783,10 +836,16 @@ describe("collectWarnings", () => { expect(collectWarnings(file, data as WarningContext, document, lineCounter)).toBeNull(); }); - it("W121: unknown io connection, keyed to the connection line", () => { - expect(warn(HARDWARE_OK.replace("connection: 1/4-inch", "connection: trs"))).toEqual([ - { code: "W121", path: "io[0].connection", line: 13 }, + it("W136: a hardware or accessory entry with no description, keyed to the name line", () => { + // CLAUDE.md requires the field on both collections; the schema never + // did, and 33 entries on main carry none, so it is staged as a warning. + const bare = HARDWARE_OK.replace("description: A pedal.\n", ""); + expect(warn(bare)).toEqual([{ code: "W136", path: "description", line: 2 }]); + expect(warn(bare.replace("categories:", "description: ''\ncategories:"))).toEqual([ + { code: "W136", path: "description", line: 2 }, ]); + expect(warn(bare, {}, "accessories")).toEqual([{ code: "W136", path: "description", line: 2 }]); + expect(warn(bare, {}, "software")).toEqual([]); }); it("W133: a free price with no provenance, and none once it carries some", () => { @@ -819,14 +878,6 @@ describe("collectWarnings", () => { ).toEqual([{ code: "W133", path: "versions[0].prices[0]", line: 19 }]); }); - it("W128: several jacks collapsed into one entry, unless the entry is a patchbay", () => { - const collapsed = HARDWARE_OK.replace("maxConnections: 1", "maxConnections: 2"); - expect(warn(collapsed)).toEqual([{ code: "W128", path: "io[0].maxConnections", line: 14 }]); - expect(warn(collapsed.replace("primaryCategory: pedal", "primaryCategory: patch-bay"))).toEqual( - [] - ); - }); - it("W124: a link repeating url or an earlier link", () => { const yaml = `${HARDWARE_OK}url: https://acme.example/widget links: @@ -923,9 +974,9 @@ specs: |- it("accumulates every warning the file earns", () => { const yaml = HARDWARE_OK.replace("name: Widget", "name: ADT | Acme").replace( - "connection: 1/4-inch", - "connection: trs" + "description: A pedal.\n", + "" ); - expect(warn(yaml).map((w) => w.code)).toEqual(["W121", "W130"]); + expect(warn(yaml).map((w) => w.code)).toEqual(["W136", "W130"]); }); }); diff --git a/scripts/dataset-audit.ts b/scripts/dataset-audit.ts index d48e938e6..e07e78812 100644 --- a/scripts/dataset-audit.ts +++ b/scripts/dataset-audit.ts @@ -6,9 +6,8 @@ * per-file: they check each entry in isolation. As the catalog grows past * ~12k entries — with imports dropping hundreds of products per PR — the * problems that slip through are *cross-entry*: the same product filed - * twice, a `compatibleWith` pointing at a slug that doesn't exist, a - * manufacturer with no products, descriptions that are technically present - * but useless. + * twice, a manufacturer with no products, descriptions that are + * technically present but useless, prices nobody has checked in years. * * This script loads the WHOLE dataset and runs the deterministic * cross-entry checks that scale with dataset size (cheap, no LLM). It is @@ -25,9 +24,11 @@ * only (not fuzzy) — a product catalog is full of * model-number families ("Bassysm-F" vs "Bassysm-J") * that edit-distance matching drowns you in. - * 2. broken-compatible — `compatibleWith` slug resolves to no software or - * hardware entry. (validate fails on this as E206; - * kept here so the audit report stays complete.) + * The same groups `pnpm validate` warns on (W134), + * from the one implementation in lib/duplicates.ts. + * 2. duplicate-url — same collection + normalized `url` (W135 in + * validate). Two entries on one product page are + * usually one product filed twice. * 3. orphan-manufacturer — a manufacturer referenced by zero products. * 4. thin-description — description present but suspiciously short. * 5. aggregator-url — canonical `url` points at an aggregator @@ -43,6 +44,13 @@ * 8. modular-missing-hp — a hardware entry filed under `modular` with no * `hp`, so the faceplate width campaign can see * its remaining coverage gap (AUREO-1110). + * 9. price-staleness — prices with no `asOf`, or one older than + * eighteen months, grouped by manufacturer so a + * re-check is one vendor's store at a time. + * + * `broken-compatible` used to be check 2. `compatibleWith` to an unknown + * slug has been the error E206 since the advisory W123 was promoted, so + * the count was always zero and the monthly issue gated on it anyway. * * Findings with `needsLlmReview: true` are collected into `flagged`, the * seam handed to Tier 2. Deterministic findings (broken refs, orphans) are @@ -57,8 +65,10 @@ import path from "node:path"; import { z } from "zod"; import { isAggregatorUrl, isManufacturerOwnDomain, urlHost } from "./lib/aggregator-domains.js"; +import { findDuplicateNameGroups, findDuplicateUrlGroups } from "./lib/duplicates.js"; import { type FindingInput, findingsDirArg, recordFindings } from "./lib/findings.js"; import { countHintCoverage, emptyHintCoverage, type HintCoverage } from "./lib/io-hints.js"; +import { collectPriceArrays } from "./lib/price-terms.js"; import type { Accessory, Content, Hardware, Manufacturer, Software } from "./lib/types.js"; import { DATA_DIR, getYamlFiles, loadYamlFile, REPO_ROOT } from "./lib/utils.js"; @@ -75,6 +85,10 @@ type Product = Software | Content | Hardware | Accessory; const THIN_DESCRIPTION_CHARS = 40; /** Cap on the LLM-bound `flagged` list, prioritized by severity. */ const MAX_FLAGGED = 100; +/** A price checked longer ago than this is stale, whatever the vendor. */ +const PRICE_STALE_MONTHS = 18; +/** Files named on a per-manufacturer finding, so a vendor with 300 entries stays one row. */ +const STALE_PRICE_FILES_SHOWN = 10; // ============================================================================= // SCHEMAS @@ -82,7 +96,7 @@ const MAX_FLAGGED = 100; const CheckSchema = z.enum([ "duplicate-name", - "broken-compatible", + "duplicate-url", "orphan-manufacturer", "thin-description", "aggregator-url", @@ -92,6 +106,7 @@ const CheckSchema = z.enum([ "cv-gate-category", "expression-typed-cv", "modular-missing-hp", + "price-staleness", ]); const SeveritySchema = z.enum(["blocking", "warning", "info"]); @@ -143,7 +158,7 @@ const DatasetAuditSchema = z.object({ manufacturers: z.number(), byCheck: z.object({ duplicateName: z.number(), - brokenCompatible: z.number(), + duplicateUrl: z.number(), orphanManufacturer: z.number(), thinDescription: z.number(), aggregatorUrl: z.number(), @@ -153,6 +168,7 @@ const DatasetAuditSchema = z.object({ cvGateCategory: z.number(), expressionTypedCv: z.number(), modularMissingHp: z.number(), + priceStaleness: z.number(), }), flagged: z.number(), }), @@ -198,20 +214,6 @@ function slugOf(file: string): string { return path.basename(file, path.extname(file)); } -/** Collapse a display name to a comparable key (lowercase, alphanumerics only). */ -export function normalizeName(name: string): string { - return ( - name - .toLowerCase() - // A "+" marks a distinct SKU, so preserve it as a "plus" token instead of - // stripping it. This covers both a trailing "+" (ProFX10v3 vs ProFX10v3+, - // Prime 4 vs Prime 4+) and an internal "+" that precedes more of the name - // (Vision+ Console vs Vision Console, SPS-1UW+ MKII vs SPS-1UW MKII). - .replace(/\+/g, "plus") - .replace(/[^a-z0-9]+/g, "") - ); -} - function descriptionText(entry: Product): string { return (entry.description ?? "").trim(); } @@ -220,12 +222,6 @@ function hasPrices(entry: Product): boolean { return Array.isArray(entry.prices) && entry.prices.length > 0; } -/** `compatibleWith` only exists on software and content in the schema. */ -function compatibleWithOf(entry: Product): string[] { - const value = (entry as Software | Content).compatibleWith; - return Array.isArray(value) ? value : []; -} - // ============================================================================= // LOAD // ============================================================================= @@ -233,15 +229,12 @@ function compatibleWithOf(entry: Product): string[] { interface Dataset { products: LoadedProduct[]; manufacturers: { slug: string; file: string; entry: Manufacturer }[]; - /** Slugs that a `compatibleWith` is allowed to point at (software + hardware). */ - hostSlugs: Set; /** Manufacturer slugs referenced by at least one product. */ usedManufacturers: Set; } function loadDataset(): Dataset { const products: LoadedProduct[] = []; - const hostSlugs = new Set(); const usedManufacturers = new Set(); for (const type of PRODUCT_TYPES) { @@ -249,7 +242,6 @@ function loadDataset(): Dataset { const entry = loadYamlFile(file); const slug = slugOf(file); products.push({ type, slug, file, entry }); - if (type === "software" || type === "hardware") hostSlugs.add(slug); if (entry.manufacturer) usedManufacturers.add(entry.manufacturer); } } @@ -260,7 +252,19 @@ function loadDataset(): Dataset { entry: loadYamlFile(file), })); - return { products, manufacturers, hostSlugs, usedManufacturers }; + return { products, manufacturers, usedManufacturers }; +} + +/** A product as the duplicate finder reads it: collection, path, name, maker, url. */ +function duplicateCandidate(p: LoadedProduct) { + return { + collection: p.type, + file: relPath(p.file), + name: p.entry.name, + manufacturer: p.entry.manufacturer, + url: (p.entry as { url?: string }).url, + product: p, + }; } // ============================================================================= @@ -275,23 +279,13 @@ function loadDataset(): Dataset { * "Bassysm-J", "Amethyst" vs "Amethyst2") that edit-distance matching would * flag by the thousands. An exact collision is still worth an LLM glance — * usually a duplicate or a cosmetic variant that belongs in one entry's - * `variants`, occasionally a genuine pair of distinct products. + * `variants`, occasionally a genuine pair of distinct products. The + * grouping is `lib/duplicates.ts`, shared with validate's W134. */ function checkDuplicateNames(products: LoadedProduct[]): Finding[] { - const buckets = new Map(); - for (const p of products) { - const norm = normalizeName(p.entry.name); - if (!norm) continue; - const key = `${p.type} ${p.entry.manufacturer ?? ""} ${norm}`; - const list = buckets.get(key) ?? []; - list.push(p); - buckets.set(key, list); - } - const findings: Finding[] = []; - for (const group of buckets.values()) { - if (group.length < 2) continue; - const first = group[0]; + for (const group of findDuplicateNameGroups(products.map(duplicateCandidate))) { + const first = group.entries[0].product; findings.push({ check: "duplicate-name", severity: "warning", @@ -299,9 +293,9 @@ function checkDuplicateNames(products: LoadedProduct[]): Finding[] { collection: first.type, name: first.entry.name, manufacturer: first.entry.manufacturer, - files: group.map((g) => relPath(g.file)), + files: group.entries.map((e) => e.file), detail: - `${group.length} entries share the normalized name "${normalizeName(first.entry.name)}" ` + + `${group.entries.length} entries share the name "${group.value}" ` + "under the same manufacturer — likely the same product filed more than once " + "(cosmetic variants belong in a single entry's `variants`).", }); @@ -310,24 +304,95 @@ function checkDuplicateNames(products: LoadedProduct[]): Finding[] { } /** - * `compatibleWith` slugs that resolve to no software/hardware entry. These - * are deterministic data errors — actionable without LLM judgment. + * Same collection + normalized `url`. A product page belongs to one + * product, so two entries on it are usually one product filed twice, and + * occasionally a vendor whose pages are one storefront (which is a url to + * fix, not an entry to merge). Shared with validate's W135. */ -function checkBrokenCompatible(products: LoadedProduct[], hostSlugs: Set): Finding[] { +function checkDuplicateUrls(products: LoadedProduct[]): Finding[] { const findings: Finding[] = []; - for (const p of products) { - const refs = compatibleWithOf(p.entry); - const broken = refs.filter((slug) => !hostSlugs.has(slug)); - if (broken.length === 0) continue; + for (const group of findDuplicateUrlGroups(products.map(duplicateCandidate))) { + const first = group.entries[0].product; findings.push({ - check: "broken-compatible", + check: "duplicate-url", severity: "warning", + needsLlmReview: true, + collection: first.type, + name: first.entry.name, + manufacturer: first.entry.manufacturer, + files: group.entries.map((e) => e.file), + detail: + `${group.entries.length} entries share the url ${group.value} — ` + + "likely the same product filed more than once; otherwise each needs its own product page.", + }); + } + return findings; +} + +/** Months between two dates, whole, ignoring the day. */ +function monthsBetween(from: Date, to: Date): number { + return ( + (to.getUTCFullYear() - from.getUTCFullYear()) * 12 + (to.getUTCMonth() - from.getUTCMonth()) + ); +} + +/** + * Prices nobody has checked lately: `asOf` missing, or older than + * eighteen months. One finding per manufacturer rather than per entry, + * because a re-check is a pass over one vendor's store and a list of + * 2,000 entries is not a worklist. Informational, and never a Tier-2 + * finding: the fix is to open the page, which no model can do here. + */ +export function checkPriceStaleness( + products: readonly LoadedProduct[], + now = new Date() +): Finding[] { + interface Tally { + manufacturer: string; + files: string[]; + missing: number; + stale: number; + } + const byMaker = new Map(); + for (const p of products) { + let missing = 0; + let stale = 0; + for (const { prices } of collectPriceArrays(p.entry)) { + for (const price of prices) { + const asOf = (price as { asOf?: unknown }).asOf; + const checked = + asOf instanceof Date ? asOf : typeof asOf === "string" && asOf ? new Date(asOf) : null; + if (!checked || Number.isNaN(checked.getTime())) missing++; + else if (monthsBetween(checked, now) >= PRICE_STALE_MONTHS) stale++; + } + } + if (missing === 0 && stale === 0) continue; + const maker = p.entry.manufacturer ?? "(none)"; + const tally = byMaker.get(maker) ?? { manufacturer: maker, files: [], missing: 0, stale: 0 }; + tally.files.push(relPath(p.file)); + tally.missing += missing; + tally.stale += stale; + byMaker.set(maker, tally); + } + + const findings: Finding[] = []; + for (const tally of [...byMaker.values()].sort((a, b) => b.files.length - a.files.length)) { + const parts: string[] = []; + if (tally.missing > 0) parts.push(`${tally.missing} with no asOf`); + if (tally.stale > 0) parts.push(`${tally.stale} checked over ${PRICE_STALE_MONTHS} months ago`); + findings.push({ + check: "price-staleness", + severity: "info", needsLlmReview: false, - collection: p.type, - name: p.entry.name, - manufacturer: p.entry.manufacturer, - files: [relPath(p.file)], - detail: `compatibleWith references no existing software/hardware: ${broken.join(", ")}`, + collection: "prices", + name: tally.manufacturer, + manufacturer: tally.manufacturer, + files: tally.files.slice(0, STALE_PRICE_FILES_SHOWN), + detail: + `${tally.files.length} entries carry prices that need a re-check (${parts.join(", ")})` + + (tally.files.length > STALE_PRICE_FILES_SHOWN + ? `; the first ${STALE_PRICE_FILES_SHOWN} files are listed.` + : "."), }); } return findings; @@ -737,7 +802,7 @@ function generateAudit(fast: boolean): DatasetAudit { const findings: Finding[] = [ ...checkDuplicateNames(dataset.products), - ...checkBrokenCompatible(dataset.products, dataset.hostSlugs), + ...checkDuplicateUrls(dataset.products), ...checkOrphanManufacturers(dataset), ...(fast ? [] : checkThinDescriptions(dataset.products)), ...checkAggregatorUrls(dataset), @@ -747,6 +812,7 @@ function generateAudit(fast: boolean): DatasetAudit { ...checkCvGateCategory(dataset.products), ...checkExpressionTypedCv(dataset.products), ...checkModularMissingHp(dataset.products), + ...checkPriceStaleness(dataset.products), ]; // The Tier-2 seam: only ambiguous findings, highest severity first, capped. @@ -757,7 +823,7 @@ function generateAudit(fast: boolean): DatasetAudit { const byCheck = { duplicateName: findings.filter((f) => f.check === "duplicate-name").length, - brokenCompatible: findings.filter((f) => f.check === "broken-compatible").length, + duplicateUrl: findings.filter((f) => f.check === "duplicate-url").length, orphanManufacturer: findings.filter((f) => f.check === "orphan-manufacturer").length, thinDescription: findings.filter((f) => f.check === "thin-description").length, aggregatorUrl: findings.filter((f) => f.check === "aggregator-url").length, @@ -767,6 +833,7 @@ function generateAudit(fast: boolean): DatasetAudit { cvGateCategory: findings.filter((f) => f.check === "cv-gate-category").length, expressionTypedCv: findings.filter((f) => f.check === "expression-typed-cv").length, modularMissingHp: findings.filter((f) => f.check === "modular-missing-hp").length, + priceStaleness: findings.filter((f) => f.check === "price-staleness").length, }; return { @@ -802,7 +869,7 @@ function printConsoleReport(audit: DatasetAudit): void { console.log(` Product entries: ${audit.summary.totalEntries}`); console.log(` Manufacturers: ${audit.summary.manufacturers}`); console.log(` Duplicate-name groups: ${audit.summary.byCheck.duplicateName}`); - console.log(` Broken compatibleWith: ${audit.summary.byCheck.brokenCompatible}`); + console.log(` Duplicate-url groups: ${audit.summary.byCheck.duplicateUrl}`); console.log(` Orphan manufacturers: ${audit.summary.byCheck.orphanManufacturer}`); console.log(` Thin descriptions: ${audit.summary.byCheck.thinDescription}`); console.log(` Aggregator urls: ${audit.summary.byCheck.aggregatorUrl}`); @@ -812,6 +879,7 @@ function printConsoleReport(audit: DatasetAudit): void { console.log(` CV/clock not audio: ${audit.summary.byCheck.cvGateCategory}`); console.log(` Expression typed cv: ${audit.summary.byCheck.expressionTypedCv}`); console.log(` Modular without hp: ${audit.summary.byCheck.modularMissingHp}`); + console.log(` Makers with stale prices: ${audit.summary.byCheck.priceStaleness}`); console.log(` Flagged for LLM review: ${audit.summary.flagged}`); console.log(); @@ -848,7 +916,7 @@ function printConsoleReport(audit: DatasetAudit): void { const groups: { title: string; check: Finding["check"]; limit: number }[] = [ { title: "🔁 Duplicate-name groups", check: "duplicate-name", limit: 30 }, - { title: "🔗 Broken compatibleWith references", check: "broken-compatible", limit: 30 }, + { title: "🔗 Duplicate-url groups", check: "duplicate-url", limit: 30 }, { title: "🏷 Orphan manufacturers", check: "orphan-manufacturer", limit: 20 }, { title: "📝 Thin descriptions", check: "thin-description", limit: 20 }, { title: "🔀 Aggregator canonical urls", check: "aggregator-url", limit: 20 }, @@ -858,6 +926,7 @@ function printConsoleReport(audit: DatasetAudit): void { { title: "🎛 CV and clock jacks outside audio", check: "cv-gate-category", limit: 20 }, { title: "🦶 Expression jacks typed cv/gate", check: "expression-typed-cv", limit: 20 }, { title: "📏 Modular entries without hp", check: "modular-missing-hp", limit: 20 }, + { title: "💸 Makers with stale prices", check: "price-staleness", limit: 20 }, ]; for (const { title, check, limit } of groups) { @@ -886,7 +955,7 @@ function printConsoleReport(audit: DatasetAudit): void { /** * The inbox rows for this audit's findings. * - * `modular-missing-hp` only, of the eleven checks. The rest are either + * `modular-missing-hp` only, of the twelve checks. The rest are either * whole-dataset relations that name several files at once * (duplicate-name, orphan-manufacturer) and so have no single entry to * close an issue against, or they already have a tool that settles them diff --git a/scripts/generate-json-schemas.ts b/scripts/generate-json-schemas.ts index 5ad59b283..e66665cf0 100644 --- a/scripts/generate-json-schemas.ts +++ b/scripts/generate-json-schemas.ts @@ -140,7 +140,7 @@ const ioSchema = { connection: { type: "string", description: - "Physical connector (e.g., 1/4-inch, xlr, usb-c). Describes the plug/jack, not the signal type. Known values are in schema/io-connections.yaml — unknown values produce warnings.", + "Physical connector (e.g., 1/4-inch, xlr, usb-c). Describes the plug/jack, not the signal type. Known values are in schema/io-connections.yaml — an unknown value fails validation (E130).", }, connectorDetail: { type: "array", items: { type: "string" } }, maxConnections: { type: "number" }, diff --git a/scripts/io-quality.ts b/scripts/io-quality.ts index 5cbcece8b..64eeaa0e6 100644 --- a/scripts/io-quality.ts +++ b/scripts/io-quality.ts @@ -17,8 +17,8 @@ * raw material the setup graph needs (densest devices first). * * The report is advisory triage: heuristics favor recall, so a human reviews - * the worklist. The high-precision subset is enforced separately as warning - * W128 in `pnpm validate`. + * the worklist. The high-precision subset is enforced separately as error + * E131 in `pnpm validate`. * * Usage: * pnpm io-quality # console report diff --git a/scripts/lib/duplicates.ts b/scripts/lib/duplicates.ts new file mode 100644 index 000000000..76813e8c1 --- /dev/null +++ b/scripts/lib/duplicates.ts @@ -0,0 +1,111 @@ +/** + * Entries that collide with another entry of the same collection. + * + * Two kinds of collision, both exact after normalisation and neither + * fuzzy. A product catalog is full of model-number families + * ("Bassysm-F" beside "Bassysm-J") that edit-distance matching would + * flag by the thousand, so the question asked here is the narrow one: + * is this the same product filed twice? + * + * - A **name** collides when the collection, the manufacturer and the + * normalised name all agree ("Pro-Q 3" and "ProQ3" under fabfilter). + * - A **url** collides when two entries in a collection point at the + * same page, read without scheme, `www.` or a trailing slash. + * + * `pnpm validate` reports each group once (W134, W135) and + * `pnpm dataset:audit` carries the same groups into the monthly issue, + * from this one implementation so the two never disagree about a pair. + * Reviewers used to be asked to spot both by eye, one import PR at a + * time, which is how 150 url groups came to sit on `main`. + */ + +export interface DuplicateCandidate { + collection: string; + /** Repo-relative path, the way the report names it. */ + file: string; + name?: string; + manufacturer?: string; + url?: string; +} + +export interface DuplicateGroup { + collection: string; + /** The value the group shares, as the first member spells it. */ + value: string; + entries: T[]; +} + +/** Collapse a display name to a comparable key (lowercase, alphanumerics only). */ +export function normalizeName(name: string): string { + return ( + name + .toLowerCase() + // A "+" marks a distinct SKU, so preserve it as a "plus" token instead of + // stripping it. This covers both a trailing "+" (ProFX10v3 vs ProFX10v3+, + // Prime 4 vs Prime 4+) and an internal "+" that precedes more of the name + // (Vision+ Console vs Vision Console, SPS-1UW+ MKII vs SPS-1UW MKII). + .replace(/\+/g, "plus") + .replace(/[^a-z0-9]+/g, "") + ); +} + +/** A url without the parts that vary between two spellings of one page. */ +export function normalizeUrl(url: string): string { + return url + .trim() + .replace(/^https?:\/\//i, "") + .replace(/^www\./i, "") + .replace(/\/+$/, "") + .toLowerCase(); +} + +function groupBy( + entries: readonly T[], + keyOf: (entry: T) => string | undefined, + sharedValue: (entry: T) => string +): DuplicateGroup[] { + const buckets = new Map(); + for (const entry of entries) { + const key = keyOf(entry); + if (!key) continue; + const list = buckets.get(`${entry.collection}\u0000${key}`) ?? []; + list.push(entry); + buckets.set(`${entry.collection}\u0000${key}`, list); + } + const groups: DuplicateGroup[] = []; + for (const group of buckets.values()) { + if (group.length < 2) continue; + groups.push({ collection: group[0].collection, value: sharedValue(group[0]), entries: group }); + } + return groups; +} + +/** + * Groups sharing collection, manufacturer and normalised name. An entry + * with no manufacturer (a manufacturer file) never groups, because the + * name alone is not enough to say two files are one product. + */ +export function findDuplicateNameGroups( + entries: readonly T[] +): DuplicateGroup[] { + return groupBy( + entries, + (entry) => { + if (!entry.name || !entry.manufacturer) return undefined; + const norm = normalizeName(entry.name); + return norm ? `${entry.manufacturer}\u0000${norm}` : undefined; + }, + (entry) => entry.name ?? "" + ); +} + +/** Groups sharing a collection and a normalised `url`. */ +export function findDuplicateUrlGroups( + entries: readonly T[] +): DuplicateGroup[] { + return groupBy( + entries, + (entry) => (entry.url ? normalizeUrl(entry.url) || undefined : undefined), + (entry) => entry.url ?? "" + ); +} diff --git a/scripts/lib/entry-id.ts b/scripts/lib/entry-id.ts new file mode 100644 index 000000000..b76266176 --- /dev/null +++ b/scripts/lib/entry-id.ts @@ -0,0 +1,13 @@ +/** + * The shape of an entry's `id`. + * + * `pnpm assign-ids` writes a 21-character nanoid (URL-safe alphabet: + * letters, digits, `-` and `_`), and nothing else is meant to write the + * field at all. Until this existed the validator checked only that the + * value was a non-empty string, so a hand-typed `id: 1`, a slug pasted + * into the field or a truncated paste validated, built and shipped, and + * Studio's `supersedes` and image lookups keyed on it. The corpus was + * clean when the pattern was introduced, which is why it is an error + * (E129) rather than a staged warning. + */ +export const ENTRY_ID_PATTERN = /^[A-Za-z0-9_-]{21}$/; diff --git a/scripts/lib/error-codes.ts b/scripts/lib/error-codes.ts index 4b5b383df..4525126ad 100644 --- a/scripts/lib/error-codes.ts +++ b/scripts/lib/error-codes.ts @@ -49,6 +49,9 @@ export enum ValidationErrorCode { E126_CONTROL_CHARACTER = "E126", E127_INVALID_PRICE_AMOUNT = "E127", E128_UNSAFE_URL_SCHEME = "E128", + E129_INVALID_ID_FORMAT = "E129", + E130_UNKNOWN_IO_CONNECTION = "E130", + E131_IO_COLLAPSED_JACKS = "E131", E199_VALIDATION_ERROR = "E199", // Reference errors (E2xx) @@ -58,6 +61,7 @@ export enum ValidationErrorCode { E203_PARENT_COMPANY_NOT_FOUND = "E203", E204_IO_TRANSLATION_MISMATCH = "E204", E206_UNKNOWN_COMPATIBLE_WITH = "E206", + E207_DUPLICATE_ID = "E207", // Content errors (E3xx) E300_INVALID_MARKDOWN = "E300", @@ -73,17 +77,17 @@ export enum ValidationErrorCode { E402_DUPLICATE_IDENTIFIER = "E402", // Advisory warnings (W1xx) — non-blocking - W120_UNKNOWN_IO_TYPE = "W120", - W121_UNKNOWN_IO_CONNECTION = "W121", W124_DUPLICATE_URL = "W124", W125_MANUFACTURER_URL_IN_LINKS = "W125", W126_SPECS_OVERLAP = "W126", W127_MISSING_SEARCH_TERMS = "W127", - W128_IO_COMBINE_CANDIDATE = "W128", W129_MANUFACTURER_IN_NAME = "W129", W130_NAME_TAGLINE = "W130", W131_PRICE_TERM_MISSING = "W131", W133_UNVERIFIED_FREE_PRICE = "W133", + W134_DUPLICATE_NAME_IN_COLLECTION = "W134", + W135_DUPLICATE_URL_IN_COLLECTION = "W135", + W136_MISSING_DESCRIPTION = "W136", } // ============================================================================= @@ -254,6 +258,18 @@ const ERROR_INFO: Record = { title: "URL scheme is not http or https", anchor: "e128-url-scheme-is-not-http-or-https", }, + [ValidationErrorCode.E129_INVALID_ID_FORMAT]: { + title: "Id is not a 21-character nanoid", + anchor: "e129-id-is-not-a-21-character-nanoid", + }, + [ValidationErrorCode.E130_UNKNOWN_IO_CONNECTION]: { + title: "Unknown IO connection", + anchor: "e130-unknown-io-connection", + }, + [ValidationErrorCode.E131_IO_COLLAPSED_JACKS]: { + title: "IO entry collapses several physical jacks", + anchor: "e131-io-entry-collapses-several-physical-jacks", + }, [ValidationErrorCode.E199_VALIDATION_ERROR]: { title: "Validation error", anchor: "e199-validation-error", @@ -284,6 +300,10 @@ const ERROR_INFO: Record = { title: "Unknown compatibleWith reference", anchor: "e206-unknown-compatiblewith-reference", }, + [ValidationErrorCode.E207_DUPLICATE_ID]: { + title: "Id claimed by two entries", + anchor: "e207-id-claimed-by-two-entries", + }, // Content errors [ValidationErrorCode.E300_INVALID_MARKDOWN]: { @@ -326,14 +346,6 @@ const ERROR_INFO: Record = { }, // Advisory warnings - [ValidationErrorCode.W120_UNKNOWN_IO_TYPE]: { - title: "Unknown IO type", - anchor: "w120-unknown-io-type", - }, - [ValidationErrorCode.W121_UNKNOWN_IO_CONNECTION]: { - title: "Unknown IO connection", - anchor: "w121-unknown-io-connection", - }, [ValidationErrorCode.W124_DUPLICATE_URL]: { title: "Duplicate URL", anchor: "w124-duplicate-url", @@ -350,10 +362,6 @@ const ERROR_INFO: Record = { title: "Entry would benefit from searchTerms", anchor: "w127-missing-search-terms", }, - [ValidationErrorCode.W128_IO_COMBINE_CANDIDATE]: { - title: "IO entry may combine multiple physical jacks", - anchor: "w128-io-entry-may-combine-multiple-physical-jacks", - }, [ValidationErrorCode.W129_MANUFACTURER_IN_NAME]: { title: "Manufacturer name duplicated in product name", anchor: "w129-manufacturer-name-duplicated-in-product-name", @@ -370,6 +378,18 @@ const ERROR_INFO: Record = { title: "Free price is missing source or asOf", anchor: "w133-free-price-is-missing-source-or-asof", }, + [ValidationErrorCode.W134_DUPLICATE_NAME_IN_COLLECTION]: { + title: "Name shared with another entry of the same maker", + anchor: "w134-name-shared-with-another-entry-of-the-same-maker", + }, + [ValidationErrorCode.W135_DUPLICATE_URL_IN_COLLECTION]: { + title: "URL shared with another entry in the collection", + anchor: "w135-url-shared-with-another-entry-in-the-collection", + }, + [ValidationErrorCode.W136_MISSING_DESCRIPTION]: { + title: "Hardware or accessory entry has no description", + anchor: "w136-hardware-or-accessory-entry-has-no-description", + }, }; // ============================================================================= diff --git a/scripts/lib/io-heuristics.ts b/scripts/lib/io-heuristics.ts index 22855a4a2..2a5de9f42 100644 --- a/scripts/lib/io-heuristics.ts +++ b/scripts/lib/io-heuristics.ts @@ -1,7 +1,7 @@ /** * Shared IO data-quality heuristics. * - * Used by both `validate.ts` (warning W128) and the `io-quality` report so the + * Used by both `validate.ts` (error E131) and the `io-quality` report so the * CI signal and the triage report stay in lockstep. */ diff --git a/scripts/validate.ts b/scripts/validate.ts index c773b4a06..f1fa1d9e2 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -13,6 +13,13 @@ import { parse as parseYaml } from "yaml"; import { z } from "zod"; import { looksLikeAcronymName } from "./lib/acronym-exclusions.js"; import { findControlCharacters, formatControlCharacterPath } from "./lib/control-characters.js"; +import { + type DuplicateCandidate, + findDuplicateNameGroups, + findDuplicateUrlGroups, + normalizeUrl, +} from "./lib/duplicates.js"; +import { ENTRY_ID_PATTERN } from "./lib/entry-id.js"; import { getDocsUrl, ValidationErrorCode } from "./lib/error-codes.js"; import { findComponentIdentifierRepeats, @@ -59,6 +66,7 @@ import { loadYamlFile, loadYamlFileWithPositions, parseErrorPath, + readRegularFile, SCHEMA_DIR, slugify, } from "./lib/utils.js"; @@ -110,6 +118,27 @@ export function isValidCategory(cat: string): boolean { return ALL_VALID_CATEGORY_INPUTS.has(cat); } +/** + * `id` as `pnpm assign-ids` writes it (E129). Optional here and required + * in the build, the same split `io[].key` takes: validate.yml runs on a + * head commit the assign-ids patch has not landed on yet. + */ +function createIdValidator() { + return z + .string() + .check((ctx) => { + if (ENTRY_ID_PATTERN.test(ctx.value)) return; + ctx.issues.push({ + code: "custom", + message: + `Invalid id '${ctx.value}': an id is the 21-character nanoid pnpm assign-ids writes ` + + "(letters, digits, '-' and '_'). Remove a hand-written id and let CI assign one.", + input: ctx.value, + }); + }) + .optional(); +} + // Capabilities are validated strictly (E119) rather than advisorily. The whole // value of the field is that two entries' lists are comparable, and a typo or a // near-miss synonym silently breaks comparability while looking populated. @@ -195,6 +224,12 @@ export function getErrorCodeFromZodIssue(issue: { const path = issue.path.join("."); const message = issue.message.toLowerCase(); + // The entry id (E129) is keyed to its path: the message names the + // value, which could carry any word the branches below key on. + if (path === "id") { + return ValidationErrorCode.E129_INVALID_ID_FORMAT; + } + // Name hygiene errors if (message.includes("name artifact")) { return ValidationErrorCode.E118_NAME_ARTIFACT; @@ -261,6 +296,12 @@ export function getErrorCodeFromZodIssue(issue: { if (message.includes("invalid io type")) { return ValidationErrorCode.E117_INVALID_IO_TYPE; } + if (message.includes("unknown io connection")) { + return ValidationErrorCode.E130_UNKNOWN_IO_CONNECTION; + } + if (message.includes("several physical jacks")) { + return ValidationErrorCode.E131_IO_COLLAPSED_JACKS; + } // Layout hint errors (E122 to E125). The cross-port rules carry a marker // phrase; a hint that fails the number check is keyed to its path so a @@ -391,20 +432,13 @@ export function validateMarkdown(content: string): { valid: boolean; error?: str } } -// Helper to normalize string or string[] to a single markdown string -export function normalizeToMarkdownString(value: string | string[]): string { - if (Array.isArray(value)) { - // Join array items with double newlines (paragraph breaks) - return value.join("\n\n"); - } - return value; -} - -// Zod schema for markdown content validation (accepts string or string[]) +// Zod schema for markdown content validation. A string only: CLAUDE.md +// has always asked for block scalars rather than YAML arrays in `details` +// and `specs`, and the corpus carried no array when the union that +// accepted one was dropped, so an array is the type error E101. const MarkdownSchema = z - .union([z.string(), z.array(z.string())]) + .string() .optional() - .transform((val) => (val ? normalizeToMarkdownString(val) : val)) .check((ctx) => { if (!ctx.value) return; const result = validateMarkdown(ctx.value); @@ -660,7 +694,21 @@ const IOSchema = z ctx.issues.push({ code: "custom", message, input: ctx.value }); } }), - connection: z.string(), + // Closed vocabulary like `type` (E117). This was the advisory W121 + // until the corpus carried one unknown value (`sc`, the optical MADI + // connector, now in the schema), so a new connector lands in + // schema/io-connections.yaml in the same PR as the entry. + connection: z.string().check((ctx) => { + if (KNOWN_IO_CONNECTIONS.has(ctx.value)) return; + const suggestion = findClosestMatch(ctx.value, KNOWN_IO_CONNECTIONS); + let message = `Unknown IO connection '${ctx.value}'.`; + if (suggestion) { + message += ` Did you mean '${suggestion}'?`; + } + message += " If this is a real connector, add it to schema/io-connections.yaml."; + message += " If it names a signal, it belongs in `type`."; + ctx.issues.push({ code: "custom", message, input: ctx.value }); + }), connectorDetail: z.array(z.string()).optional(), maxConnections: z.number(), position: z @@ -791,6 +839,7 @@ const createNameValidator = () => // ============================================================================= const ManufacturerSchema = z.object({ + id: createIdValidator(), name: createNameValidator(), companyName: z.string().optional(), parentCompany: z.string().optional(), @@ -802,6 +851,7 @@ const ManufacturerSchema = z.object({ const SoftwareSchema = z .object({ + id: createIdValidator(), name: createNameValidator(), manufacturer: z.string().min(1, "Manufacturer reference is required"), categories: z @@ -870,6 +920,7 @@ const SoftwareSchema = z const HardwareSchema = z .object({ + id: createIdValidator(), name: createNameValidator(), manufacturer: z.string().min(1, "Manufacturer reference is required"), categories: z @@ -927,6 +978,27 @@ const HardwareSchema = z } }); }) + .check((ctx) => { + // One io entry per physical jack: a single-jack connection claiming + // more than one connection is several jacks collapsed into one entry, + // the mistake bulk imports made most. The heuristic (shared with + // `pnpm io-quality`) exempts named aggregates and patchbay rows. It + // was the advisory W128 until the corpus carried no hit. + const data = ctx.value; + if (!data.io || data.io.length === 0) return; + data.io.forEach((port, i) => { + if (!isIoCombineCandidate(port, { primaryCategory: data.primaryCategory })) return; + ctx.issues.push({ + code: "custom", + message: + `io entry '${port.name}' sets maxConnections ${port.maxConnections} on a single-jack ` + + `connection ('${port.connection}'), which collapses several physical jacks into one ` + + "entry. Give each jack its own io entry with maxConnections: 1.", + path: ["io", i, "maxConnections"], + input: port, + }); + }); + }) .check((ctx) => { // IO keys must be unique within the entry (they identify ports for // Studio setup edges — see io-keys.ts). @@ -998,6 +1070,7 @@ const HardwareSchema = z // Content schema: like Software but without formats, platforms, identifiers const ContentSchema = z .object({ + id: createIdValidator(), name: createNameValidator(), manufacturer: z.string().min(1, "Manufacturer reference is required"), categories: z @@ -1044,6 +1117,7 @@ const ContentSchema = z // Accessory schema: like Hardware but without io, variants const AccessorySchema = z .object({ + id: createIdValidator(), name: createNameValidator(), manufacturer: z.string().min(1, "Manufacturer reference is required"), categories: z @@ -1109,9 +1183,9 @@ interface DataWithOptionalFields { supersedes?: string; identifiers?: Record; componentIdentifiers?: Record; - description?: string | string[]; - details?: string | string[]; - specs?: string | string[]; + description?: string; + details?: string; + specs?: string; } /** @@ -1443,9 +1517,7 @@ export function validateFile( // Matches text cut mid-word ("featur...") or ending with generic ellipsis, // but excludes legitimate trailing phrases like "and more..." or "etc..." if (data.description) { - const descText = Array.isArray(data.description) - ? data.description.join("\n\n") - : data.description; + const descText = data.description; if (/\.{3,}\s*$/.test(descText) && !/(?:more|etc|others|so on)\.{3,}\s*$/i.test(descText)) { const line = getLineForPath(document, lineCounter, ["description"]); const errorCode = ValidationErrorCode.E304_TRUNCATED_CONTENT; @@ -1553,7 +1625,7 @@ export function checkCompatibleWith( export interface WarningContext { name?: string; primaryCategory?: string; - io?: Array<{ name: string; type: string; connection: string; maxConnections?: number }>; + description?: string; url?: string; links?: Array<{ url: string }>; manufacturer?: string; @@ -1625,35 +1697,22 @@ export function collectWarnings( } } - // io.type is not checked here: it is a hard error (E117) via the Zod IOSchema, - // so an unknown value fails validation outright. io.connection remains advisory. - if (Array.isArray(data.io)) { - for (let i = 0; i < data.io.length; i++) { - const io = data.io[i]; - - if (io.connection && !KNOWN_IO_CONNECTIONS.has(io.connection)) { - const line = getLineForPath(document, lineCounter, ["io", i, "connection"]); - warnings.push({ - code: ValidationErrorCode.W121_UNKNOWN_IO_CONNECTION, - message: `Unknown IO connection '${io.connection}' on '${io.name}'. Consider adding to schema/io-connections.yaml if valid.`, - path: `io[${i}].connection`, - line: line ?? undefined, - }); - } - - // A single-jack connection carries one physical link; maxConnections>1 - // usually means several jacks were collapsed into one entry (aggregates - // like "All Slots" are excluded). - if (isIoCombineCandidate(io, { primaryCategory: data.primaryCategory })) { - const line = getLineForPath(document, lineCounter, ["io", i, "maxConnections"]); - warnings.push({ - code: ValidationErrorCode.W128_IO_COMBINE_CANDIDATE, - message: `IO '${io.name}' sets maxConnections ${io.maxConnections} on a single-jack connection ('${io.connection}'). If this is several physical jacks, give each its own io entry with maxConnections: 1.`, - path: `io[${i}].maxConnections`, - line: line ?? undefined, - }); - } - } + // W136: CLAUDE.md requires `description` on hardware and accessories, + // and the schema never did. Staged as a warning while 33 entries on + // `main` carry none; io.type and io.connection are hard errors (E117, + // E130) in the Zod IOSchema and are not repeated here. + const collection = path.basename(path.dirname(filePath)); + if ( + (collection === "hardware" || collection === "accessories") && + !(typeof data.description === "string" && data.description.trim()) + ) { + const line = getLineForPath(document, lineCounter, ["name"]); + warnings.push({ + code: ValidationErrorCode.W136_MISSING_DESCRIPTION, + message: `A ${collection} entry needs a description (CLAUDE.md "Data Entry Format"). Write one sentence from the maker's page.`, + path: "description", + line: line ?? undefined, + }); } // Check for duplicate URLs within the file @@ -1687,15 +1746,9 @@ export function collectWarnings( if (data.manufacturer && manufacturerUrlMap) { const mfrUrl = manufacturerUrlMap.get(data.manufacturer); if (mfrUrl) { - const normalize = (u: string) => - u - .replace(/^https?:\/\//, "") - .replace(/^www\./, "") - .replace(/\/+$/, "") - .toLowerCase(); - const normalizedMfr = normalize(mfrUrl); - - if (data.url && normalize(data.url) === normalizedMfr) { + const normalizedMfr = normalizeUrl(mfrUrl); + + if (data.url && normalizeUrl(data.url) === normalizedMfr) { const line = getLineForPath(document, lineCounter, ["url"]); warnings.push({ code: ValidationErrorCode.W125_MANUFACTURER_URL_IN_LINKS, @@ -1708,7 +1761,7 @@ export function collectWarnings( if (Array.isArray(data.links)) { for (let i = 0; i < data.links.length; i++) { const linkUrl = data.links[i].url; - if (linkUrl && normalize(linkUrl) === normalizedMfr) { + if (linkUrl && normalizeUrl(linkUrl) === normalizedMfr) { const line = getLineForPath(document, lineCounter, ["links", i, "url"]); warnings.push({ code: ValidationErrorCode.W125_MANUFACTURER_URL_IN_LINKS, @@ -1920,7 +1973,7 @@ function validate(): ValidationResult { const slug = path.basename(file, path.extname(file)); allManufacturers.add(slug); try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const mfrData = parseYaml(content) as { url?: string; name?: string }; if (mfrData.url) manufacturerUrlMap.set(slug, mfrData.url); if (mfrData.name) manufacturerNameMap.set(slug, mfrData.name); @@ -1941,17 +1994,33 @@ function validate(): ValidationResult { allHardwareSlugs.add(path.basename(file, path.extname(file))); } + // Every product's name, manufacturer and url, for the cross-file + // duplicate groups (W134, W135) reported after the per-file pass. + type DuplicateFields = { name?: string; manufacturer?: string; url?: string }; + const duplicateCandidates: DuplicateCandidate[] = []; + const noteCandidate = (collection: string, file: string, data: DuplicateFields) => { + duplicateCandidates.push({ + collection, + file: path.relative(process.cwd(), file), + name: typeof data.name === "string" ? data.name : undefined, + manufacturer: typeof data.manufacturer === "string" ? data.manufacturer : undefined, + url: typeof data.url === "string" ? data.url : undefined, + }); + }; + // Collect software IDs and supersedes relationships, and every entry's // identifiers for the cross-file E402 check below const identifierOwners: IdentifierOwner[] = []; for (const file of softwareFiles) { const slug = path.basename(file, path.extname(file)); try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const data = parseYaml(content) as { id?: string; supersedes?: string } & Omit< IdentifierOwner, "slug" - >; + > & + DuplicateFields; + noteCandidate("software", file, data); identifierOwners.push({ slug, identifiers: data.identifiers, @@ -1973,8 +2042,9 @@ function validate(): ValidationResult { for (const file of contentFiles) { const slug = path.basename(file, path.extname(file)); try { - const rawContent = fs.readFileSync(file, "utf-8"); - const data = parseYaml(rawContent) as { id?: string; supersedes?: string }; + const rawContent = readRegularFile(file); + const data = parseYaml(rawContent) as { id?: string; supersedes?: string } & DuplicateFields; + noteCandidate("content", file, data); if (data.id) { contentIds.add(data.id); contentIdToSlug.set(data.id, slug); @@ -1991,8 +2061,9 @@ function validate(): ValidationResult { for (const file of hardwareFiles) { const slug = path.basename(file, path.extname(file)); try { - const content = fs.readFileSync(file, "utf-8"); - const data = parseYaml(content) as { id?: string; supersedes?: string }; + const content = readRegularFile(file); + const data = parseYaml(content) as { id?: string; supersedes?: string } & DuplicateFields; + noteCandidate("hardware", file, data); if (data.id) { hardwareIds.add(data.id); hardwareIdToSlug.set(data.id, slug); @@ -2009,8 +2080,9 @@ function validate(): ValidationResult { for (const file of accessoryFiles) { const slug = path.basename(file, path.extname(file)); try { - const rawContent = fs.readFileSync(file, "utf-8"); - const data = parseYaml(rawContent) as { id?: string; supersedes?: string }; + const rawContent = readRegularFile(file); + const data = parseYaml(rawContent) as { id?: string; supersedes?: string } & DuplicateFields; + noteCandidate("accessories", file, data); if (data.id) { accessoryIds.add(data.id); accessoryIdToSlug.set(data.id, slug); @@ -2149,6 +2221,45 @@ function validate(): ValidationResult { } } + // W134 and W135: the same product filed twice, by name or by url. Each + // group is reported once, on its first file, naming the others, so the + // count in the summary is a count of pairs to look at rather than of + // files. Advisory while `main` carries them (66 name groups and 150 + // url groups when the check was written); the monthly dataset audit + // carries the same groups into its issue. + for (const group of findDuplicateNameGroups(duplicateCandidates)) { + const [first, ...rest] = group.entries; + warnings.push({ + file: first.file, + warnings: [ + { + code: ValidationErrorCode.W134_DUPLICATE_NAME_IN_COLLECTION, + message: + `'${group.value}' is also the name of ${rest.map((e) => e.file).join(", ")} under the ` + + "same manufacturer. Merge the entries, fold a cosmetic variant into `variants`, or " + + "disambiguate the names.", + path: "name", + }, + ], + }); + } + for (const group of findDuplicateUrlGroups(duplicateCandidates)) { + const [first, ...rest] = group.entries; + warnings.push({ + file: first.file, + warnings: [ + { + code: ValidationErrorCode.W135_DUPLICATE_URL_IN_COLLECTION, + message: + `url '${group.value}' is also the url of ${rest.map((e) => e.file).join(", ")}. ` + + "Two entries on one page are usually one product filed twice; otherwise give each " + + "its own product page.", + path: "url", + }, + ], + }); + } + // Check for cycles in supersedes chains (software) for (const [id, _supersedes] of softwareSupersedesMap) { const cycle = detectSupersedeCycle(id, softwareSupersedesMap, softwareIdToSlug); @@ -2267,13 +2378,18 @@ function validateIds(): IdValidationResult { "accessories", ]; + // One map across every collection (E207). Product images are served + // from R2 keyed by id alone, and `pnpm assign-ids` draws from one pool, + // so an id shared by a hardware entry and a manufacturer is a collision + // even though the two tables never meet. + const seenIds = new Map(); // id -> collection/slug + for (const collection of collections) { const files = getYamlFiles(path.join(DATA_DIR, collection)); - const seenIds = new Map(); // id -> slug for (const file of files) { try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const data = parseYaml(content) as { id?: unknown }; const slug = path.basename(file, path.extname(file)); @@ -2289,11 +2405,12 @@ function validateIds(): IdValidationResult { // Check for duplicates if (seenIds.has(data.id)) { errors.push( - `${collection}: duplicate id '${data.id}' in '${seenIds.get(data.id)}' and '${slug}'` + `${ValidationErrorCode.E207_DUPLICATE_ID}: id '${data.id}' is claimed by ` + + `'${seenIds.get(data.id)}' and '${collection}/${slug}' (${getDocsUrl(ValidationErrorCode.E207_DUPLICATE_ID)})` ); stats.duplicates++; } - seenIds.set(data.id, slug); + seenIds.set(data.id, `${collection}/${slug}`); stats.withIds++; } else { stats.withoutIds++; From 3ef0da369493ccb07dcf2019ad991f07ce840501 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:54:29 +0000 Subject: [PATCH 2/6] fix(scripts): guard every data read, catch retired ids getYamlFiles lists regular files off the dirent, and the six CI reads that bypassed readRegularFile go through it; assign-ids drops its private copy. check-id-immutability reads MRDT with -M, so a rename with a new id, a deletion and a file turned symlink no longer pass; an id may leave only under the retirement label, which validate.yml passes as PR_LABELS. validate-urls shells out through an argv array. Part of AUREO-1483 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- .github/workflows/validate.yml | 7 + .../__tests__/check-id-immutability.test.ts | 134 ++++++++++++ scripts/__tests__/utils.test.ts | 22 ++ scripts/assign-ids.ts | 18 +- scripts/check-id-immutability.ts | 194 +++++++++++++++--- scripts/generate-patch.ts | 4 +- scripts/lib/utils.ts | 13 +- scripts/validate-translations.ts | 12 +- 8 files changed, 348 insertions(+), 56 deletions(-) create mode 100644 scripts/__tests__/check-id-immutability.test.ts diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index a70b6a9e2..2d1bb412b 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -80,8 +80,15 @@ jobs: - name: Check formatting run: pnpm format:check + # An id may leave the catalog only on purpose: the check fails a + # deletion or a rename that drops an id unless the pull request + # carries the `retirement` label. The labels reach the script through + # env, never `${{ }}` inside `run:`, since a label name is text + # somebody typed. - name: Check ID immutability if: github.event_name == 'pull_request' + env: + PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} run: pnpm exec tsx scripts/check-id-immutability.ts origin/main # `pnpm assign-ids` owns `id` and `io[].key`, and the build refuses an diff --git a/scripts/__tests__/check-id-immutability.test.ts b/scripts/__tests__/check-id-immutability.test.ts new file mode 100644 index 000000000..179950e33 --- /dev/null +++ b/scripts/__tests__/check-id-immutability.test.ts @@ -0,0 +1,134 @@ +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { findIdImmutabilityErrors, RETIREMENT_LABEL } from "../check-id-immutability.js"; + +// The check is run against a throwaway git repository rather than a +// mocked diff, because what it escaped on (a rename git pairs with -M, a +// deletion `--diff-filter=M` never lists) is git's own behaviour. + +const ENTRY_A = `id: AAAAAAAAAAAAAAAAAAAAA +name: Alpha +manufacturer: acme +primaryCategory: pedal +description: A pedal with enough prose for git to pair a rename. +io: + - key: K1aaaaaa + name: Input + signalFlow: input + category: audio + type: instrument + connection: 1/4-inch + maxConnections: 1 + position: Right +`; + +const ENTRY_B = ENTRY_A.replace("AAAAAAAAAAAAAAAAAAAAA", "BBBBBBBBBBBBBBBBBBBBB") + .replace("Alpha", "Beta") + .replace("K1aaaaaa", "K2bbbbbb"); + +describe("findIdImmutabilityErrors", () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + const git = (cwd: string, ...args: string[]) => + execFileSync("git", args, { cwd, encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"] }); + + /** A repository with two hardware entries committed; returns its path and the base sha. */ + function repo(): { cwd: string; base: string } { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), "catalog-ids-")); + dirs.push(cwd); + git(cwd, "init", "-q"); + git(cwd, "config", "user.email", "t@example.com"); + git(cwd, "config", "user.name", "t"); + git(cwd, "config", "commit.gpgsign", "false"); + fs.mkdirSync(path.join(cwd, "data", "hardware"), { recursive: true }); + write(cwd, "a.yaml", ENTRY_A); + write(cwd, "b.yaml", ENTRY_B); + commit(cwd, "base"); + return { cwd, base: git(cwd, "rev-parse", "HEAD").trim() }; + } + const write = (cwd: string, name: string, text: string) => + fs.writeFileSync(path.join(cwd, "data", "hardware", name), text); + const commit = (cwd: string, message: string) => { + git(cwd, "add", "-A"); + git(cwd, "-c", "core.hooksPath=/dev/null", "commit", "-q", "-m", message); + }; + + it("passes an unchanged tree and a modified file that keeps its id", () => { + const { cwd, base } = repo(); + expect(findIdImmutabilityErrors({ baseRef: base, cwd })).toEqual({ errors: [], checked: 0 }); + write(cwd, "a.yaml", ENTRY_A.replace("name: Alpha", "name: Alpha II")); + commit(cwd, "rename product"); + expect(findIdImmutabilityErrors({ baseRef: base, cwd })).toEqual({ errors: [], checked: 1 }); + }); + + it("fails a modified file whose id changed", () => { + const { cwd, base } = repo(); + write(cwd, "a.yaml", ENTRY_A.replace("AAAAAAAAAAAAAAAAAAAAA", "CCCCCCCCCCCCCCCCCCCCC")); + commit(cwd, "new id"); + const { errors } = findIdImmutabilityErrors({ baseRef: base, cwd }); + expect(errors).toEqual([ + "data/hardware/a.yaml: ID was changed from 'AAAAAAAAAAAAAAAAAAAAA' to 'CCCCCCCCCCCCCCCCCCCCC'. IDs are immutable once assigned.", + ]); + }); + + it("passes a rename that keeps the id and fails one that changes it, once", () => { + const { cwd, base } = repo(); + git(cwd, "mv", "data/hardware/b.yaml", "data/hardware/b-mk2.yaml"); + commit(cwd, "rename file"); + expect(findIdImmutabilityErrors({ baseRef: base, cwd }).errors).toEqual([]); + + write(cwd, "b-mk2.yaml", ENTRY_B.replace("BBBBBBBBBBBBBBBBBBBBB", "DDDDDDDDDDDDDDDDDDDDD")); + commit(cwd, "rename with new id"); + const { errors } = findIdImmutabilityErrors({ baseRef: base, cwd }); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatch(/^data\/hardware\/b-mk2\.yaml: ID was changed from 'BBBB/); + }); + + it("fails a deletion unless the pull request carries the retirement label", () => { + const { cwd, base } = repo(); + fs.rmSync(path.join(cwd, "data", "hardware", "b.yaml")); + commit(cwd, "delete"); + const { errors } = findIdImmutabilityErrors({ baseRef: base, cwd }); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatch( + /^data\/hardware\/b\.yaml: id 'BBBBBBBBBBBBBBBBBBBBB' is on [0-9a-f]+ and on no file here\./ + ); + expect( + findIdImmutabilityErrors({ baseRef: base, cwd, labels: ["data", RETIREMENT_LABEL] }).errors + ).toEqual([]); + }); + + it("passes a deletion whose id moved to another file in the same change", () => { + // Two entries merged into one keep the surviving id; the other file goes. + const { cwd, base } = repo(); + fs.rmSync(path.join(cwd, "data", "hardware", "b.yaml")); + write(cwd, "beta.yaml", ENTRY_B); + commit(cwd, "merge"); + expect(findIdImmutabilityErrors({ baseRef: base, cwd }).errors).toEqual([]); + }); + + it("reports a port whose key moved", () => { + const { cwd, base } = repo(); + write(cwd, "a.yaml", ENTRY_A.replace("K1aaaaaa", "K9zzzzzz")); + commit(cwd, "churn key"); + const { errors } = findIdImmutabilityErrors({ baseRef: base, cwd }); + expect(errors).toEqual([ + "data/hardware/a.yaml: io port 'Input' changed key from 'K1aaaaaa' to 'K9zzzzzz'. IO keys are immutable once assigned.", + ]); + }); + + it("refuses a head file that is a symlink rather than reading through it", () => { + const { cwd, base } = repo(); + fs.rmSync(path.join(cwd, "data", "hardware", "a.yaml")); + fs.symlinkSync("/dev/null", path.join(cwd, "data", "hardware", "a.yaml")); + commit(cwd, "symlink"); + const { errors } = findIdImmutabilityErrors({ baseRef: base, cwd }); + expect(errors.some((e) => /a\.yaml: .*symbolic link/.test(e))).toBe(true); + }); +}); diff --git a/scripts/__tests__/utils.test.ts b/scripts/__tests__/utils.test.ts index 48a664ae2..0dd87c5b9 100644 --- a/scripts/__tests__/utils.test.ts +++ b/scripts/__tests__/utils.test.ts @@ -6,6 +6,7 @@ import { escapeSQL, findClosestMatch, formatValidOptions, + getYamlFiles, loadYamlFile, parseErrorPath, readRegularFile, @@ -48,6 +49,27 @@ describe("readRegularFile", () => { }); }); +describe("getYamlFiles", () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + it("lists regular .yaml and .yml files, sorted, and nothing that is not a regular file", () => { + // A listing is a list of names: a symlink named x.yaml is not a catalog + // file, and neither is a directory named one. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "catalog-list-")); + dirs.push(dir); + fs.writeFileSync(path.join(dir, "b.yaml"), "name: B\n"); + fs.writeFileSync(path.join(dir, "a.yml"), "name: A\n"); + fs.writeFileSync(path.join(dir, "notes.md"), ""); + fs.symlinkSync("/dev/zero", path.join(dir, "link.yaml")); + fs.mkdirSync(path.join(dir, "dir.yaml")); + expect(getYamlFiles(dir)).toEqual([path.join(dir, "a.yml"), path.join(dir, "b.yaml")]); + expect(getYamlFiles(path.join(dir, "missing"))).toEqual([]); + }); +}); + describe("findClosestMatch", () => { it("returns exact match when found", () => { const options = new Set(["synth", "effect", "sampler"]); diff --git a/scripts/assign-ids.ts b/scripts/assign-ids.ts index ac12ba479..210e28970 100644 --- a/scripts/assign-ids.ts +++ b/scripts/assign-ids.ts @@ -17,19 +17,7 @@ import { nanoid } from "nanoid"; import { parseDocument } from "yaml"; import { assignIoKeys } from "./lib/io-keys.js"; import type { Collection } from "./lib/types.js"; - -const REPO_ROOT = path.resolve(import.meta.dirname, ".."); -const DATA_DIR = path.join(REPO_ROOT, "data"); - -function getYamlFiles(dir: string): string[] { - if (!fs.existsSync(dir)) { - return []; - } - return fs - .readdirSync(dir) - .filter((f) => f.endsWith(".yaml") || f.endsWith(".yml")) - .map((f) => path.join(dir, f)); -} +import { DATA_DIR, getYamlFiles, readRegularFile } from "./lib/utils.js"; /** Load all existing IDs across all collections into a Set */ function loadExistingIds(): Set { @@ -45,7 +33,7 @@ function loadExistingIds(): Set { for (const collection of collections) { const files = getYamlFiles(path.join(DATA_DIR, collection)); for (const file of files) { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const doc = parseDocument(content); const data = doc.toJSON() as Record; if (typeof data.id === "string" && data.id.length > 0) { @@ -82,7 +70,7 @@ function assignIds(): void { const files = getYamlFiles(path.join(DATA_DIR, collection)); for (const file of files) { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const doc = parseDocument(content); const data = doc.toJSON() as Record; diff --git a/scripts/check-id-immutability.ts b/scripts/check-id-immutability.ts index 6e1bdb021..55bcb2d8c 100644 --- a/scripts/check-id-immutability.ts +++ b/scripts/check-id-immutability.ts @@ -1,72 +1,201 @@ /** * ID Immutability Check * - * Ensures that existing IDs are not modified in PRs. - * Compares modified YAML files against the base branch. + * An entry's `id` is the key Studio stores in a user's setups and the key + * product images are served under, so once assigned it never changes and + * never disappears. This compares the pull request's tree with the base + * branch and fails on three things: + * + * - a modified or renamed file whose `id` differs from the base; + * - an `id` present on the base that no file on the head carries, which + * is what a deletion or a rename-with-new-id looks like; + * - a hardware port whose stable `key` moved (see io-keys.ts). + * + * Until this read `--diff-filter=MRD` it listed modified files only and + * `continue`d when either side was missing, so a rename that changed the + * id and a plain deletion both passed. A retirement is the one legitimate + * way an id leaves the catalog, and it is declared rather than inferred: + * the pull request carries the `retirement` label, which validate.yml + * passes in `PR_LABELS`. + * * Run with: tsx scripts/check-id-immutability.ts */ import { execFileSync } from "node:child_process"; -import fs from "node:fs"; import path from "node:path"; +import { pathToFileURL } from "node:url"; import { parse as parseYaml } from "yaml"; import { findIoKeyViolations } from "./lib/io-keys.js"; import type { IO } from "./lib/types.js"; +import { readRegularFile } from "./lib/utils.js"; + +/** The label that lets an id leave the catalog on purpose. */ +export const RETIREMENT_LABEL = "retirement"; + +interface Entry { + id?: unknown; + io?: IO[]; +} + +interface DataChange { + status: "M" | "R" | "D" | "T"; + /** Path on the base branch. */ + basePath: string; + /** Path on the head, absent for a deletion. */ + headPath?: string; +} + +export interface CheckOptions { + baseRef: string; + /** The checkout to compare, the repository root. */ + cwd: string; + /** The pull request's labels, as validate.yml hands them over. */ + labels?: readonly string[]; +} -const baseRef = process.argv[2] ?? "origin/main"; +function git(cwd: string, args: string[]): string { + return execFileSync("git", args, { cwd, encoding: "utf-8" }); +} -function getModifiedDataFiles(): string[] { +/** + * The data files the head changed against the base: modified, renamed + * (with `-M` so git pairs the old and new path), deleted, and type-changed + * (a file replaced by a symlink is `T`, not `M`, and is the one case the + * guarded read below exists for). A `--name-only` listing cannot say which + * a path is, so this reads `--name-status`, where a rename row carries + * both paths. + */ +function getChangedDataFiles(cwd: string, baseRef: string): DataChange[] { + let output: string; try { - const output = execFileSync( - "git", - ["diff", "--name-only", "--diff-filter=M", baseRef, "--", "data/**/*.yaml"], - { encoding: "utf-8" } - ); - return output.split("\n").filter(Boolean); + output = git(cwd, [ + "diff", + "--name-status", + "-M", + "--diff-filter=MRDT", + baseRef, + "--", + "data/**/*.yaml", + ]); } catch { return []; } + const changes: DataChange[] = []; + for (const row of output.split("\n")) { + if (!row) continue; + const [code, first, second] = row.split("\t"); + const status = code?.[0]; + if ((status === "M" || status === "T") && first) + changes.push({ status, basePath: first, headPath: first }); + else if (status === "R" && first && second) + changes.push({ status, basePath: first, headPath: second }); + else if (status === "D" && first) changes.push({ status, basePath: first }); + } + return changes; } -function getBaseFileContent(filePath: string): string | null { +function readBaseEntry(cwd: string, baseRef: string, filePath: string): Entry | null { try { - return execFileSync("git", ["show", `${baseRef}:${filePath}`], { - encoding: "utf-8", - }); + return parseYaml(git(cwd, ["show", `${baseRef}:${filePath}`])) as Entry; } catch { return null; } } -function checkIdImmutability(): void { - const modifiedFiles = getModifiedDataFiles(); - const errors: string[] = []; - - for (const file of modifiedFiles) { - const baseContent = getBaseFileContent(file); - if (!baseContent) continue; +function readHeadEntry(cwd: string, filePath: string): Entry { + return parseYaml(readRegularFile(path.join(cwd, filePath))) as Entry; +} - const baseData = parseYaml(baseContent) as { id?: unknown; io?: IO[] }; +/** + * Every id the working tree carries, read off one `git grep` rather than + * 21k file reads. The working tree and not HEAD, because the file check + * above reads the working tree too and the two have to agree about what + * "here" is. + */ +function idsInWorkingTree(cwd: string): Set { + const ids = new Set(); + let output: string; + try { + output = git(cwd, ["grep", "-h", "-E", "^id: ", "--", "data/**/*.yaml"]); + } catch { + // git grep exits 1 when nothing matches, which an empty tree does. + return ids; + } + for (const line of output.split("\n")) { + const id = line.replace(/^id:\s*/, "").trim(); + if (id) ids.add(id.replace(/^["']|["']$/g, "")); + } + return ids; +} - const currentPath = path.resolve(file); - if (!fs.existsSync(currentPath)) continue; +/** The violations in `cwd` against `baseRef`, each a line for the log. */ +export function findIdImmutabilityErrors(options: CheckOptions): { + errors: string[]; + checked: number; +} { + const { baseRef, cwd } = options; + const changes = getChangedDataFiles(cwd, baseRef); + const errors: string[] = []; + /** Files whose id change is already reported, so a rename is not reported twice. */ + const reported = new Set(); - const currentContent = fs.readFileSync(currentPath, "utf-8"); - const currentData = parseYaml(currentContent) as { id?: unknown; io?: IO[] }; + for (const change of changes) { + if (!change.headPath) continue; + const baseData = readBaseEntry(cwd, baseRef, change.basePath); + if (!baseData) continue; + let currentData: Entry; + try { + currentData = readHeadEntry(cwd, change.headPath); + } catch (err) { + errors.push(`${change.headPath}: ${err instanceof Error ? err.message : String(err)}`); + continue; + } if (baseData.id !== undefined && currentData.id !== baseData.id) { + reported.add(change.basePath); errors.push( - `${file}: ID was changed from '${baseData.id}' to '${currentData.id}'. IDs are immutable once assigned.` + `${change.headPath}: ID was changed from '${baseData.id}' to '${currentData.id}'. IDs are immutable once assigned.` ); } if (Array.isArray(baseData.io) && Array.isArray(currentData.io)) { for (const violation of findIoKeyViolations(baseData.io, currentData.io)) { - errors.push(`${file}: ${violation}`); + errors.push(`${change.headPath}: ${violation}`); } } } + // An id on the base that the head no longer carries anywhere. Checked + // against the whole head tree rather than the changed file, because a + // retired file's id could legitimately move to another file in the same + // pull request (a merge of two entries keeps one id). + const departures = changes.filter( + (c) => (c.status === "R" || c.status === "D") && !reported.has(c.basePath) + ); + if (departures.length > 0 && !(options.labels ?? []).includes(RETIREMENT_LABEL)) { + const headIds = idsInWorkingTree(cwd); + for (const change of departures) { + const baseData = readBaseEntry(cwd, baseRef, change.basePath); + const id = baseData?.id; + if (typeof id !== "string" || headIds.has(id)) continue; + errors.push( + `${change.basePath}: id '${id}' is on ${baseRef} and on no file here. ` + + `An id never leaves the catalog by accident; label the pull request '${RETIREMENT_LABEL}' to retire it.` + ); + } + } + + return { errors, checked: changes.length }; +} + +function main(): void { + const baseRef = process.argv[2] ?? "origin/main"; + const labels = (process.env.PR_LABELS ?? "") + .split(",") + .map((label) => label.trim()) + .filter(Boolean); + const { errors, checked } = findIdImmutabilityErrors({ baseRef, cwd: process.cwd(), labels }); + if (errors.length > 0) { console.error("❌ ID immutability check failed!\n"); for (const error of errors) { @@ -75,7 +204,10 @@ function checkIdImmutability(): void { process.exit(1); } - console.log(`✅ ID immutability check passed (${modifiedFiles.length} files checked)`); + console.log(`✅ ID immutability check passed (${checked} files checked)`); } -checkIdImmutability(); +// Run only as a CLI, so the test suite can import the check. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main(); +} diff --git a/scripts/generate-patch.ts b/scripts/generate-patch.ts index 1d6182eee..f7e6fbb07 100644 --- a/scripts/generate-patch.ts +++ b/scripts/generate-patch.ts @@ -28,7 +28,7 @@ import Database from "better-sqlite3"; import { parse as parseYaml } from "yaml"; import type { Change } from "./lib/types.js"; -import { DATA_DIR, escapeSQL, OUTPUT_DIR } from "./lib/utils.js"; +import { DATA_DIR, escapeSQL, OUTPUT_DIR, readRegularFile } from "./lib/utils.js"; const PATCHES_DIR = path.join(OUTPUT_DIR, "patches"); const DEFAULT_DB = path.join(OUTPUT_DIR, "catalog.sqlite"); @@ -533,7 +533,7 @@ export function insertStatements( function readEntryId(category: Change["category"], slug: string): string | null { try { const file = path.join(DATA_DIR, category, `${slug}.yaml`); - return (parseYaml(fs.readFileSync(file, "utf-8")) as { id?: string }).id ?? null; + return (parseYaml(readRegularFile(file)) as { id?: string }).id ?? null; } catch { return null; } diff --git a/scripts/lib/utils.ts b/scripts/lib/utils.ts index 17fd952ce..91bfa6e74 100644 --- a/scripts/lib/utils.ts +++ b/scripts/lib/utils.ts @@ -180,13 +180,22 @@ export function parseErrorPath(pathStr: string): (string | number)[] { } /** - * Get all YAML files in a directory + * The YAML files in a collection directory. + * + * Regular files only, read off the dirent rather than the name: a + * directory listing is a list of names, and a name says nothing about + * what is at it. A committed `data/software/x.yaml -> /dev/zero` is a + * symlink, not a catalog file, so it is not listed, and the read that + * follows still goes through `readRegularFile`, because a listing is a + * statement about then and the read is now. */ export function getYamlFiles(dir: string): string[] { if (!fs.existsSync(dir)) return []; return ( fs - .readdirSync(dir) + .readdirSync(dir, { withFileTypes: true }) + .filter((entry) => entry.isFile()) + .map((entry) => entry.name) .filter((f) => f.endsWith(".yaml") || f.endsWith(".yml")) // Sorted so the SQLite build is deterministic: readdir order is // filesystem-dependent, and insertion order feeds autoincrement ids. diff --git a/scripts/validate-translations.ts b/scripts/validate-translations.ts index 09c020cdf..6751e6fa0 100644 --- a/scripts/validate-translations.ts +++ b/scripts/validate-translations.ts @@ -19,7 +19,7 @@ import type { Manufacturer, Software, } from "./lib/types.js"; -import { DATA_DIR, getYamlFiles, loadYamlFile, SCHEMA_DIR } from "./lib/utils.js"; +import { DATA_DIR, getYamlFiles, loadYamlFile, readRegularFile, SCHEMA_DIR } from "./lib/utils.js"; // ============================================================================= // LOAD APPROVED LOCALES @@ -280,7 +280,7 @@ function validateTranslations(): TranslationValidationResult { const manufacturerFiles = getYamlFiles(path.join(DATA_DIR, "manufacturers")); for (const file of manufacturerFiles) { try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const data = parseYaml(content) as Manufacturer; if (data.translations) { @@ -309,7 +309,7 @@ function validateTranslations(): TranslationValidationResult { const softwareFiles = getYamlFiles(path.join(DATA_DIR, "software")); for (const file of softwareFiles) { try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const data = parseYaml(content) as Software; if (data.translations) { @@ -338,7 +338,7 @@ function validateTranslations(): TranslationValidationResult { const contentDataFiles = getYamlFiles(path.join(DATA_DIR, "content")); for (const file of contentDataFiles) { try { - const rawContent = fs.readFileSync(file, "utf-8"); + const rawContent = readRegularFile(file); const data = parseYaml(rawContent) as Content; if (data.translations) { @@ -367,7 +367,7 @@ function validateTranslations(): TranslationValidationResult { const hardwareFiles = getYamlFiles(path.join(DATA_DIR, "hardware")); for (const file of hardwareFiles) { try { - const content = fs.readFileSync(file, "utf-8"); + const content = readRegularFile(file); const data = parseYaml(content) as Hardware; if (data.translations) { @@ -396,7 +396,7 @@ function validateTranslations(): TranslationValidationResult { const accessoryFiles = getYamlFiles(path.join(DATA_DIR, "accessories")); for (const file of accessoryFiles) { try { - const rawContent = fs.readFileSync(file, "utf-8"); + const rawContent = readRegularFile(file); const data = parseYaml(rawContent) as Accessory; if (data.translations) { From e480d1cbaff84c6c6d7d14f65a22e9657eadc5dc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:54:30 +0000 Subject: [PATCH 3/6] perf(build): unindexed fts id, commit-dated stamp The nanoid is a join key, never a search term, so every *_fts table stores it UNINDEXED; the column and the bm25 weights Studio passes are unchanged. updated_at is the HEAD commit date so one commit builds one database. Part of AUREO-1483 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- scripts/build-sqlite.ts | 23 ++++++++++++++++++++++- scripts/schema.sql | 18 +++++++++++++----- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/scripts/build-sqlite.ts b/scripts/build-sqlite.ts index 3647deb19..26ab4a3f9 100644 --- a/scripts/build-sqlite.ts +++ b/scripts/build-sqlite.ts @@ -6,6 +6,7 @@ * Run with: pnpm build */ +import { execFileSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; @@ -1566,12 +1567,32 @@ function populateDatabase(db: Database.Database, dataDir: string, version: strin VALUES (?, ?) `); updateMeta.run("version", version); - updateMeta.run("updated_at", new Date().toISOString()); + updateMeta.run("updated_at", buildTimestamp()); updateMeta.run("schema_version", CATALOG_SCHEMA_VERSION); return softwareCount + contentCount + hardwareCount + accessoryCount; } +/** + * The `updated_at` stamp: the HEAD commit's date, so two builds of one + * commit produce one database (the release attests the bytes, and a + * wall-clock stamp made every rebuild a different artifact). The clock + * is the fallback for a build outside a checkout. + */ +function buildTimestamp(): string { + try { + const committed = execFileSync("git", ["log", "-1", "--format=%cI"], { + encoding: "utf-8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + const date = new Date(committed); + if (!Number.isNaN(date.getTime())) return date.toISOString(); + } catch { + // Not a git checkout, or git is missing: the clock is all there is. + } + return new Date().toISOString(); +} + // ============================================================================= // MAIN // ============================================================================= diff --git a/scripts/schema.sql b/scripts/schema.sql index a4cf0c950..0f6eeb20c 100644 --- a/scripts/schema.sql +++ b/scripts/schema.sql @@ -752,9 +752,17 @@ CREATE INDEX idx_hardware_io_translations ON hardware_io_translations(hardware_i -- FULL-TEXT SEARCH -- ============================================================================= +-- `id` is stored but UNINDEXED in every *_fts table: it is the join key +-- back to the base row (Studio selects it and filters `id = x.id`), never +-- a thing a person searches for, and indexing a 21-character nanoid per +-- row only grew the index and let a search for a name match an id that +-- happened to contain it. The MATCH interface is unchanged: the column +-- is still there, in the same position, so the bm25() weights Studio +-- passes per column still line up. + -- Manufacturer FTS index CREATE VIRTUAL TABLE IF NOT EXISTS manufacturers_fts USING fts5( - id, + id UNINDEXED, name, company_name, description, @@ -764,7 +772,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS manufacturers_fts USING fts5( -- Content FTS index CREATE VIRTUAL TABLE IF NOT EXISTS content_fts USING fts5( - id, + id UNINDEXED, name, manufacturer_name, categories, @@ -777,7 +785,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS content_fts USING fts5( -- Note: Not using content='' (contentless mode) because we need to retrieve -- the id column to join back to the software table for full results CREATE VIRTUAL TABLE IF NOT EXISTS software_fts USING fts5( - id, + id UNINDEXED, name, manufacturer_name, categories, @@ -790,7 +798,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS software_fts USING fts5( -- Note: Not using content='' (contentless mode) because we need to retrieve -- the id column to join back to the hardware table for full results CREATE VIRTUAL TABLE IF NOT EXISTS hardware_fts USING fts5( - id, + id UNINDEXED, name, manufacturer_name, categories, @@ -801,7 +809,7 @@ CREATE VIRTUAL TABLE IF NOT EXISTS hardware_fts USING fts5( -- Accessories FTS index CREATE VIRTUAL TABLE IF NOT EXISTS accessories_fts USING fts5( - id, + id UNINDEXED, name, manufacturer_name, categories, From f845b21aa9e805e69dc2c4284a3dca13179dba05 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:54:30 +0000 Subject: [PATCH 4/6] ci(release): sign in a job that runs nothing Split by what each job holds: version runs the changesets action, build installs and builds with a read-only token and hands dist/ on as an artifact, sign-and-release installs nothing, checks the artifact, verifies minisign by digest and signs under umask 077 with the key in a mktemp file. Part of AUREO-1483 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- .github/workflows/release.yml | 176 ++++++++++++++++++++++++++-------- 1 file changed, 138 insertions(+), 38 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 590de0754..f1c91bded 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,15 +10,34 @@ concurrency: ${{ github.workflow }}-${{ github.ref }} permissions: contents: read +# Three jobs, split by what each holds, on the reasoning of assign-ids.yml +# and url-health.yml: a job that runs code from the tree (`pnpm install` +# executes dependency lifecycle scripts, and the build is scripts/*.ts) +# references no credential it does not need, and the job holding the +# signing key runs nothing from the tree at all. +# +# version changesets/action. It needs the checkout and +# `pnpm run version`, so it installs, and it needs a +# write token to open the version PR. That token is the +# one credential here, and it was already in the job +# that ran install before the split. +# build install, validate, build, index, checksums, SBOM. A +# read-only token and nothing else; hands dist/ on as +# an artifact. +# sign-and-release downloads that artifact, installs nothing, verifies +# minisign by digest, signs under umask 077 with the +# key in a mktemp file, attests and publishes. Until +# the split, `pnpm install` and the build ran in the +# job that wrote the signing key to /tmp under the +# default umask. jobs: - release: + version: runs-on: ubuntu-latest permissions: contents: write pull-requests: write - # For build-provenance attestation of release artifacts - id-token: write - attestations: write + outputs: + has-changesets: ${{ steps.changesets.outputs['has-changesets'] }} steps: - name: Checkout @@ -37,9 +56,6 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile - - name: Validate before release - run: pnpm validate - - name: Create Release Pull Request or Tag id: changesets uses: changesets/action@198f833dd7d863100ea6e28967bc9a9fdefadb0a # v2.1.0 @@ -49,37 +65,126 @@ jobs: commit-message: "chore: version packages" pr-title: "chore: version packages" + build: + needs: version + if: needs.version.outputs.has-changesets == 'false' + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + # fetch-depth 1 is enough: the build stamps `updated_at` with the + # HEAD commit's date, which a shallow clone carries. + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - name: Setup Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version: "24" + + - name: Setup pnpm + uses: pnpm/action-setup@0977fd99725f1db4007ccb2928dbb4e90d06cc86 # v6 + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Validate before release + run: pnpm validate + - name: Build database - if: steps.changesets.outputs['has-changesets'] == 'false' run: pnpm build - name: Build web index - if: steps.changesets.outputs['has-changesets'] == 'false' run: pnpm build:index - name: Generate checksum - if: steps.changesets.outputs['has-changesets'] == 'false' run: | sha256sum dist/catalog.sqlite > dist/catalog.sqlite.sha256 sha256sum dist/catalog-index.json > dist/catalog-index.json.sha256 - # Split from the signing step below so the signing secrets are not in - # the environment while unverified bytes are being downloaded and - # extracted. `curl | tar xz` ran an executable straight out of a - # release URL: anything that could replace that archive — upstream - # account compromise, a re-tagged release, an interfering proxy — - # chose the code that then ran alongside the catalog signing key. + # Here rather than beside the signature because the SBOM describes + # the dependencies the build ran with, and node_modules exists only + # in this job. + - name: Generate SBOM + uses: anchore/sbom-action@e22c389904149dbc22b58101806040fa8d37a610 # v0 + with: + artifact-name: catalog-sbom.spdx.json + output-file: dist/catalog-sbom.spdx.json + + - name: Upload build output + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: catalog-dist + path: | + dist/catalog.sqlite + dist/catalog.sqlite.sha256 + dist/catalog-index.json + dist/catalog-index.json.sha256 + dist/catalog-sbom.spdx.json + retention-days: 1 + if-no-files-found: error + + sign-and-release: + needs: build + runs-on: ubuntu-latest + permissions: + contents: write + # For build-provenance attestation of release artifacts + id-token: write + attestations: write + + steps: + # No `pnpm install` runs here, so no husky hooksPath gets configured + # either. The checkout is data: package.json for the version, the + # CHANGELOG for the release body, the tags for the exists check. + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Download build output + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 + with: + name: catalog-dist + path: dist + + # The artifact was produced by a job that ran code from the tree, so + # it is data to check, not a directory to trust: exactly the files + # the build is expected to hand over, each matching its checksum. + - name: Verify build output + run: | + set -euo pipefail + expected='catalog-index.json + catalog-index.json.sha256 + catalog-sbom.spdx.json + catalog.sqlite + catalog.sqlite.sha256' + if [ "$(ls -A dist | LC_ALL=C sort)" != "$expected" ]; then + echo "::error::dist/ holds something other than the five build files; refusing to sign it" + ls -la dist + exit 1 + fi + (cd dist && sha256sum -c catalog.sqlite.sha256 catalog-index.json.sha256) + + # Verified by a digest pinned here, in review, in git, before the + # signing key exists anywhere in this job. `curl | tar xz` ran an + # executable straight out of a release URL: anything that could + # replace that archive — upstream account compromise, a re-tagged + # release, an interfering proxy — chose the code that then ran + # alongside the catalog signing key. # - # The digest is the trust anchor: it lives here, in review, in git. - # It was taken from the archive upstream signs with the minisign - # public key published in its README + # The digest was taken from the archive upstream signs with the + # minisign public key published in its README # (RWQf6LRCGA9i53mlYecO4IzT51TGPpvWucNSCh1CBM0QTaLn73Y7GFO3), whose # detached signature is minisign-0.11-linux.tar.gz.minisig on the # same release. Re-derive it the same way when bumping the version; # verifying that signature here instead would mean checking the # download with the binary out of the download. - name: Install minisign - if: steps.changesets.outputs['has-changesets'] == 'false' env: MINISIGN_VERSION: "0.11" MINISIGN_SHA256: f0a0954413df8531befed169e447a66da6868d79052ed7e892e50a4291af7ae0 @@ -94,37 +199,34 @@ jobs: rm -f "$archive" minisign -v + # The key lives in a mktemp file created under umask 077, so it is + # readable by this user alone for the seconds it exists, and the + # trap removes it however the step ends. - name: Sign catalog with minisign - if: steps.changesets.outputs['has-changesets'] == 'false' env: CATALOG_SIGNING_KEY: ${{ secrets.CATALOG_SIGNING_KEY }} CATALOG_SIGNING_KEY_PASSWORD: ${{ secrets.CATALOG_SIGNING_KEY_PASSWORD }} run: | - echo "$CATALOG_SIGNING_KEY" > /tmp/minisign.key + set -euo pipefail + umask 077 + key=$(mktemp) + trap 'rm -f "$key"' EXIT + printf '%s\n' "$CATALOG_SIGNING_KEY" > "$key" VERSION=$(node -p "require('./package.json').version") echo "$CATALOG_SIGNING_KEY_PASSWORD" | minisign -S \ - -s /tmp/minisign.key \ + -s "$key" \ -m dist/catalog.sqlite \ -t "catalog v${VERSION}" echo "$CATALOG_SIGNING_KEY_PASSWORD" | minisign -S \ - -s /tmp/minisign.key \ + -s "$key" \ -m dist/catalog-index.json \ -t "catalog-index v${VERSION}" - rm /tmp/minisign.key - - - name: Generate SBOM - if: steps.changesets.outputs['has-changesets'] == 'false' - uses: anchore/sbom-action@e22c389904149dbc22b58101806040fa8d37a610 # v0 - with: - artifact-name: catalog-sbom.spdx.json - output-file: dist/catalog-sbom.spdx.json # Sigstore build provenance for the release artifacts (SLSA). The # bundle is also attached to the release as catalog.intoto.jsonl so # consumers (and OpenSSF Scorecard) can verify it offline. Best-effort: # a bookkeeping step must never be able to break a release. - name: Attest build provenance - if: steps.changesets.outputs['has-changesets'] == 'false' id: attest continue-on-error: true uses: actions/attest-build-provenance@4d101475d8b20a2381f78447822ac1eab6504dd8 # v4 @@ -134,18 +236,16 @@ jobs: dist/catalog-index.json - name: Stage provenance bundle for release - if: steps.changesets.outputs['has-changesets'] == 'false' && steps.attest.outcome == 'success' + if: steps.attest.outcome == 'success' env: BUNDLE_PATH: ${{ steps.attest.outputs.bundle-path }} run: cp "$BUNDLE_PATH" dist/catalog.intoto.jsonl - name: Get package version - if: steps.changesets.outputs['has-changesets'] == 'false' id: pkg run: echo "version=v$(node -p "require('./package.json').version")" >> $GITHUB_OUTPUT - name: Check if tag exists - if: steps.changesets.outputs['has-changesets'] == 'false' id: tag_check run: | if git rev-parse "${{ steps.pkg.outputs.version }}" >/dev/null 2>&1; then @@ -161,7 +261,7 @@ jobs: # section and the generated list follows it for maintainers. The # version reaches awk through `env:`, never interpolated into `run:`. - name: Extract release notes from CHANGELOG - if: steps.changesets.outputs['has-changesets'] == 'false' && steps.tag_check.outputs.exists == 'false' + if: steps.tag_check.outputs.exists == 'false' env: VERSION: ${{ steps.pkg.outputs.version }} run: | @@ -174,7 +274,7 @@ jobs: fi - name: Create GitHub Release - if: steps.changesets.outputs['has-changesets'] == 'false' && steps.tag_check.outputs.exists == 'false' + if: steps.tag_check.outputs.exists == 'false' uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 # v3 with: tag_name: ${{ steps.pkg.outputs.version }} From b3bbca85252a33c4ada84ad9899e54cbf0a4ace1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 07:39:43 +0000 Subject: [PATCH 5/6] docs: duplicates and ids in CLAUDE.md (AUREO-1483) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- CLAUDE.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 950d9128f..fddd30716 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -796,6 +796,24 @@ the one key reported as E199 instead, checked on the raw object before any schema runs, because product images live in R2 keyed by id. A field that is genuinely new data goes into the schema in the same PR. +## Duplicates and Ids + +**A duplicate is a group, reported once.** `pnpm validate` warns W134 when +two entries in a collection share a manufacturer and a normalised name, +and W135 when they share a `url` (read without scheme, `www.` or a +trailing slash), on the first file of each group naming the others; +`pnpm dataset:audit` lists the same groups from the one implementation +in `scripts/lib/duplicates.ts`, so the two never disagree about a pair. +Both are advisory because `main` carried 66 name groups and 149 url +groups when the check was written. Reviewers are no longer asked to spot +exact collisions by eye; only a product filed under a different name is +still theirs to find. + +An `id` is the 21-character nanoid `pnpm assign-ids` writes (E129), +unique across every collection (E207), and leaves the catalog only under +a pull request labelled `retirement`, which is what lets +`check-id-immutability` fail a deletion or a rename that drops one. + ## Identifiers `identifiers` is keyed by plugin format (`au`, `vst3`, `aax`, `clap`, From 454d67cca84b3d64a4955f065c676acacbf7ac82 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 07:53:56 +0000 Subject: [PATCH 6/6] ci: audits file findings, validate on labels (AUREO-1483) The monthly audit runs the five corpus audits with --findings and uploads findings.jsonl (AUREO-1480). Validate re-runs on a label change so the retirement gate can be satisfied. The context generator calls connections strict. One url-group count everywhere. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013X2d8vEWjRnkZydpXo94JF --- .github/workflows/dataset-audit.yml | 25 ++++++++++++++++++++++++- .github/workflows/validate.yml | 6 +++++- docs/VALIDATION_ERRORS.md | 2 +- schema/CONTEXT.md | 2 +- scripts/generate-context.ts | 2 +- scripts/lib/duplicates.ts | 2 +- scripts/validate.ts | 2 +- 7 files changed, 34 insertions(+), 7 deletions(-) diff --git a/.github/workflows/dataset-audit.yml b/.github/workflows/dataset-audit.yml index cee42e85e..c3203dcb5 100644 --- a/.github/workflows/dataset-audit.yml +++ b/.github/workflows/dataset-audit.yml @@ -34,10 +34,33 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # Every audit appends its settled findings to findings/findings.jsonl + # (see "Findings" in CLAUDE.md); the racks repo's file-findings.ts + # carries that file into the catalog-submissions inbox, one issue per + # finding, ever. The terminal reports are unchanged. Each audit is + # `|| true`: a red audit must not stop the next one or the issue + # update below, and the findings file is uploaded whatever happened. - name: Generate dataset audit id: audit run: | - pnpm dataset:audit --json > audit.json + mkdir -p findings + pnpm dataset:audit --json --findings findings > audit.json + + - name: Run the corpus audits with findings + run: | + pnpm power-input-audit --findings findings || true + pnpm speaker-level-audit --findings findings || true + pnpm capability-gaps --findings findings || true + pnpm prose-layout-audit --findings findings || true + wc -l findings/findings.jsonl || true + + - name: Upload findings + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: audit-findings + path: findings/findings.jsonl + if-no-files-found: ignore + retention-days: 45 - name: Create or update dataset audit issue uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 2d1bb412b..1e8017179 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -14,8 +14,12 @@ on: - "pnpm-lock.yaml" - "pnpm-workspace.yaml" - ".github/workflows/validate.yml" - # Keep these paths in sync with the push trigger above. + # Keep these paths in sync with the push trigger above. `labeled` and + # `unlabeled` are listed because the ID immutability step reads the + # `retirement` label: without them a maintainer adding the label gets + # no new run, and a re-run keeps the original payload. pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] branches: [main] paths: - "data/**" diff --git a/docs/VALIDATION_ERRORS.md b/docs/VALIDATION_ERRORS.md index d3c54948d..594d4af8f 100644 --- a/docs/VALIDATION_ERRORS.md +++ b/docs/VALIDATION_ERRORS.md @@ -1069,7 +1069,7 @@ Two or more entries in one collection carry the same `url`, compared without scheme, `www.` and trailing slashes. A product page belongs to one product, so this is usually one product filed twice, and occasionally a vendor whose entries all point at the storefront, which is a `url` to fix rather than entries to -merge. Reported once per group. Advisory while `main` carries 150 groups. +merge. Reported once per group. Advisory while `main` carries 149 groups. **Fix:** Merge duplicates, or point each entry at its own product page. A product with no page of its own leaves `url` out. diff --git a/schema/CONTEXT.md b/schema/CONTEXT.md index 6f9dc9904..5c9eb0037 100644 --- a/schema/CONTEXT.md +++ b/schema/CONTEXT.md @@ -836,7 +836,7 @@ rear-panel diagram — assign them with `pnpm enrich-io `, not during bulk - `ground` - `proprietary` -### IO Connections (advisory — unknown values produce warnings) +### IO Connections (strict — unknown values fail validation with E130) - `1/4-inch` - `tt` diff --git a/scripts/generate-context.ts b/scripts/generate-context.ts index 60424e379..d28cd5981 100644 --- a/scripts/generate-context.ts +++ b/scripts/generate-context.ts @@ -296,7 +296,7 @@ rear-panel diagram — assign them with \`pnpm enrich-io \`, not during bu ${context.ioTypes.map((v) => `- \`${v}\``).join("\n")} -### IO Connections (advisory — unknown values produce warnings) +### IO Connections (strict — unknown values fail validation with E130) ${context.ioConnections.map((v) => `- \`${v}\``).join("\n")} diff --git a/scripts/lib/duplicates.ts b/scripts/lib/duplicates.ts index 76813e8c1..489d29a01 100644 --- a/scripts/lib/duplicates.ts +++ b/scripts/lib/duplicates.ts @@ -16,7 +16,7 @@ * `pnpm dataset:audit` carries the same groups into the monthly issue, * from this one implementation so the two never disagree about a pair. * Reviewers used to be asked to spot both by eye, one import PR at a - * time, which is how 150 url groups came to sit on `main`. + * time, which is how 149 url groups came to sit on `main`. */ export interface DuplicateCandidate { diff --git a/scripts/validate.ts b/scripts/validate.ts index f1fa1d9e2..065712fff 100644 --- a/scripts/validate.ts +++ b/scripts/validate.ts @@ -2224,7 +2224,7 @@ function validate(): ValidationResult { // W134 and W135: the same product filed twice, by name or by url. Each // group is reported once, on its first file, naming the others, so the // count in the summary is a count of pairs to look at rather than of - // files. Advisory while `main` carries them (66 name groups and 150 + // files. Advisory while `main` carries them (66 name groups and 149 // url groups when the check was written); the monthly dataset audit // carries the same groups into its issue. for (const group of findDuplicateNameGroups(duplicateCandidates)) {