From 3cb2c9eb8393ef51e7707f5a387a58c3d9d5cd29 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 04:09:07 +0000 Subject: [PATCH] fix(docs-audit): derive the docs-drift advisory from what pages document, not package edges The advisory resolved pages by PACKAGE DEPENDENCY -- "which hand-written docs reference @objectstack/x". That is a dependency-graph proxy answering a semantic question, and #9192 measured it wrong in BOTH directions on PR #9191 (the three metadata read verbs): 3 pages listed of which 1 was relevant, while the 2 pages that actually document the changed surface were absent, because they document it through the SDK -- which does not depend on the implementing package at all. A page is now listed when it NAMES something the change touched. Three anchor kinds: the documentable declaration enclosing each changed line (top-level, or a member of a top-level container -- a local inside a function body is not surface, which is what drops the measured `singular` false positive); the wire routes a changed symbol's registrar handler serves; and the client method the route ledgers bind to those routes, which is the hop that carries the derivation across the surface boundary the package graph cannot cross. Two guards keep it precision-first, both running BEFORE the route bridge so a weak name cannot mint route anchors from every handler that mentions it: a shape guard (a single lowercase word like `label` or `object` matched 82 and 113 of 178 pages) and a corpus-share guard (`ObjectQL` is code-shaped and genuinely changed, and names 59 of 178 pages). Everything either guard removes is published, as is every changed file that yielded no anchor -- silence from this tool must never be readable as absence, which is the whole subject of the card. Measured before/after across ten real PRs; the PR comment now also states what the run could not see, at the point of use. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --- .github/workflows/docs-drift-check.yml | 63 +- scripts/docs-audit/README.md | 133 +++- scripts/docs-audit/affected-docs.mjs | 809 ++++++++++++++++++++- scripts/docs-audit/check-affected-docs.mjs | 4 +- 4 files changed, 965 insertions(+), 44 deletions(-) diff --git a/.github/workflows/docs-drift-check.yml b/.github/workflows/docs-drift-check.yml index a658920b38..97ef030f57 100644 --- a/.github/workflows/docs-drift-check.yml +++ b/.github/workflows/docs-drift-check.yml @@ -1,10 +1,18 @@ name: Docs Drift Check -# When a PR changes packages/** code, flag the hand-written docs that reference the -# affected packages so they can be re-verified for implementation accuracy before the -# drift lands on main. Advisory only — posts a PR comment, never fails the build. -# The actual LLM audit is run on-demand / on a schedule via the `docs-accuracy-audit` -# workflow, scoped to exactly the docs this check lists. +# When a PR changes packages/** code, flag the hand-written docs that NAME something the +# change touched — a symbol, a wire route, or the SDK method a route ledger binds to it — +# so they can be re-verified for implementation accuracy before the drift lands on main. +# Advisory only: posts a PR comment, never fails the build. The actual LLM audit is run +# on-demand / on a schedule via the `docs-accuracy-audit` workflow, scoped to exactly the +# docs this check lists. +# +# It used to list pages by PACKAGE DEPENDENCY ("which docs mention @objectstack/x"), and +# #9192 measured that wrong in both directions on a real PR: 2 of 3 listed pages were +# irrelevant, and the 2 pages that actually documented the changed surface were missing +# because they document it through the SDK, which does not depend on the implementing +# package. The comment now also states what the run could NOT see — the derivation being +# read past its precision, with its silence taken for absence, is what #9192 records. on: pull_request: @@ -75,6 +83,30 @@ jobs: const docs = data.docs || []; const pkgs = (data.changedPackages || []).map(p => p.name || p.dir); const marker = ''; + // #9192 — the derivation now matches pages that NAME something the change + // touched (symbol / route / SDK-method anchors) instead of pages that merely + // mention a changed package. Everything the derivation could NOT see is + // stated in the comment, at the point of use: the failure this fixed was not + // the tool lying, it was a reader taking its silence for absence. + const anchorList = data.anchors || []; + const anchorless = data.anchorlessChanges || []; + const overbroad = data.overbroadAnchors || []; + const crossCutting = data.crossCuttingSymbols || []; + const weak = data.weakAnchorsDropped || []; + const coarse = data.packageMentionDocs || []; + const rederive = `node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}`; + const limits = []; + if (anchorless.length) limits.push(`**${anchorless.length}** changed file(s) yielded no anchor (\`${anchorless.slice(0, 3).join('`, `')}\`${anchorless.length > 3 ? ', …' : ''}) — pages documenting those are invisible to this run`); + if (crossCutting.length) limits.push(`**${crossCutting.length}** cross-cutting symbol(s) contributed no route anchor: \`${crossCutting.join('`, `')}\``); + if (overbroad.length) limits.push(`**${overbroad.length}** anchor(s) matched too much of the corpus to be a work list: \`${overbroad.join('`, `')}\``); + if (weak.length) limits.push(`**${weak.length}** name(s) were too generic to anchor anything (single lowercase words)`); + // Rendered whenever there is anything to say, INCLUDING when the only thing to + // say is "the wide net exists and holds N pages". A short list is the right + // answer here, but a reader must be able to tell a short list from a blind one + // without leaving the PR. + const limitsBlock = (limits.length || coarse.length) + ? ['', '
What this run could not see', '', ...limits.map(l => `- ${l}`), ...(limits.length ? [''] : []), `Coarse fallback — **${coarse.length}** page(s) merely *mention* a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): \`${rederive}\` → \`packageMentionDocs\`.`, '
'] + : []; // The release-owned rows are PARTITIONED OUT of the editable list, never // dropped (#6893, following the #4920 ruling). They keep getting audited — // `docs` above is still the full set the audit workflow is scoped to — but @@ -96,14 +128,19 @@ jobs: const capped = editable.length > EDITABLE_ROW_CAP; let body; if (docs.length === 0) { - body = `${marker}\n### 📓 Docs Drift Check\nNo hand-written docs reference the ${pkgs.length} changed package(s). ✅`; + // "Nothing found" and "I derived nothing to look for" are DIFFERENT results + // and must never render alike — that conflation is #9192's own subject. + const headline = anchorList.length === 0 + ? `Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from **${pkgs.length}** changed package(s)), so **this run has no opinion** about the docs.` + : `**${anchorList.length}** anchor(s) derived from **${pkgs.length}** changed package(s); no hand-written page names any of them. ✅`; + body = [marker, '### 📓 Docs Drift Check', headline, ...limitsBlock].join('\n'); } else { const detail = (data.detail || []).reduce((m, d) => (m[d.doc] = d.via, m), {}); const row = d => `- \`${d}\`${detail[d] ? ` _(via ${detail[d].join(', ')})_` : ''}`; body = [ marker, '### 📓 Docs Drift Check', - `This PR changes **${pkgs.length}** package(s): ${pkgs.map(p => `\`${p}\``).join(', ')}.`, + `This PR changes **${pkgs.length}** package(s): ${pkgs.map(p => `\`${p}\``).join(', ')}, touching **${anchorList.length}** documentable anchor(s).`, ]; if (capped) { // NO row list above the cap — folding rows behind a details tag would @@ -113,7 +150,7 @@ jobs: // fidelity is one command away, never lost. body.push( '', - `**${editable.length}** hand-written doc(s) reference the affected code — list omitted above ${EDITABLE_ROW_CAP} rows. Re-derive: \`node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}\`.`, + `**${editable.length}** hand-written doc(s) name something this change touched — list omitted above ${EDITABLE_ROW_CAP} rows. Re-derive: \`node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}\`.`, ); if (readOnly.length) { body.push( @@ -125,7 +162,7 @@ jobs: if (editable.length) { body.push( '', - `**${editable.length}** hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:`, + `**${editable.length}** hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:`, '', editable.map(row).join('\n'), ); @@ -133,7 +170,7 @@ jobs: if (readOnly.length) { body.push( '', - `⛔ **${readOnly.length}** release-owned page(s) ${editable.length ? 'also ' : ''}reference the affected code. These are **read-only**:`, + `⛔ **${readOnly.length}** release-owned page(s) ${editable.length ? 'also ' : ''}name something this change touched. These are **read-only**:`, '', readOnly.map(row).join('\n'), '', @@ -144,9 +181,13 @@ jobs: ); } } + body.push(...limitsBlock); body.push( '', - '> Advisory only. To re-verify, run the `docs-accuracy-audit` workflow scoped to these files:', + '> Advisory only, and a **precision-first** one (#9192): a page is listed because it names a', + '> symbol, wire route or SDK method this diff touched — not because it mentions a changed', + '> package. Each row says which anchor put it there, so a wrong row is reportable rather than', + '> merely annoying. To re-verify, run the `docs-accuracy-audit` workflow scoped to these files:', '> `node scripts/docs-audit/affected-docs.mjs origin/' + baseRef + '` → pass the list as `args.docs`.', ); body = body.join('\n'); diff --git a/scripts/docs-audit/README.md b/scripts/docs-audit/README.md index b9cdc02edb..0b1ea22c74 100644 --- a/scripts/docs-audit/README.md +++ b/scripts/docs-audit/README.md @@ -9,8 +9,8 @@ The system has four parts, layered cheapest-and-earliest first: ## 1. `affected-docs.mjs` — change → docs mapping (the linchpin) -Maps a set of `packages/**` changes to the hand-written docs that reference the -affected packages, so an audit can be scoped to what actually changed. +Maps a set of `packages/**` changes to the hand-written docs that **name something the +change touched**, so an audit can be scoped to what actually changed. ```bash # docs affected by changes on this branch vs origin/main @@ -22,15 +22,114 @@ node scripts/docs-audit/affected-docs.mjs --json origin/main # every hand-written doc (full audit scope) node scripts/docs-audit/affected-docs.mjs --all -# pin the change classifiers + package-root derivation (needs no repo state; CI runs this before the mapping) +# pin the classifiers, package-root and anchor derivations (needs no repo state; CI runs this before the mapping) node scripts/docs-audit/affected-docs.mjs --self-test ``` -Heuristic: a doc is *affected* by a changed package `P` if it mentions `P`'s npm -name (`@objectstack/`) or repo path (`P`'s directory, e.g. -`packages/services/service-automation`). Over-inclusion is preferred over misses; the -periodic **full** audit (part 4) is the backstop for docs that describe a package -without naming it. +**Derivation (#9192): a doc is *affected* when it NAMES something the change touched.** +Not when it mentions the changed package — that predicate is a dependency-graph proxy +answering a semantic question, and it was measured wrong in *both* directions on PR #9191 +(three read verbs in `@objectstack/metadata-protocol`): 3 pages listed of which 1 was +relevant, while the 2 pages that actually document the changed surface — +`api/client-sdk.mdx` and `kernel/contracts/metadata-service.mdx` — were absent, because +they document it through the **SDK** surface, which does not depend on the implementing +package at all. + +Over-inclusion is not free, and that is the correction. A wrong-both-ways advisory trains +its reader to skip it, and then it fails on the PR where it is right — the same bill +exclusion 1 below already paid. The derivation is therefore **precision-first**: a shorter +right list beats a longer noisy one. + +Three anchor kinds, each exact: + +| anchor | what it is | how it is derived | +|:--|:--|:--| +| `symbol` | a documentable declaration the diff touched | the top-level declaration, or a member of a top-level **container** (class / interface / type / enum / schema object), enclosing each changed line — on **both** sides of the diff, so a removed export still anchors the pages naming it | +| `route` | a wire path the change touched | a path literal on a changed line, plus every route whose **registrar handler** references a changed symbol | +| `sdk` | the client method bound to an anchor route | the declared `route` ⟷ `client` rows in the repo's route ledgers | + +The `route` and `sdk` hops are what carry the derivation across the surface boundary the +package graph cannot cross: `auditMetaItem` (changed) → `GET /api/v1/meta/:type/:name/audit` +(`rest-server.ts` registrar) → `meta.getAudit` (`rest-route-ledger.ts`) → the token +`api/client-sdk.mdx` actually contains. + +**A local variable is not documentable surface.** That one rule is what drops the measured +false positive: `const singular = request.type;` inside a method body is not an anchor, so +`kernel/services-checklist.mdx` — whose only `singular` is a service *slot name* — is no +longer listed. A `const` object **is** a container (its keys are metadata property names, +which docs do name); a function body is not. + +### Two guards, and both publish what they removed + +The first build of this derivation was, on some PRs, *noisier* than the proxy it replaced +(134 rows where the old tool gave 26). Two guards fixed that, and both run **before** the +route bridge — a name left in the set does not merely add a noisy row, it mints noisy route +and SDK anchors from every registrar handler that mentions it: + +1. **Shape** — an anchor must be code-shaped (camelCase / PascalCase / snake_case / + dotted). `label`, `object`, `start`, `locale` and `sections` all arrived as real + declarations and matched 82, 113, 43, 13 and 10 of 178 pages; confining them to code + spans does not help, because those words live in code spans too. Reported as + `weakAnchorsDropped`. The recall cost is a genuinely lowercase export (`parse`, `mask`). +2. **Corpus share** — an anchor matching more than 15% of the corpus is a hub term, not an + identifier. `ObjectQL` is code-shaped, genuinely changed, and named by 59 of 178 pages; + it cannot tell an author which page to re-read. Reported as `overbroadAnchors`, with the + count that condemned it. + +Plus a cap on the route bridge itself: a symbol wired into more than 3 routes is a +cross-cutting helper, and "which routes mention this name" then answers *every* route. +Reported as `crossCuttingSymbols`. `SCREAMING_SNAKE` constants are kept out of the bridge +entirely — a data table is consulted by handlers, it is not their implementation. + +### What it cannot see is reported, never implied + +`anchorlessChanges` lists changed files that yielded no anchor at all; a non-empty value +means the list is incomplete **by a known amount**, and an empty `docs` beside it must +never be read as "no page documents this change". The superseded coarse set is still +computed and emitted as `packageMentionDocs`, labelled — an audit that deliberately wants +the wide net can still ask for it, and keeping it visible is how a reader tells a *narrow* +list from a *blind* one. The PR comment renders all of this in a collapsed section, because +the failure #9192 records was never the tool lying — it was the tool never signalling its +own limits at the point of use. + +### Measured, before and after + +Ten real PRs, each re-derived at its own merge base with its own docs corpus. `docs` rows: + +| PR / commit | old (package-mention) | new (anchor) | +|:--|--:|--:| +| #9191 — the three metadata read verbs (the filing card's specimen) | 4 | **3** | +| `0668f02a6` fix(rest): closed `ErrorCode` union on the error responder | 26 | 14 | +| `75b7c240a` feat(spec): `master_detail` + `controlled_by_parent` | 113 | 32 | +| `07ad42463` fix(cli): `os meta resync` skip-count explanation | 22 | **0** | +| `7a537ce90` feat(spec): strict top-level stack keys | 113 | 13 | +| `445ae4deb` fix(auth): auth emails follow the deployment locale | 13 | 3 | +| `30b1c636a` feat(spec): register 9 REST wire codes | 113 | 4 | +| `650cd3daa` fix(objectql): delete-cascade registry reads | 14 | **0** | +| `3851f87f0` feat(spec,plugin-security): partial field masking | 116 | 19 | +| `d5156b965` refactor(metadata-protocol): drop dead `objects` tolerances | 4 | 4 | + +The #9191 row reads 4 where the filing card says "the bot listed three pages": `docs` is +the full set and the comment partitions `content/docs/releases/v9.mdx` into its own +read-only section (#6893), so 3 editable rows + 1 release-owned row = 4. + +On #9191 the change is qualitative, not just smaller: all three previously-listed pages +are gone and the two pages the filing card measured as *missing* are back, each with the +anchor that put it there (`getAudit`/`getReferences` for `client-sdk.mdx`, `getHistory` +for `metadata-service.mdx`). + +The two zeroes are the honest shape of the trade, not a bug: `07ad42463` derives +`MetaResync` and `resyncSkipExplanationLine`, and no hand-written page names either, so the +run says so and points at the coarse set — where the old tool's 22 rows were every page +mentioning `@objectstack/cli`. A CLI **command name** (`os meta resync`) is exactly the +recall class the shape guard costs us: it is a lowercase word, so it cannot anchor. + +**Cost** (the card's open question): the anchor derivation reads the same 178-page corpus +the old one did, plus the 18 route-registrar/ledger sources (~875 KB) and one `git show` +per changed file per side. Measured end-to-end on the ten PRs above, `node affected-docs.mjs` +went from 85-195 ms to 114-582 ms. The heaviest case is the widest diff; every case stays +well under a second, against a job that already spends seconds checking out the repo and +setting up Node. It is the right default for every PR. **How a changed file maps to its package:** the package root is the **deepest ancestor directory with a `package.json`**, resolved from the filesystem — never a hand-kept @@ -88,8 +187,8 @@ stale are dropped before the changed-package roots are derived: The excluded counts are reported in the summary line and as `testFilesSkipped` / `scriptFilesSkipped` / `devOnlyManifestsSkipped` in `--json`, so the narrowing is never -silent. `--self-test` pins the classifiers *and* the package-root derivation against -paths that must and must not match (`commands/test.ts` is implementation; +silent. `--self-test` pins the classifiers, the package-root derivation *and* the anchor +derivation against inputs that must and must not match (`commands/test.ts` is implementation; `foo.conformance.test.ts` is not; a container directory must never come out as a package root; `dependencies` is never dev-only). @@ -196,9 +295,17 @@ out of an in-memory copy and requires that check to go red. ## 2. CI gate — `.github/workflows/docs-drift-check.yml` On any PR that touches `packages/**`, runs `affected-docs.mjs` against the base branch -and posts/updates a single advisory PR comment listing the docs that reference the -changed code. **Never fails the build** — it only flags drift at the source, before it -lands on `main`. Reviewers (or an on-demand audit run) decide whether to re-verify. +and posts/updates a single advisory PR comment listing the docs that name something the +change touched — each row carrying **the anchor that put it there**, so a wrong row is +reportable rather than merely annoying. **Never fails the build** — it only flags drift at +the source, before it lands on `main`. Reviewers (or an on-demand audit run) decide whether +to re-verify. + +The comment also carries a collapsed **"What this run could not see"** section: +anchorless files, cross-cutting symbols, over-broad anchors, and the coarse +package-mention count. That is the point-of-use half of #9192 — every one of the three +derived-list failures in that shift was caught only because a dev widened the probe past +what the tool offered, never because the tool signalled its own limits where it was read. ### The comment forks release-owned pages into a read-only section (#6893) diff --git a/scripts/docs-audit/affected-docs.mjs b/scripts/docs-audit/affected-docs.mjs index 0623e74cfd..85d6842e7e 100644 --- a/scripts/docs-audit/affected-docs.mjs +++ b/scripts/docs-audit/affected-docs.mjs @@ -1,22 +1,56 @@ #!/usr/bin/env node -// Map a set of `packages/**` code changes to the hand-written docs that reference -// the affected packages, so a doc-accuracy audit can be scoped to what actually -// changed instead of re-auditing every hand-written doc (178 of them today) each time. +// Map a set of `packages/**` code changes to the hand-written docs that NAME something +// the change touched, so a doc-accuracy audit can be scoped to what actually changed +// instead of re-auditing every hand-written doc (178 of them today) each time. // // Usage: // node scripts/docs-audit/affected-docs.mjs [sinceRef] # docs affected by changes since (default origin/main) // node scripts/docs-audit/affected-docs.mjs --all # every hand-written doc (full audit) -// node scripts/docs-audit/affected-docs.mjs --json [...] # emit JSON {docs, changedPackages, ...} instead of a path list -// node scripts/docs-audit/affected-docs.mjs --self-test # pin the change classifiers + package-root derivation (no repo state needed) +// node scripts/docs-audit/affected-docs.mjs --json [...] # emit JSON {docs, anchors, anchorlessChanges, ...} instead of a path list +// node scripts/docs-audit/affected-docs.mjs --self-test # pin the change classifiers, package-root and ANCHOR derivations (no repo state needed) // // Scope: hand-written docs only = content/docs/**/*.mdx MINUS content/docs/references/** // (references are generated from packages/spec and handled by a separate regenerate pass). // -// Heuristic: a doc is "affected" by a changed package P if the doc text mentions P's -// npm name (`@objectstack/`) or its repo path (the package's directory, e.g. -// `packages/services/service-automation`). Over-inclusion is intentionally preferred -// over misses; the periodic FULL audit is the backstop for docs that describe a -// package without naming it. +// DERIVATION (#9192): a doc is "affected" when it NAMES SOMETHING THE CHANGE TOUCHED — +// an ANCHOR — not when it merely mentions the changed package. +// +// The package-mention predicate this replaced ("which hand-written docs reference +// `@objectstack/metadata-protocol`") is a DEPENDENCY-GRAPH PROXY answering a SEMANTIC +// question, and it was measured wrong in BOTH directions on one real PR (#9191, the +// three read verbs `auditMetaItem` / `historyMetaItem` / `findReferencesToMeta`): +// +// - 3 pages listed, 1 relevant. `concepts/metadata-lifecycle.mdx` had zero hits on +// either probe set; `kernel/services-checklist.mdx` matched only on a service SLOT +// NAME. A page that names the package need not document the changed symbol. +// - 2 pages that DO document the changed surface were absent: `api/client-sdk.mdx` +// (`meta.getReferences` / `meta.getAudit`) and `kernel/contracts/metadata-service.mdx` +// (`getHistory?(type, name, …)`). They document it through the SDK/contract surface, +// which does not depend on the implementing package at all. +// +// Over-inclusion is NOT free here, and that is the correction: a wrong-both-ways +// advisory trains its reader to skip it, and then it fails on the PR where it is right +// (the same bill exclusion 1 below already paid). So this derivation is PRECISION-FIRST: +// a shorter right list beats a longer noisy one. Three anchor kinds, each exact: +// +// symbol — a DOCUMENTABLE declaration the diff touched: a top-level declaration, or a +// member of a top-level container (class / interface / type / enum / schema +// object). Locals inside a function body are NOT documentable surface — that +// single rule is what drops the `singular` false positive above. Taken from +// BOTH sides of the diff, so a REMOVED export still anchors the pages naming it. +// route — a wire path the change touched: a path literal on a changed line, plus every +// route whose REGISTRAR HANDLER references a changed symbol (that is the +// mechanical `auditMetaItem` → `GET /api/v1/meta/:type/:name/audit` link). +// sdk — the client method a route ledger BINDS to an anchor route +// (`meta.getAudit`). The declared cross-surface table is what carries the +// derivation over the boundary the package graph cannot cross, and it is what +// puts `api/client-sdk.mdx` back on the list. +// +// What this cannot see is REPORTED, never implied: files that yield no anchor at all are +// listed as `anchorlessChanges`, and the coarse package-mention set is still computed and +// emitted as `packageMentionDocs` — labelled coarse, not rendered as a work list. Silence +// from this tool must never be readable as "there is nothing there"; that misreading is +// the whole subject of #9192. // // Three exclusions, though — change classes that cannot make an implementation-accuracy // doc stale, dropped before the changed package roots are derived (everything else @@ -112,6 +146,107 @@ const isReleaseOwned = (doc) => doc.startsWith(RELEASE_OWNED_PREFIX); */ const DEV_ONLY_PACKAGE_JSON_KEYS = new Set(['scripts', 'devDependencies']); +/** + * Statement heads a LINE-BASED declaration probe must never mistake for a declared name. + * `if (…) {`, `switch (x) {` and `await something(` all have the shape "identifier then + * an opening bracket" that the member pattern looks for, and every one of them would + * otherwise become an anchor on the strength of a control-flow line. + */ +const NON_DECLARATION_HEADS = new Set([ + 'if', 'for', 'while', 'switch', 'catch', 'return', 'await', 'throw', 'new', 'else', 'do', + 'try', 'typeof', 'delete', 'void', 'yield', 'case', 'break', 'continue', 'with', 'in', 'of', + 'function', 'class', 'const', 'let', 'var', 'import', 'export', 'declare', 'this', 'super', + 'async', 'static', 'public', 'private', 'protected', 'readonly', 'abstract', 'override', + 'get', 'set', 'default', 'implements', 'extends', 'satisfies', 'as', +]); + +/** + * Declaration names too generic to identify anything. An anchor's whole job is to point + * at ONE surface; a name that half the corpus uses in an unrelated sense points at the + * corpus. These are dropped from the anchor set, never from the diff — the change is + * still counted, it just contributes no anchor through that name. + * + * `field` / `fields` are in here despite being real metadata vocabulary, and the reason + * is the doc side rather than the code side: as a DECLARATION name they are almost always + * a local shape (`const fields = …`), while as a doc token they appear in a code span on + * nearly every data-modelling page. That pairing is exactly the wrong-both-ways trade + * this rewrite exists to stop. A genuine field-surface change anchors through the + * property literal or the schema export instead. + */ +const GENERIC_ANCHOR_NAMES = new Set([ + 'type', 'types', 'name', 'names', 'value', 'values', 'data', 'result', 'results', + 'options', 'opts', 'config', 'context', 'ctx', 'error', 'errors', 'item', 'items', + 'list', 'index', 'key', 'keys', 'ids', 'request', 'response', 'req', 'res', 'limit', + 'offset', 'count', 'total', 'message', 'status', 'code', 'path', 'paths', 'file', + 'files', 'input', 'output', 'args', 'params', 'props', 'state', 'init', 'main', 'run', + 'test', 'build', 'check', 'singular', 'plural', 'entry', 'entries', 'record', 'records', + 'row', 'rows', 'field', 'fields', 'source', 'target', 'kind', 'mode', 'level', 'scope', + 'handler', 'callback', 'result', 'output', 'events', 'event', +]); + +/** + * Declaration probes, ordered — first match wins, so the keyword forms sit ahead of the + * catch-all member/property form (otherwise `const x = …` would be read as a member `x`). + * + * `container: true` marks a kind whose direct children are themselves documentable + * surface (a class's methods, an interface's members, a schema object's keys). A + * `function` is NOT a container: the `const` on line 2 of a function body is a local, and + * treating it as surface is precisely how `singular` reached the advisory. + */ +const DECL_PATTERNS = [ + { kind: 'class', container: true, re: /^\s*(?:export\s+)?(?:default\s+)?(?:declare\s+)?(?:abstract\s+)?class\s+([A-Za-z_$][\w$]*)/ }, + { kind: 'interface', container: true, re: /^\s*(?:export\s+)?(?:declare\s+)?interface\s+([A-Za-z_$][\w$]*)/ }, + { kind: 'enum', container: true, re: /^\s*(?:export\s+)?(?:declare\s+)?(?:const\s+)?enum\s+([A-Za-z_$][\w$]*)/ }, + { kind: 'namespace', container: true, re: /^\s*(?:export\s+)?(?:declare\s+)?(?:namespace|module)\s+([A-Za-z_$][\w$]*)/ }, + { kind: 'type', container: true, re: /^\s*(?:export\s+)?(?:declare\s+)?type\s+([A-Za-z_$][\w$]*)/ }, + { kind: 'function', container: false, re: /^\s*(?:export\s+)?(?:declare\s+)?(?:default\s+)?(?:async\s+)?function\s*\*?\s*([A-Za-z_$][\w$]*)/ }, + { kind: 'binding', container: null, re: /^\s*(?:export\s+)?(?:declare\s+)?(?:const|let|var)\s+([A-Za-z_$][\w$]*)/ }, + // Members and object/schema keys: `name(`, `name<`, `name:`, `name?:`, `name =`. + { kind: 'member', container: false, re: /^\s*(?:(?:public|private|protected|static|readonly|abstract|override|async|get|set)\s+)*\*?\s*([A-Za-z_$][\w$]*)\s*(?:\?\s*)?(?:[(<:]|=[^=>])/ }, +]; + +/** + * Where route REGISTRARS live. Deliberately a filename convention rather than a hand-kept + * file list — the same choice the package-root derivation made for the same reason (#4162: + * a hardcoded list fails again on container number eight). A registrar this misses costs + * recall on the `sdk` anchor kind only, and `anchorlessChanges` reports the silence. + */ +const REGISTRAR_FILE_RE = /(?:^|\/)(?:[\w.-]*route[\w.-]*|[\w.-]*-server)\.ts$/; + +/** Route LEDGERS — the declared `route` ⟷ `client` tables the `sdk` anchor rides on. */ +const LEDGER_FILE_RE = /(?:^|\/)[\w.-]*route-ledger\.ts$/; + +/** How far past a `path:` line a registrar's handler body is scanned for identifiers. */ +const REGISTRAR_HANDLER_WINDOW = 150; + +/** + * Above this many routes, a changed symbol is a CROSS-CUTTING helper rather than one + * route's implementation, and the symbol → route bridge stops firing for it. The three + * read verbs the #9192 measurement is built on map to exactly one route each; the REST + * error responders that blew the list up map to six and more. + */ +const MAX_ROUTES_PER_SYMBOL = 3; + +/** + * The share of the hand-written corpus above which an anchor is a hub term rather than an + * identifier. Calibrated against this repo's measured hub anchors — `ObjectQL` at 59/178 + * pages (33%) and `object` at 113/178 (63%) — versus the real ones a change should keep: + * `FieldSchema` at 10 and `SecurityPlugin` at 5. 15% (26 pages today) sits in that gap. + */ +const OVERBROAD_ANCHOR_SHARE = 0.15; + +const indentOf = (line) => line.length - line.trimStart().length; + +/** + * An anchor is CODE-SHAPED when its own spelling marks it as an identifier — + * camelCase, PascalCase, snake_case, dotted. Those may be matched anywhere in a doc. + * + * An all-lowercase single word cannot be told from prose, so it is matched only inside + * code spans and fenced blocks. Same anchor set either way; the shape decides how much + * of the page it is allowed to see. + */ +const isCodeShaped = (name) => /[A-Z]/.test(name.slice(1)) || name.includes('_') || name.includes('.'); + // Short-circuit before any git or filesystem work — the self-test needs no repo state. if (args.includes('--self-test')) { selfTest(); @@ -300,6 +435,213 @@ const liveManifestIo = { }, }; +// --- 2b. anchors: what the change actually touched --------------------------- + + +/** + * The declaration a single source LINE declares, or `null`. Line-based on purpose: this + * script is dependency-free by contract (the CI job that runs it never installs anything), + * so there is no TypeScript parser to reach for. The cost of that is bounded by the two + * rules around it — `NON_DECLARATION_HEADS` throws out statement heads, and only rank 0/1 + * results are ever accepted (see `documentableDeclarationsAt`). + */ +function declarationOn(line) { + if (/^\s*(?:[)}\]]|\/\/|\/\*|\*)/.test(line)) return null; // closers and comment bodies + for (const { kind, container, re } of DECL_PATTERNS) { + const m = line.match(re); + if (!m) continue; + const name = m[1]; + if (NON_DECLARATION_HEADS.has(name)) return null; + // A `const` is a container only when it is not a function in disguise: a schema + // object (`export const X = z.object({`) owns its keys, an arrow function owns locals. + const isContainer = container === null ? !/=>|\bfunction\b/.test(line) : container; + return { name, kind, container: isContainer }; + } + return null; +} + +/** + * The declaration chain enclosing (or standing on) `idx`, innermost first. + * + * Closing-bracket lines are skipped WITHOUT lowering the running indent: ` }> {` ends a + * multi-line signature, and letting it consume indent level 4 would hide the method + * signature above it and hand the change to the previous sibling method instead. + */ +function declarationChainAt(lines, idx) { + const chain = []; + let indent = indentOf(lines[idx]); + const own = declarationOn(lines[idx]); + if (own) chain.push({ ...own, indent }); + for (let i = idx - 1; i >= 0; i--) { + const line = lines[i]; + if (!line.trim()) continue; + if (/^\s*[)}\]]/.test(line)) continue; + const li = indentOf(line); + if (li >= indent) continue; + const d = declarationOn(line); + if (d) chain.push({ ...d, indent: li }); + indent = li; + if (li === 0) break; + } + return chain; +} + +/** + * The DOCUMENTABLE declarations a changed line belongs to — at most one name. + * + * Most-specific-wins: a changed method body anchors on the METHOD, not on its class. + * Emitting the container too would mean every edit anywhere in a 20k-line class flagged + * every page that names the class — the coarse-proxy failure this rewrite is undoing, + * reintroduced one level down. The container is the FALLBACK, used when the inner name is + * generic or absent (a changed entry inside `export const FIELD_TYPES = [...]` has no + * declaration of its own, and `FIELD_TYPES` is the right anchor for it). + */ +function documentableDeclarationsAt(lines, idx) { + const chain = declarationChainAt(lines, idx); + if (!chain.length) return []; + const outer = chain[chain.length - 1]; + const inner = chain.length > 1 ? chain[chain.length - 2] : null; + const usable = (d) => d && !GENERIC_ANCHOR_NAMES.has(d.name) && !GENERIC_ANCHOR_NAMES.has(d.name.toLowerCase()) && d.name.length >= 3; + if (inner && outer.container && usable(inner)) return [inner.name]; + if (usable(outer) && outer.kind !== 'member') return [outer.name]; + return []; +} + + +/** New- and old-side line numbers touched, parsed out of a `-U0` unified diff. */ +function changedLineNumbers(diffText) { + const oldLines = []; + const newLines = []; + for (const line of diffText.split('\n')) { + const m = line.match(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/); + if (!m) continue; + const oldStart = Number(m[1]); + const oldCount = m[2] === undefined ? 1 : Number(m[2]); + const newStart = Number(m[3]); + const newCount = m[4] === undefined ? 1 : Number(m[4]); + for (let i = 0; i < oldCount; i++) oldLines.push(oldStart + i); + for (let i = 0; i < newCount; i++) newLines.push(newStart + i); + } + return { oldLines, newLines }; +} + +/** + * The route tail of a registrar path literal: `${metaPath}/:type/:name/audit` → + * `/:type/:name/audit`. Interpolations are stripped rather than resolved — the tail is + * matched against a ledger row's full wire path by suffix, so the static part is enough. + * + * Returns `null` unless the result looks like an API ROUTE rather than a file path: at + * least two segments, at least one static segment, no segment carrying a source-file + * extension, and either a `:param`/`{param}` segment or an `/api/` prefix. Without that + * last clause every `packages/rest/src/...` written in a comment became a "route". + */ +function routeTailOf(literal) { + const stripped = String(literal).replace(/\$\{[^}]*\}/g, ''); + const m = stripped.match(/(?:\/[A-Za-z0-9_:.$*{}-]+){2,}/); + if (!m) return null; + const tail = m[0]; + const segs = tail.split('/').filter(Boolean); + if (segs.length < 2) return null; + if (segs.some((s) => /\.(?:ts|tsx|js|mjs|cjs|json|md|mdx|ya?ml|html|css)$/.test(s))) return null; + const isParam = (s) => s.startsWith(':') || (s.startsWith('{') && s.endsWith('}')); + if (!segs.some((s) => !isParam(s))) return null; + if (!segs.some(isParam) && !tail.startsWith('/api/')) return null; + return tail; +} + +/** + * A doc-side matcher for a route tail. Parameter segments match any of the three + * spellings a page may use — `:type`, `{type}`, or a concrete example value — so + * `GET /api/v1/meta/object/account/history` in a code block still counts as documenting + * `/:type/:name/history`. The static segments are what keep that from over-matching. + */ +function routePatternFor(tail) { + const isParam = (s) => s.startsWith(':') || (s.startsWith('{') && s.endsWith('}')); + const body = tail + .split('/') + .filter(Boolean) + .map((s) => (isParam(s) ? '(?::[A-Za-z_$][\\w$]*|\\{[A-Za-z_$][\\w$]*\\}|[A-Za-z0-9_%-]+)' : s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))) + .join('/'); + return new RegExp(`/${body}(?![\\w-])`); +} + +/** Route tails and identifier-shaped string literals appearing on the changed lines. */ +function literalAnchorsFromLines(lines, changed) { + const routes = new Set(); + const literals = new Set(); + for (const n of changed) { + const line = lines[n - 1]; + if (line === undefined) continue; + for (const m of line.replace(/\$\{[^}]*\}/g, '').matchAll(/(?:\/[A-Za-z0-9_:.$*{}-]+){2,}/g)) { + const tail = routeTailOf(m[0]); + if (tail) routes.add(tail); + } + for (const m of line.matchAll(/['"]([A-Za-z][\w.$-]{3,63})['"]/g)) { + const lit = m[1]; + if (GENERIC_ANCHOR_NAMES.has(lit.toLowerCase())) continue; + // Identifier-shaped only: snake_case, camelCase or dotted. A quoted English word + // ('ignore', 'utf8') is not a surface anyone documents by that spelling. + if (!/^[a-z][a-z0-9]*(?:_[a-z0-9]+)+$/.test(lit) && !/^[a-z]+(?:[A-Z][A-Za-z0-9]*)+$/.test(lit) && !/^[a-z][a-z0-9]*(?:\.[a-z][A-Za-z0-9]*)+$/.test(lit)) continue; + literals.add(lit); + } + } + return { routes, literals }; +} + +/** Documentable declaration names touched on one side of one file's diff. */ +function symbolAnchorsFromSource(text, changed) { + const lines = text.split('\n'); + const names = new Set(); + for (const n of changed) { + if (n - 1 < 0 || n - 1 >= lines.length) continue; + for (const name of documentableDeclarationsAt(lines, n - 1)) names.add(name); + } + return names; +} + +/** + * `path:` literals in a route registrar, each mapped to the identifiers its handler body + * mentions. This is the mechanical half of the SDK bridge: a changed protocol method + * appears in the handler of the route it serves, which the ledger then binds to a client + * method the docs actually name. + */ +function parseRegistrarSource(text) { + const lines = text.split('\n'); + const sites = []; + for (let i = 0; i < lines.length; i++) { + const m = lines[i].match(/(?:^|[\s{,(])path\s*:\s*([`'"])(.*?)\1/); + if (m) sites.push({ line: i, tail: routeTailOf(m[2]) }); + } + const byTail = new Map(); + for (let k = 0; k < sites.length; k++) { + const { line, tail } = sites[k]; + if (!tail) continue; + const next = k + 1 < sites.length ? sites[k + 1].line : lines.length; + const end = Math.min(next, line + REGISTRAR_HANDLER_WINDOW, lines.length); + let ids = byTail.get(tail); + if (!ids) byTail.set(tail, (ids = new Set())); + for (let j = line; j < end; j++) { + for (const id of lines[j].matchAll(/[A-Za-z_$][\w$]*/g)) ids.add(id[0]); + } + } + return byTail; +} + +/** `{ route, client }` rows out of a route ledger — the declared cross-surface table. */ +function parseLedgerSource(text) { + const rows = []; + const routeRe = /route\s*:\s*'([^']+)'/g; + let m; + while ((m = routeRe.exec(text)) !== null) { + const rest = text.slice(m.index, routeRe.lastIndex + 1200); + const nextRoute = rest.slice(1).search(/route\s*:\s*'/); + const window = nextRoute === -1 ? rest : rest.slice(0, nextRoute + 1); + const client = window.match(/client\s*:\s*'([^']+)'/); + rows.push({ route: m[1], client: client ? client[1] : null }); + } + return rows; +} + /** * Pin the change classifiers and the package-root derivation against known-good and * known-bad paths. The two ways this tool turns into a miss: an exclusion silently @@ -450,6 +792,206 @@ function selfTest() { ]; for (const [doc, want, label] of releaseOwnedCases) check('isReleaseOwned', label, doc, want, isReleaseOwned(doc)); + // ---- the anchor derivation (#9192) ---------------------------------------- + // The measured failure this replaced was wrong in BOTH directions, so the pins come in + // both directions too: the false positive that must stay dropped (a local named + // `singular`), and the true positive that must stay found (a changed method reaching + // `client-sdk.mdx` through the route registrar and the route ledger). + + // A verbatim-shaped excerpt of `packages/metadata-protocol/src/protocol.ts` at the + // change #9192 was measured on. Indentation is load-bearing — the rank rule is what + // separates the method from the local, and `}> {` is the closer that must not consume + // the signature's indent level. + const protocolSource = [ + 'export class ObjectStackProtocolImplementation implements IObjectStackProtocol {', + ' /** ADR-0010 §3.6 protection-audit trail. */', + ' async auditMetaItem(request: {', + ' type: string;', + ' name: string;', + ' }): Promise<{', + ' events: Array<{ note: string | null }>;', + ' }> {', + ' request = canonicalizeMetaRequestType(request);', + ' const singular = request.type;', + ' return this.readAudit(singular);', + ' }', + '', + ' async historyMetaItem(request: { type: string }): Promise {', + ' if (!ObjectStackProtocolImplementation.isOverlayAllowed(request.type)) {', + ' return;', + ' }', + ' }', + '}', + ].join('\n'); + const anchorsAt = (src, lineNo) => symbolAnchorsFromSource(src, [lineNo]); + const symbolCases = [ + // [1-based line, expected anchor set, label] + [9, ['auditMetaItem'], 'a changed METHOD BODY anchors on the method, not on its 20k-line class'], + [10, ['auditMetaItem'], 'a local `const singular` is NOT documentable surface — the #9192 false positive'], + [11, ['auditMetaItem'], 'a plain statement still resolves to the enclosing method'], + [3, ['auditMetaItem'], 'the signature line itself'], + [15, ['historyMetaItem'], 'the closer `}` of the previous method must not hand this to auditMetaItem'], + [16, ['historyMetaItem'], 'a nested `if` block resolves past the intermediate scope'], + [1, ['ObjectStackProtocolImplementation'], 'a changed CLASS LINE anchors on the class — the container fallback'], + ]; + for (const [line, want, label] of symbolCases) { + check('symbolAnchorsFromSource', label, `line ${line}`, JSON.stringify(want), JSON.stringify([...anchorsAt(protocolSource, line)])); + } + + // A schema object: its KEYS are documentable surface, because a metadata property name + // is exactly what a docs page names. Same rank rule, opposite verdict from the local + // above — a `const` object is a container, a function body is not. + const schemaSource = [ + 'export const ObjectSchema = z.object({', + ' controlled_by_parent: z.boolean().optional(),', + '});', + '', + 'export function buildObject(input: unknown) {', + ' const draftBuffer = normalize(input);', + ' return draftBuffer;', + '}', + ].join('\n'); + const containerCases = [ + [2, ['controlled_by_parent'], 'a schema KEY is surface — the const object is a container'], + [6, ['buildObject'], 'a local inside a FUNCTION is not surface; the function is'], + [1, ['ObjectSchema'], 'the schema declaration itself'], + ]; + for (const [line, want, label] of containerCases) { + check('symbolAnchorsFromSource', label, `line ${line}`, JSON.stringify(want), JSON.stringify([...anchorsAt(schemaSource, line)])); + } + + // Statement heads must never be read as declarations — `if (x) {` has the same shape as + // a class member, and a control-flow line becoming an anchor is silent noise. + const declCases = [ + [' if (limit > 0) {', null, 'an if-statement is not a declaration'], + [' switch (kind) {', null, 'a switch is not a declaration'], + [' return this.readAudit(singular);', null, 'a return is not a declaration'], + [' } else if (x) {', null, 'a closer line is never a declaration'], + [' // path: `/api/v1/x/:id`', null, 'a comment body is never a declaration'], + ['export const FIELD_TYPES = [', 'FIELD_TYPES', 'an exported const'], + ['export interface IMetadataService {', 'IMetadataService', 'an exported interface'], + [' async auditMetaItem(request: {', 'auditMetaItem', 'a class method'], + [' getHistory?(type: string): Promise;', 'getHistory', 'an OPTIONAL interface member — the `?` must not hide it'], + ]; + for (const [line, want, label] of declCases) { + const d = declarationOn(line); + check('declarationOn', label, line.trim(), want, d ? d.name : null); + } + const containerFlagCases = [ + ['export const ObjectSchema = z.object({', true, 'a schema object owns its keys'], + ['export const handle = (req) => {', false, 'an arrow function owns locals, not surface'], + ['export const run = function () {', false, 'a function expression is not a container'], + ['export class Protocol {', true, 'a class owns its methods'], + ['export function build(x) {', false, 'a function body holds locals'], + ]; + for (const [line, want, label] of containerFlagCases) { + const d = declarationOn(line); + check('declarationOn.container', label, line.trim(), want, d ? d.container : null); + } + + // The shape guard. Measured pull in both columns: the left-hand names identify one + // surface; the right-hand ones matched 82-113 of 178 pages apiece. + const shapeCases = [ + ['auditMetaItem', true, 'camelCase'], ['ObjectSchema', true, 'PascalCase'], + ['ERROR_CODE_LEDGER', true, 'SCREAMING_SNAKE'], ['controlled_by_parent', true, 'snake_case'], + ['meta.getAudit', true, 'a dotted client path'], + ['label', false, 'a single lowercase word is corpus vocabulary'], + ['object', false, 'ditto — 113 of 178 pages'], + ['locale', false, 'ditto'], ['query', false, 'an SDK method tail that is also English'], + ]; + for (const [name, want, label] of shapeCases) check('isCodeShaped', label, name, want, isCodeShaped(name)); + + // Route tails: an API route is an anchor, a source path written in a comment is not. + const routeTailCases = [ + ['${metaPath}/:type/:name/audit', '/:type/:name/audit', 'interpolation stripped, tail kept'], + ['/api/v1/meta/:type/:name/history', '/api/v1/meta/:type/:name/history', 'a full wire path'], + ['/api/v1/meta/types', '/api/v1/meta/types', 'static-only, but under /api/'], + ['packages/rest/src/rest-route-ledger.ts', null, 'a SOURCE PATH is not a route'], + ['content/docs/api/client-sdk.mdx', null, 'a docs path is not a route'], + ['/meta/types', null, 'static-only and not under /api/ — too weak to anchor'], + ['/audit', null, 'one segment is not a route'], + ]; + for (const [literal, want, label] of routeTailCases) check('routeTailOf', label, literal, want, routeTailOf(literal)); + + const routeMatchCases = [ + ['/:type/:name/history', 'see `GET /api/v1/meta/:type/:name/history` for the trail', true, 'the colon spelling'], + ['/:type/:name/history', 'GET /api/v1/meta/{type}/{name}/history', true, 'the brace spelling'], + ['/:type/:name/history', 'GET /api/v1/meta/object/account/history', true, 'a concrete example URL'], + ['/:type/:name/history', 'GET /api/v1/meta/object/account/audit', false, 'a different static segment does not match'], + ['/:type/:name/history', 'the history of a record', false, 'prose does not match a route'], + ]; + for (const [tail, text, want, label] of routeMatchCases) { + check('routePatternFor', label, `${tail} vs ${JSON.stringify(text)}`, want, routePatternFor(tail).test(text)); + } + + // `-U0` hunk headers, including the one-line form where the count is omitted. + const diffText = [ + 'diff --git a/x.ts b/x.ts', + '@@ -6376 +6376,3 @@', + '-old', + '+a', + '+b', + '+c', + '@@ -13396,2 +13440 @@', + ].join('\n'); + const parsed = changedLineNumbers(diffText); + check('changedLineNumbers', 'new-side lines', 'hunks', JSON.stringify([6376, 6377, 6378, 13440]), JSON.stringify(parsed.newLines)); + check('changedLineNumbers', 'old-side lines (a REMOVED export still anchors)', 'hunks', JSON.stringify([6376, 13396, 13397]), JSON.stringify(parsed.oldLines)); + + // The two declared tables the SDK bridge rides on. + const registrarSource = [ + 'this.routeManager.register({', + " method: 'GET',", + ' path: `${metaPath}/:type/:name/audit`,', + ' handler: async (req, res) => {', + ' const p = await this.resolveProtocol();', + ' if (typeof p.auditMetaItem !== \'function\') return;', + ' },', + '});', + 'this.routeManager.register({', + " method: 'GET',", + ' path: `${metaPath}/:type/:name/history`,', + ' handler: async (req, res) => {', + ' await p.historyMetaItem(req.params);', + ' },', + '});', + ].join('\n'); + const registrar = parseRegistrarSource(registrarSource); + check('parseRegistrarSource', 'the audit route is indexed by its tail', 'tail', true, registrar.has('/:type/:name/audit')); + check('parseRegistrarSource', 'its handler symbols are captured', 'auditMetaItem', true, !!registrar.get('/:type/:name/audit')?.has('auditMetaItem')); + check('parseRegistrarSource', 'a handler does NOT absorb the NEXT route\'s symbols', 'historyMetaItem', false, !!registrar.get('/:type/:name/audit')?.has('historyMetaItem')); + check('parseRegistrarSource', 'the second route is indexed too', 'historyMetaItem', true, !!registrar.get('/:type/:name/history')?.has('historyMetaItem')); + + const ledgerSource = [ + 'export const REST_ROUTE_LEDGER = [', + " { route: 'GET /api/v1/meta/:type/:name/references', family: 'metadata', disposition: 'sdk', client: 'meta.getReferences' },", + " { route: 'GET /api/v1/meta/:type/:name/audit', family: 'metadata', disposition: 'sdk', client: 'meta.getAudit' },", + " { route: 'GET /api/v1/health', family: 'ops', disposition: 'server-only' },", + '];', + ].join('\n'); + const ledger = parseLedgerSource(ledgerSource); + check('parseLedgerSource', 'every row is read', 'row count', 3, ledger.length); + check('parseLedgerSource', 'the audit row binds its client method', 'meta.getAudit', 'meta.getAudit', ledger.find((r) => r.route.endsWith('/audit'))?.client); + check('parseLedgerSource', 'a server-only row claims no client', 'null client', null, ledger.find((r) => r.route.endsWith('/health'))?.client); + check('parseLedgerSource', 'a row never inherits the NEXT row\'s client', 'references', 'meta.getReferences', ledger[0].client); + + // End to end over those three fixtures: the #9192 recall miss must come back. + // `auditMetaItem` (changed) → `/:type/:name/audit` (registrar) → `meta.getAudit` + // (ledger) → the token `api/client-sdk.mdx` actually contains. + const bridged = registrar.get('/:type/:name/audit'); + const bridgeRow = ledger.find((r) => bridged && r.route.endsWith('/:type/:name/audit')); + check('bridge', 'a changed protocol method reaches the SDK method the docs name', 'auditMetaItem → getAudit', 'getAudit', bridgeRow?.client?.split('.').pop()); + + // String literals on a changed line: an identifier-shaped one is surface, English is not. + const litLines = [" if (rule === 'controlled_by_parent') return maskFieldValue(v);", " fs.readFileSync(p, 'utf8');", " logger.warn('ignore');"]; + const lits = literalAnchorsFromLines(litLines, [1, 2, 3]).literals; + const literalCases = [ + ['controlled_by_parent', true, 'a snake_case literal IS an authoring surface'], + ['utf8', false, 'an encoding name is not surface'], + ['ignore', false, 'an English word is not surface'], + ]; + for (const [lit, want, label] of literalCases) check('literalAnchorsFromLines', label, lit, want, lits.has(lit)); + if (failed) { console.error(`\n✗ affected-docs self-test failed (${failed} case(s)).`); process.exit(1); @@ -487,16 +1029,199 @@ for (const dir of pkgRoots) { changedPackages.push({ dir, name }); } -// --- 3. match docs that mention an affected package ------------------------ +// --- 3. derive the ANCHORS the change touched ------------------------------ +// One pass per changed file, both sides of the diff: the HEAD side for what the change +// now declares, the base side so a REMOVED export still anchors the pages naming it. +const symbolAnchors = new Set(); +const routeAnchors = new Set(); +const literalAnchors = new Set(); +const anchorlessChanges = []; + +const readAt = (ref, file) => { + try { return sh(`git show ${ref}:${file}`); } catch { return null; } +}; + +for (const f of implementationChanges) { + if (!/\.(?:ts|tsx|js|mjs|cjs)$/.test(f)) { anchorlessChanges.push(f); continue; } + let diffText = ''; + try { diffText = sh(`git diff -U0 ${baseRef} HEAD -- ${JSON.stringify(f)}`); } catch { /* keep empty */ } + const { oldLines, newLines } = changedLineNumbers(diffText); + const before = oldLines.length ? readAt(baseRef, f) : null; + const after = newLines.length ? (readAt('HEAD', f) ?? (existsSync(join(repoRoot, f)) ? readFileSync(join(repoRoot, f), 'utf8') : null)) : null; + let found = 0; + for (const [text, changed] of [[after, newLines], [before, oldLines]]) { + if (!text) continue; + for (const name of symbolAnchorsFromSource(text, changed)) { symbolAnchors.add(name); found++; } + const { routes, literals } = literalAnchorsFromLines(text.split('\n'), changed); + for (const r of routes) { routeAnchors.add(r); found++; } + for (const l of literals) { literalAnchors.add(l); found++; } + } + if (!found) anchorlessChanges.push(f); +} + +// --- 3b. admit only DISCRIMINATING anchors, then bridge from the survivors -------- +// Two guards stand between the raw anchor set and the list, and BOTH publish what they +// removed. They exist because the first measured build of this derivation was, on some +// PRs, noisier than the package proxy it replaced — 134 rows where the old tool gave 26. +// +// 1. SHAPE. An anchor must be code-shaped (camelCase / PascalCase / snake_case / +// dotted). A single all-lowercase word cannot be told from the vocabulary the docs +// are written in: `label`, `object`, `start`, `locale` and `sections` all arrived as +// real declarations and matched 82, 113, 43, 13 and 10 pages respectively. Confining +// them to code spans does not help — those words live in code spans too. The recall +// cost is a genuinely lowercase export (`parse`, `mask`), listed in +// `weakAnchorsDropped` rather than swallowed. +// 2. CORPUS SHARE. An anchor matching more than `OVERBROAD_ANCHOR_SHARE` of the corpus +// is a hub term, not an identifier: `ObjectQL` is code-shaped, genuinely changed, and +// named by 59 of 178 pages — it cannot tell an author which page to re-read. Dropped +// and published in `overbroadAnchors`, with the count that condemned it. +// +// Both guards run BEFORE the bridge, not after it, and that ordering is the fix rather +// than a detail: the bridge answers "which routes mention this name", so a name left in +// the set does not merely add a noisy row — it mints noisy ROUTE and SDK anchors from +// every registrar handler that happens to mention it. Measured both ways: `label` / +// `start` / `subject` (locals in the auth-email change 445ae4deb) pulled `/:object/import` +// and `/forms/:slug` into an advisory about email templates, and `ObjectQL` did the same +// to the objectql cascade fix 650cd3daa. +const docTexts = handwritten.map((doc) => readFileSync(join(repoRoot, doc), 'utf8')); +const overbroadLimit = Math.max(3, Math.floor(handwritten.length * OVERBROAD_ANCHOR_SHARE)); +const weakAnchorsDropped = []; +const overbroadAnchors = []; +const anchors = []; +const hitsByAnchor = []; +const escape = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +const symbolRe = (name) => new RegExp(`(? new RegExp(`(? overbroadLimit) { overbroadAnchors.push(`${token} (${kind}, ${docs.length} pages)`); return false; } + anchors.push({ kind, token }); + hitsByAnchor.push(docs); + return true; +} + +// PHASE 1 — the anchors read straight off the diff. +const bridgeSymbols = []; +for (const name of [...symbolAnchors].sort()) { + if (!admitAnchor('symbol', name, symbolRe(name))) continue; + // A SCREAMING_SNAKE constant is a data table, not a route's implementation: it is + // referenced by handlers that merely consult it. Admitted as a doc anchor (it names a + // real surface — `ERROR_CODE_LEDGER` found 4 pages on 30b1c636a), but kept OUT of the + // route bridge, where it dragged `/approvals/requests/:id/remind` into a wire-code + // registration change. + if (!/^[A-Z0-9_$]+$/.test(name)) bridgeSymbols.push(name); +} +for (const name of [...literalAnchors].sort()) admitAnchor('literal', name, symbolRe(name)); + +// PHASE 2 — carry the surviving symbols across the surface boundary the package graph +// cannot cross. A changed protocol method appears in the HANDLER of the route it serves; +// the route ledgers then bind that route to the client method the SDK docs actually name. +// Both hops are declared data in the repo, not inference — and this is the hop that puts +// `api/client-sdk.mdx` back on the list for a `packages/metadata-protocol` change. +const sdkAnchors = new Set(); +const crossCuttingSymbols = []; +if (bridgeSymbols.length) { + const registrarFiles = []; + const walkSrc = (dir) => { + let entries; + try { entries = readdirSync(dir, { withFileTypes: true }); } catch { return; } + for (const e of entries) { + if (e.name === 'node_modules' || e.name === 'dist' || e.name === '.turbo') continue; + const p = join(dir, e.name); + if (e.isDirectory()) walkSrc(p); + else if (e.isFile() && e.name.endsWith('.ts') && !isTestFile(e.name)) { + const rel = relative(repoRoot, p); + if (LEDGER_FILE_RE.test(rel) || REGISTRAR_FILE_RE.test(rel)) registrarFiles.push(rel); + } + } + }; + walkSrc(join(repoRoot, 'packages')); + + const ledgerRows = []; + const registrarByTail = new Map(); + for (const rel of registrarFiles) { + let text; + try { text = readFileSync(join(repoRoot, rel), 'utf8'); } catch { continue; } + if (LEDGER_FILE_RE.test(rel)) ledgerRows.push(...parseLedgerSource(text)); + if (REGISTRAR_FILE_RE.test(rel)) { + for (const [tail, ids] of parseRegistrarSource(text)) { + let acc = registrarByTail.get(tail); + if (!acc) registrarByTail.set(tail, (acc = new Set())); + for (const id of ids) acc.add(id); + } + } + } + + // symbol → route, capped: the bridge answers "which routes mention this name", and for + // a CROSS-CUTTING helper that is every route it is wired into. Measured on the REST + // error-responder change 0668f02a6: `sendError` & co. pulled in six unrelated route + // families whose pages document nothing that change touched. Above the cap a symbol + // contributes no route anchor — and says so in `crossCuttingSymbols`. + const routesBySymbol = new Map(); + for (const [tail, ids] of registrarByTail) { + for (const s of bridgeSymbols) { + if (!ids.has(s)) continue; + let tails = routesBySymbol.get(s); + if (!tails) routesBySymbol.set(s, (tails = new Set())); + tails.add(tail); + } + } + for (const [s, tails] of routesBySymbol) { + if (tails.size > MAX_ROUTES_PER_SYMBOL) { crossCuttingSymbols.push(`${s} (${tails.size} routes)`); continue; } + for (const t of tails) routeAnchors.add(t); + } + // route → client method (the ledger's declared binding), and the reverse direction for + // free: a changed SDK method name pulls in the route it is bound to. + for (const { route, client } of ledgerRows) { + if (!client) continue; + const tail = client.split('.').pop(); + if ([...routeAnchors].some((t) => route.endsWith(t))) { + sdkAnchors.add(client); + // The BARE tail is an anchor only when its own spelling is distinctive. + // `getBookTree` identifies one method; `import` / `query` / `revoke` are English, + // and matching them corpus-wide put 116 and 84 pages on the list respectively + // (measured, 0668f02a6). The dotted form (`data.query`) stays, and it is precise. + if (tail && isCodeShaped(tail) && !GENERIC_ANCHOR_NAMES.has(tail.toLowerCase())) sdkAnchors.add(tail); + } else if (tail && bridgeSymbols.includes(tail)) { + const routeTail = routeTailOf(route.replace(/^[A-Z]+\s+/, '')); + if (routeTail) routeAnchors.add(routeTail); + } + } +} + +// PHASE 3 — the bridged anchors face the same two guards. +for (const name of [...sdkAnchors].sort()) admitAnchor('sdk', name, dottedRe(name)); +// Route tails are never "weak": a multi-segment wire path is distinctive by construction. +for (const tail of [...routeAnchors].sort()) admitAnchor('route', tail, routePatternFor(tail)); + +// --- 3c. the pages that name a surviving anchor ---------------------------- +const affectedByDoc = new Map(); +for (let k = 0; k < anchors.length; k++) { + for (const i of hitsByAnchor[k]) { + let via = affectedByDoc.get(i); + if (!via) affectedByDoc.set(i, (via = [])); + via.push(`${anchors[k].token} (${anchors[k].kind})`); + } +} const affected = []; -for (const doc of handwritten) { - const text = readFileSync(join(repoRoot, doc), 'utf8'); - const hits = []; +for (let i = 0; i < handwritten.length; i++) { + const via = affectedByDoc.get(i); + if (via) affected.push({ doc: handwritten[i], via: [...new Set(via)], releaseOwned: isReleaseOwned(handwritten[i]) }); +} + +// The superseded package-mention set, kept and LABELLED rather than deleted. It is the +// coarse over-approximation this rewrite stopped presenting as a work list; an audit that +// deliberately wants the wide net (the periodic backstop) can still ask for it, and +// keeping it visible is how a reader can tell "narrow list" from "nothing found". +const packageMentionDocs = []; +for (let i = 0; i < handwritten.length; i++) { for (const { dir, name } of changedPackages) { - if (name && text.includes(name)) hits.push(name); - else if (text.includes(dir)) hits.push(dir); + if ((name && docTexts[i].includes(name)) || docTexts[i].includes(dir)) { packageMentionDocs.push(handwritten[i]); break; } } - if (hits.length) affected.push({ doc, via: [...new Set(hits)], releaseOwned: isReleaseOwned(doc) }); } // Report what was excluded rather than dropping it silently — a tool that quietly @@ -507,16 +1232,41 @@ if (scriptFilesSkipped > 0) skipNotes.push(`${scriptFilesSkipped} tooling script if (devOnlyManifestsSkipped > 0) skipNotes.push(`${devOnlyManifestsSkipped} package.json edit(s) excluded — only dev-time keys (${[...DEV_ONLY_PACKAGE_JSON_KEYS].join('/')}) changed`); const skipNote = skipNotes.length ? ` (${skipNotes.join('; ')})` : ''; +const anchorSummary = anchors.length + ? `${anchors.length} anchor(s) — ${symbolAnchors.size} symbol, ${routeAnchors.size} route, ${sdkAnchors.size} sdk, ${literalAnchors.size} literal` + : 'no anchors derived'; +const anchorlessNote = anchorlessChanges.length + ? `; ⚠️ ${anchorlessChanges.length} changed file(s) yielded no anchor — this run cannot see pages documenting them` + : ''; +const overbroadNote = overbroadAnchors.length + ? `; ${overbroadAnchors.length} over-broad anchor(s) dropped (${overbroadAnchors.join(', ')})` + : ''; +const crossCuttingNote = crossCuttingSymbols.length + ? `; ${crossCuttingSymbols.length} cross-cutting symbol(s) contributed no route anchor (${crossCuttingSymbols.join(', ')})` + : ''; + emit( affected.map((a) => a.doc), changedPackages, - `${affected.length} docs affected by ${changedPackages.length} changed package(s) since ${sinceRef}${skipNote}`, + `${affected.length} docs name something this change touched (${anchorSummary}) across ${changedPackages.length} changed package(s) since ${sinceRef}${skipNote}${anchorlessNote}${crossCuttingNote}${overbroadNote}`, affected, { testFilesSkipped, scriptFilesSkipped, devOnlyManifestsSkipped }, + { + anchors: anchors.map((a) => ({ kind: a.kind, token: a.token })), + anchorlessChanges, + crossCuttingSymbols, + weakAnchorsDropped, + overbroadAnchors, + packageMentionDocs, + }, ); -function emit(docList, changedPackages, summary, detail, skipped = {}) { +function emit(docList, changedPackages, summary, detail, skipped = {}, anchorInfo = {}) { const { testFilesSkipped = 0, scriptFilesSkipped = 0, devOnlyManifestsSkipped = 0 } = skipped; + const { + anchors: anchorList = [], anchorlessChanges: anchorless = [], crossCuttingSymbols: crossCutting = [], + weakAnchorsDropped: weak = [], overbroadAnchors: overbroad = [], packageMentionDocs: coarse = [], + } = anchorInfo; if (asJson) { process.stdout.write( JSON.stringify( @@ -532,6 +1282,27 @@ function emit(docList, changedPackages, summary, detail, skipped = {}) { // A partition, not a filter — `releaseOwnedDocs ⊆ docs` always. releaseOwnedDocs: docList.filter(isReleaseOwned), detail: detail || null, + // What the change was found to TOUCH. Published so a reader can check the + // derivation instead of trusting it — the failure #9192 records is a derived + // list consumed as authoritative, and an anchor set is the cheapest way to + // make "why is this page here / why is that one not" answerable at a glance. + anchors: anchorList, + // The declared blind spot: changed files this run could derive nothing from. + // Non-empty means the list below is INCOMPLETE by a known amount — never read + // an empty `docs` as "no page documents this change" while this is non-empty. + anchorlessChanges: anchorless, + // The other declared narrowing: symbols wired into so many routes that the + // route bridge would have answered "every route" instead of "this one". + crossCuttingSymbols: crossCutting, + // Anchors the two guards removed, each with the reason it was removed. Neither + // guard is allowed to narrow the list silently — that is the #9192 failure mode + // one level down, and these two fields are what keep it reviewable. + weakAnchorsDropped: weak, + overbroadAnchors: overbroad, + // The superseded COARSE set: docs merely MENTIONING a changed package. Kept for + // the deliberately-wide backstop, and labelled so it is never mistaken for the + // work list again (it was measured wrong in both directions — see the header). + packageMentionDocs: coarse, testFilesSkipped, scriptFilesSkipped, devOnlyManifestsSkipped, diff --git a/scripts/docs-audit/check-affected-docs.mjs b/scripts/docs-audit/check-affected-docs.mjs index 8aada10f64..2a5ea7aa9e 100644 --- a/scripts/docs-audit/check-affected-docs.mjs +++ b/scripts/docs-audit/check-affected-docs.mjs @@ -3,7 +3,9 @@ /** * check-affected-docs (#9187) — the discoverable name for affected-docs.mjs's - * own `--self-test`. + * own `--self-test`, which pins the change classifiers, the package-root derivation + * and (since #9192) the symbol / route / SDK anchor derivation that decides which + * hand-written pages a code change is advertised against. * * node scripts/docs-audit/check-affected-docs.mjs *