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 *