From 3b68534c73e0f6506185c64f8b515e10e53c103d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:04:00 +0000 Subject: [PATCH] feat(spec): fail CI when a schema variant is missing from its hand-written doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The liveness gate asks whether a property does anything; check-doc-authoring asks whether docs use the right authoring form. Neither asked the inverse-drift question: does a variant the schema DECLARES appear in the prose at all? content/docs/references/ is generated from these schemas and cannot drift, but the hand-written pages are typed by humans and do. The founding case shipped for months. apps.mdx said the navigation tree "supports eight item types" and enumerated eight; the schema had nine — `separator` was added to match the objectui renderer (#1878/#1891/#1894) and no hand-written page ever learned about it. Nothing failed, because nothing was looking in this direction. #4165 raised the cost: a mistyped discriminator now answers with the list of valid values, and the doc's enumeration is exactly what an author checks it against. Identity is `:`, deliberately not a source path — a union is reachable by several paths and the walk order picks one, so a path would churn. Keying on the variant set means adding or removing a variant CHANGES THE KEY, failing the ratchet and sending the author back through the ledger to the doc. variant-docs.json classifies all 20 unions found by reflection: 5 governed, 15 exempt as generated-reference-only (documented solely under references/, which is generated) or not-authorable (wire protocol, engine RPC, or a loader-derived discriminator such as `viewKind`, which no author ever types). Verified the gate actually bites, not just that it passes: removing `separator` from apps.mdx fails with exit 1 naming the variant; deleting a ledger entry fails as an undeclared union; mutating a key fails as an orphan plus an undeclared union — which is precisely the pair an author sees when they add a variant. Scope limit stated in the script header: a variant counts as documented if the page contains `: ''` or `` `` ``. The second form is loose, and for short generic variants it can match an unrelated sentence — a pass means the page says the word, not that it documents it correctly. This catches the omission that shipped for months; it does not replace reading the page. Refs #4001, #4165 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4 --- .changeset/variant-doc-drift-gate.md | 25 +++ .github/workflows/spec-liveness-check.yml | 11 + packages/spec/package.json | 1 + packages/spec/scripts/check-variant-docs.mts | 209 +++++++++++++++++++ packages/spec/variant-docs.json | 144 +++++++++++++ 5 files changed, 390 insertions(+) create mode 100644 .changeset/variant-doc-drift-gate.md create mode 100644 packages/spec/scripts/check-variant-docs.mts create mode 100644 packages/spec/variant-docs.json diff --git a/.changeset/variant-doc-drift-gate.md b/.changeset/variant-doc-drift-gate.md new file mode 100644 index 0000000000..e46063d058 --- /dev/null +++ b/.changeset/variant-doc-drift-gate.md @@ -0,0 +1,25 @@ +--- +"@objectstack/spec": patch +--- + +Add a gate that fails CI when a discriminated-union variant the schema declares is +never mentioned in the hand-written doc bound to it. + +The liveness gate asks whether a property does anything; `check-doc-authoring` asks +whether docs use the right authoring form. Neither asked the inverse-drift question: +does a variant the schema declares appear in the prose at all? `content/docs/references/` +is generated from the schemas and cannot drift, but the hand-written pages are typed by +humans and do. + +The founding case shipped for months: `content/docs/ui/apps.mdx` said the navigation +tree "supports eight item types" and enumerated eight. The schema had nine — `separator` +was added to match the objectui renderer and no hand-written page ever learned about it. +Making `NavigationItemSchema` a discriminated union raised the cost of that gap, because +a mistyped `type` now answers with the list of valid discriminators and the doc's +enumeration is what an author checks it against. + +`packages/spec/variant-docs.json` classifies all 20 unions: 5 governed (every variant +must be mentioned in a bound doc), 15 exempt as either generated-reference-only or +not-authorable. The ledger key is the discriminator plus its sorted variant set, so +adding or removing a variant changes the key and sends the author back through the +ledger — and therefore back to the doc. diff --git a/.github/workflows/spec-liveness-check.yml b/.github/workflows/spec-liveness-check.yml index 5ca1641b03..ace1d49945 100644 --- a/.github/workflows/spec-liveness-check.yml +++ b/.github/workflows/spec-liveness-check.yml @@ -18,6 +18,9 @@ on: paths: - 'packages/spec/**' - 'packages/qa/dogfood/**' + # The variant/doc gate reads hand-written pages, so editing one can break it + # without touching packages/spec at all. + - 'content/docs/**' permissions: contents: read @@ -50,3 +53,11 @@ jobs: # commonest authoring error, and it must not also be the widest grant. - name: Check empty-state semantics run: pnpm --filter @objectstack/spec check:empty-state + + # #4165 follow-up. Those two gates ask what a property does and what its empty + # value means. This asks the inverse-drift question neither covers: does a variant + # the schema declares appear in the hand-written doc at all? references/ is + # generated and cannot drift; the hand-written pages are typed by humans and do. + # apps.mdx said the nav tree had "eight item types" while the schema had nine. + - name: Check discriminated-union variants are documented + run: pnpm --filter @objectstack/spec check:variant-docs diff --git a/packages/spec/package.json b/packages/spec/package.json index 01c4507899..e3e6d9d66d 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -207,6 +207,7 @@ "test:coverage": "vitest run --coverage", "check:liveness": "tsx scripts/liveness/check-liveness.mts", "check:empty-state": "tsx scripts/liveness/check-empty-state.mts", + "check:variant-docs": "tsx scripts/check-variant-docs.mts", "gen:react-blocks": "tsx scripts/build-react-blocks-contract.ts", "check:react-blocks": "tsx scripts/build-react-blocks-contract.ts --check", "check:react-conformance": "tsx scripts/check-react-blocks-conformance.ts", diff --git a/packages/spec/scripts/check-variant-docs.mts b/packages/spec/scripts/check-variant-docs.mts new file mode 100644 index 0000000000..03427a54c2 --- /dev/null +++ b/packages/spec/scripts/check-variant-docs.mts @@ -0,0 +1,209 @@ +#!/usr/bin/env tsx +// Discriminated-union variant → hand-written doc gate. +// +// WHY THIS EXISTS (#4165). The liveness gate asks whether a schema property does +// anything, and check-doc-authoring asks whether docs use the right authoring form. +// Neither asks the inverse-drift question: does a variant the SCHEMA declares appear +// in the hand-written doc at all? content/docs/references/ is generated from these +// schemas and so cannot drift, but the hand-written pages are typed by humans and do. +// +// The founding case: content/docs/ui/apps.mdx said the navigation tree "supports eight +// item types" and enumerated eight. The schema had nine — `separator` had been added to +// match the objectui renderer (#1878/#1891/#1894) and no hand-written page ever learned +// about it. Nothing failed, because nothing was looking in this direction. +// +// #4165 raised the cost of that gap: NavigationItemSchema is now discriminated on `type`, +// so a mistyped discriminator answers with "Invalid discriminator value. Expected ..." — +// the doc's enumeration is precisely what an author checks that list against. And +// SeparatorNavItemSchema is .strict() over only type/id/order, so the base props the doc +// promised "all navigation items" share are hard errors on a separator. +// +// THE RATCHET. `key` is ':' — not a source +// path, because a union is reachable by several paths and the walk order picks one, which +// would churn. Keying on the variant set means adding or removing a variant CHANGES THE +// KEY, so the ledger no longer matches and CI sends the author back to variant-docs.json +// — and from there to the doc. An unknown key (new union) and an orphaned key (deleted or +// renamed union) both fail: no new undeclared surface, no stale claims. +// +// WHAT "MENTIONED" MEANS — and what this gate does NOT prove. A variant counts as +// documented if the bound doc contains either `: ''` (the code- +// sample form) or `` `` `` (the prose/table form). The second form is +// deliberately loose, and for short generic variants (`object`, `api`, `value`) it can +// match an unrelated sentence — so a pass here means "the page at least says the word", +// NOT "the page documents it correctly". This gate catches the omission that shipped for +// months; it is not a substitute for reading the page. Tightening the loose form would +// cost more false failures than the precision is worth. +// +// Usage: +// tsx check-variant-docs.mts # fail on undeclared/orphaned/undocumented +// tsx check-variant-docs.mts --list # print every discovered union and its key + +import fs from 'node:fs'; +import path from 'node:path'; +import url from 'node:url'; + +// Safe to set here despite ESM hoisting: the only static imports above are node +// built-ins, and every schema module is pulled in via dynamic `await import()` +// below — i.e. after this line runs. Keeps the script correct when invoked +// directly (`tsx scripts/check-variant-docs.mts`), not just via the pnpm script. +process.env.OS_EAGER_SCHEMAS ??= '1'; + +const HERE = path.dirname(url.fileURLToPath(import.meta.url)); +const SPEC = path.resolve(HERE, '..'); +const REPO = path.resolve(SPEC, '../..'); +const SRC = path.join(SPEC, 'src'); +const LEDGER = path.join(SPEC, 'variant-docs.json'); +const LIST = process.argv.includes('--list'); + +type Entry = { + key: string; label: string; + docs?: string[]; exempt?: string; reason?: string; note?: string; +}; + +const defOf = (s: any) => s?._zod?.def ?? s?._def; + +function zodFiles(dir: string, acc: string[] = []): string[] { + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, e.name); + if (e.isDirectory()) zodFiles(p, acc); + else if (e.name.endsWith('.zod.ts')) acc.push(p); + } + return acc; +} + +const seen = new Set(); +const found = new Map(); + +function walk(s: any, p: string, depth: number): void { + if (!s || typeof s !== 'object' || depth > 14 || seen.has(s)) return; + seen.add(s); + const d = defOf(s); + if (!d) return; + + if (d.type === 'union' && d.discriminator) { + const variants: string[] = []; + for (const o of d.options ?? []) { + const od = defOf(o); + const shape = typeof od?.shape === 'function' ? od.shape() : od?.shape; + const dd = defOf(shape?.[d.discriminator]); + const vals = dd?.values ?? (dd?.value !== undefined ? [dd.value] : []); + for (const v of vals) variants.push(String(v)); + } + if (variants.length) { + const key = `${d.discriminator}:${[...variants].sort().join('|')}`; + if (!found.has(key)) found.set(key, { disc: d.discriminator, variants, path: p }); + } + } + + // Structural recursion. Wrapped in try/catch because a lazy getter can reference a + // module that is still initialising mid-import; an unresolvable branch must not abort + // the whole sweep (it will be reached again via another path). + try { + if (typeof d.getter === 'function') walk(d.getter(), p, depth + 1); + const shape = typeof d.shape === 'function' ? d.shape() : d.shape; + if (shape) for (const [k, v] of Object.entries(shape)) walk(v, `${p}.${k}`, depth + 1); + if (d.element) walk(d.element, `${p}[]`, depth + 1); + if (d.innerType) walk(d.innerType, p, depth + 1); + if (d.valueType) walk(d.valueType, `${p}{}`, depth + 1); + if (d.left) walk(d.left, p, depth + 1); + if (d.right) walk(d.right, p, depth + 1); + if (d.in) walk(d.in, p, depth + 1); + if (d.out) walk(d.out, p, depth + 1); + for (const o of d.options ?? []) walk(o, p, depth + 1); + for (const t of d.items ?? []) walk(t, p, depth + 1); + } catch { /* unresolvable branch — reached via another path if it matters */ } +} + +const files = zodFiles(SRC).sort(); +for (const f of files) { + let mod: Record; + try { mod = await import(url.pathToFileURL(f).href); } catch { continue; } + const rel = path.relative(SRC, f).replace(/\.zod\.ts$/, ''); + for (const [name, val] of Object.entries(mod)) { + if (val && typeof val === 'object' && defOf(val)) walk(val, `${rel}#${name}`, 0); + } +} + +if (LIST) { + const rows = [...found.entries()].sort((a, b) => a[1].path.localeCompare(b[1].path)); + console.log(`${rows.length} discriminated union(s):\n`); + for (const [key, v] of rows) console.log(`${v.path}\n key: ${key}\n ${v.variants.join(', ')}\n`); + process.exit(0); +} + +const ledger: { entries: Entry[] } = JSON.parse(fs.readFileSync(LEDGER, 'utf-8')); +const byKey = new Map(ledger.entries.map((e) => [e.key, e])); +const errors: string[] = []; + +// Duplicate keys in the ledger would let one entry silently shadow another. +const dupes = ledger.entries.map((e) => e.key).filter((k, i, a) => a.indexOf(k) !== i); +for (const k of new Set(dupes)) errors.push(`duplicate ledger key: ${k}`); + +// Ratchet A — every discovered union must be declared. +for (const [key, v] of found) { + if (byKey.has(key)) continue; + errors.push( + `undeclared discriminated union at ${v.path}\n` + + ` key: ${key}\n` + + ` variants (${v.variants.length}): ${v.variants.join(', ')}\n` + + ` → add it to packages/spec/variant-docs.json: bind \`docs\` (and make sure each variant\n` + + ` is mentioned there), or declare \`exempt\` with a reason.`, + ); +} + +// Ratchet B — every ledger entry must still correspond to a real union. A changed +// variant set surfaces here as an orphan plus an undeclared entry above. +for (const e of ledger.entries) { + if (!found.has(e.key)) { + errors.push( + `orphaned ledger entry "${e.label}"\n` + + ` key: ${e.key}\n` + + ` → no union with this discriminator/variant set exists any more. If variants changed,\n` + + ` update the key AND re-check the bound doc; if the union is gone, delete the entry.`, + ); + } +} + +// Coverage — every variant of a governed union must be mentioned in a bound doc. +const esc = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +let governed = 0, exempt = 0; +for (const e of ledger.entries) { + const u = found.get(e.key); + if (!u) continue; + if (e.exempt) { exempt++; continue; } + if (!e.docs?.length) { + errors.push(`ledger entry "${e.label}" declares neither \`docs\` nor \`exempt\`.`); + continue; + } + governed++; + + const missingDocs = e.docs.filter((d) => !fs.existsSync(path.join(REPO, d))); + if (missingDocs.length) { + errors.push(`ledger entry "${e.label}" binds doc(s) that do not exist: ${missingDocs.join(', ')}`); + continue; + } + const blob = e.docs.map((d) => fs.readFileSync(path.join(REPO, d), 'utf-8')).join('\n'); + const undocumented = u.variants.filter((v) => { + const anchored = new RegExp(`${esc(u.disc)}\\s*:\\s*['"\`]${esc(v)}['"\`]`); + const token = new RegExp('`' + esc(v) + '`'); + return !anchored.test(blob) && !token.test(blob); + }); + if (undocumented.length) { + errors.push( + `"${e.label}" — ${undocumented.length} variant(s) the schema declares but the doc never mentions:\n` + + ` ${undocumented.join(', ')}\n` + + ` doc(s): ${e.docs.join(', ')}\n` + + ` → document them, or if they are genuinely not author-facing, re-classify the entry.`, + ); + } +} + +if (errors.length) { + console.error(`\n✗ variant/doc gate: ${errors.length} problem(s)\n`); + for (const e of errors) console.error(` ${e}\n`); + process.exit(1); +} +console.log( + `✓ variant/doc gate: ${found.size} discriminated union(s) — ` + + `${governed} governed (every variant mentioned in a bound doc), ${exempt} exempt.`, +); diff --git a/packages/spec/variant-docs.json b/packages/spec/variant-docs.json new file mode 100644 index 0000000000..1c944f68bf --- /dev/null +++ b/packages/spec/variant-docs.json @@ -0,0 +1,144 @@ +{ + "$comment": [ + "Discriminated-union variant → hand-written doc ledger. See scripts/check-variant-docs.mts.", + "", + "`key` is the union's identity: ':'. It is", + "deliberately NOT a source path — a union is reachable by several paths and the walk order", + "decides which one you see, so a path would churn. Keying on the variant set instead means", + "adding or removing a variant CHANGES THE KEY, which fails the ratchet and forces the author", + "back through this file — and therefore back to the doc. That is the whole point.", + "", + "Each entry either binds `docs` (governed: every variant must appear in at least one of them)", + "or declares `exempt` with a reason. Two exemption classes are in use:", + " generated-reference-only — the type is documented solely under content/docs/references/,", + " which build-docs.ts generates FROM these schemas. Generated docs cannot drift.", + " not-authorable — wire protocol, engine RPC, or a runtime-derived discriminator. Nobody", + " hand-writes the variant, so no hand-written doc owes it a mention." + ], + "entries": [ + { + "key": "type:action|component|dashboard|group|object|page|report|separator|url", + "label": "app navigation item", + "docs": ["content/docs/ui/apps.mdx"], + "note": "The gate's founding case: apps.mdx claimed eight types and omitted `separator` (#4165)." + }, + { + "key": "provider:api|object|schema|value", + "label": "view data provider", + "docs": [ + "content/docs/ui/views.mdx", + "content/docs/ui/forms.mdx", + "content/docs/data-modeling/fields.mdx" + ] + }, + { + "key": "type:conditional|cross_field|format|json_schema|script|state_machine", + "label": "validation rule", + "docs": [ + "content/docs/data-modeling/validation-rules.mdx", + "content/docs/data-modeling/validation.mdx" + ] + }, + { + "key": "language:expression|js", + "label": "hook body language", + "docs": ["content/docs/automation/hook-bodies.mdx"] + }, + { + "key": "kind:file|http|object", + "label": "knowledge source kind", + "docs": ["content/docs/protocol/knowledge.mdx"] + }, + + { + "key": "type:inline|npm|remote", + "label": "widget implementation", + "exempt": "generated-reference-only", + "reason": "Documented at content/docs/references/ui/widget.mdx, generated from WidgetManifestSchema." + }, + { + "key": "type:cron|interval|once", + "label": "job schedule", + "exempt": "generated-reference-only", + "reason": "Documented at content/docs/references/system/job.mdx." + }, + { + "key": "type:api-key|basic|bearer|none|oauth2", + "label": "connector authentication", + "exempt": "generated-reference-only", + "reason": "Documented at content/docs/references/integration/connector-auth.mdx." + }, + { + "key": "type:api-key|basic|bearer|none", + "label": "connector auth (environment-artifact projection)", + "exempt": "generated-reference-only", + "reason": "Narrower projection of the connector-auth union carried in the deploy artifact; same generated reference." + }, + { + "key": "strategy:isolated_db|isolated_schema|shared_schema", + "label": "tenant isolation strategy", + "exempt": "generated-reference-only", + "reason": "Documented at content/docs/references/system/tenant.mdx. Operator-set, not tenant-authored." + }, + { + "key": "type:cast|constant|javascript|lookup|map", + "label": "sync mapping transform", + "exempt": "generated-reference-only", + "reason": "Generated reference only; no hand-written page covers the sync transform set yet." + }, + { + "key": "kind:action|http|navigate", + "label": "settings-manifest handler", + "exempt": "generated-reference-only", + "reason": "Generated reference only; consumed by Setup/Studio, not tenant-authored metadata." + }, + { + "key": "viewKind:form|list", + "label": "view item kind", + "exempt": "not-authorable", + "reason": "Loader-derived. Authors write the `list` / `listViews` / `formViews` containers; the loader expands them and stamps `viewKind`. No author ever types it." + }, + { + "key": "type:ack|cursor|edit|error|event|ping|pong|presence|subscribe|unsubscribe", + "label": "websocket message", + "exempt": "not-authorable", + "reason": "Wire protocol between client and gateway." + }, + { + "key": "status:approved|expired|pending", + "label": "device-token response status", + "exempt": "not-authorable", + "reason": "API response envelope, not authored metadata." + }, + { + "key": "method:aggregate|batch|count|delete|execute|find|findOne|insert|update|vectorFind", + "label": "data-engine request", + "exempt": "not-authorable", + "reason": "Engine RPC contract." + }, + { + "key": "method:aggregate|count|delete|execute|find|findOne|insert|update|vectorFind", + "label": "data-engine batch request item", + "exempt": "not-authorable", + "reason": "Engine RPC contract; the batch member set excludes `batch` itself." + }, + { + "key": "type:delete|insert|retain", + "label": "collaboration op component", + "exempt": "not-authorable", + "reason": "CRDT operation encoding." + }, + { + "key": "type:g-counter|lww-register|or-set|pn-counter|text", + "label": "CRDT merge state", + "exempt": "not-authorable", + "reason": "CRDT internal state encoding." + }, + { + "key": "type:add_field|create_object|delete_object|execute_sql|modify_field|remove_field|rename_object", + "label": "migration changeset operation", + "exempt": "not-authorable", + "reason": "Emitted by the diff engine, not hand-written." + } + ] +}