Skip to content

Commit 2e1d82a

Browse files
fix(docs-audit): derive the docs-drift advisory from what pages document, not package edges (#9229)
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. Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza Co-authored-by: Claude <noreply@anthropic.com>
1 parent d31785f commit 2e1d82a

4 files changed

Lines changed: 965 additions & 44 deletions

File tree

.github/workflows/docs-drift-check.yml

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
name: Docs Drift Check
22

3-
# When a PR changes packages/** code, flag the hand-written docs that reference the
4-
# affected packages so they can be re-verified for implementation accuracy before the
5-
# drift lands on main. Advisory only — posts a PR comment, never fails the build.
6-
# The actual LLM audit is run on-demand / on a schedule via the `docs-accuracy-audit`
7-
# workflow, scoped to exactly the docs this check lists.
3+
# When a PR changes packages/** code, flag the hand-written docs that NAME something the
4+
# change touched — a symbol, a wire route, or the SDK method a route ledger binds to it —
5+
# so they can be re-verified for implementation accuracy before the drift lands on main.
6+
# Advisory only: posts a PR comment, never fails the build. The actual LLM audit is run
7+
# on-demand / on a schedule via the `docs-accuracy-audit` workflow, scoped to exactly the
8+
# docs this check lists.
9+
#
10+
# It used to list pages by PACKAGE DEPENDENCY ("which docs mention @objectstack/x"), and
11+
# #9192 measured that wrong in both directions on a real PR: 2 of 3 listed pages were
12+
# irrelevant, and the 2 pages that actually documented the changed surface were missing
13+
# because they document it through the SDK, which does not depend on the implementing
14+
# package. The comment now also states what the run could NOT see — the derivation being
15+
# read past its precision, with its silence taken for absence, is what #9192 records.
816

917
on:
1018
pull_request:
@@ -75,6 +83,30 @@ jobs:
7583
const docs = data.docs || [];
7684
const pkgs = (data.changedPackages || []).map(p => p.name || p.dir);
7785
const marker = '<!-- docs-drift-check -->';
86+
// #9192 — the derivation now matches pages that NAME something the change
87+
// touched (symbol / route / SDK-method anchors) instead of pages that merely
88+
// mention a changed package. Everything the derivation could NOT see is
89+
// stated in the comment, at the point of use: the failure this fixed was not
90+
// the tool lying, it was a reader taking its silence for absence.
91+
const anchorList = data.anchors || [];
92+
const anchorless = data.anchorlessChanges || [];
93+
const overbroad = data.overbroadAnchors || [];
94+
const crossCutting = data.crossCuttingSymbols || [];
95+
const weak = data.weakAnchorsDropped || [];
96+
const coarse = data.packageMentionDocs || [];
97+
const rederive = `node scripts/docs-audit/affected-docs.mjs --json origin/${baseRef}`;
98+
const limits = [];
99+
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`);
100+
if (crossCutting.length) limits.push(`**${crossCutting.length}** cross-cutting symbol(s) contributed no route anchor: \`${crossCutting.join('`, `')}\``);
101+
if (overbroad.length) limits.push(`**${overbroad.length}** anchor(s) matched too much of the corpus to be a work list: \`${overbroad.join('`, `')}\``);
102+
if (weak.length) limits.push(`**${weak.length}** name(s) were too generic to anchor anything (single lowercase words)`);
103+
// Rendered whenever there is anything to say, INCLUDING when the only thing to
104+
// say is "the wide net exists and holds N pages". A short list is the right
105+
// answer here, but a reader must be able to tell a short list from a blind one
106+
// without leaving the PR.
107+
const limitsBlock = (limits.length || coarse.length)
108+
? ['', '<details><summary>What this run could not see</summary>', '', ...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\`.`, '</details>']
109+
: [];
78110
// The release-owned rows are PARTITIONED OUT of the editable list, never
79111
// dropped (#6893, following the #4920 ruling). They keep getting audited —
80112
// `docs` above is still the full set the audit workflow is scoped to — but
@@ -96,14 +128,19 @@ jobs:
96128
const capped = editable.length > EDITABLE_ROW_CAP;
97129
let body;
98130
if (docs.length === 0) {
99-
body = `${marker}\n### 📓 Docs Drift Check\nNo hand-written docs reference the ${pkgs.length} changed package(s). ✅`;
131+
// "Nothing found" and "I derived nothing to look for" are DIFFERENT results
132+
// and must never render alike — that conflation is #9192's own subject.
133+
const headline = anchorList.length === 0
134+
? `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.`
135+
: `**${anchorList.length}** anchor(s) derived from **${pkgs.length}** changed package(s); no hand-written page names any of them. ✅`;
136+
body = [marker, '### 📓 Docs Drift Check', headline, ...limitsBlock].join('\n');
100137
} else {
101138
const detail = (data.detail || []).reduce((m, d) => (m[d.doc] = d.via, m), {});
102139
const row = d => `- \`${d}\`${detail[d] ? ` _(via ${detail[d].join(', ')})_` : ''}`;
103140
body = [
104141
marker,
105142
'### 📓 Docs Drift Check',
106-
`This PR changes **${pkgs.length}** package(s): ${pkgs.map(p => `\`${p}\``).join(', ')}.`,
143+
`This PR changes **${pkgs.length}** package(s): ${pkgs.map(p => `\`${p}\``).join(', ')}, touching **${anchorList.length}** documentable anchor(s).`,
107144
];
108145
if (capped) {
109146
// NO row list above the cap — folding rows behind a details tag would
@@ -113,7 +150,7 @@ jobs:
113150
// fidelity is one command away, never lost.
114151
body.push(
115152
'',
116-
`**${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}\`.`,
153+
`**${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}\`.`,
117154
);
118155
if (readOnly.length) {
119156
body.push(
@@ -125,15 +162,15 @@ jobs:
125162
if (editable.length) {
126163
body.push(
127164
'',
128-
`**${editable.length}** hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:`,
165+
`**${editable.length}** hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:`,
129166
'',
130167
editable.map(row).join('\n'),
131168
);
132169
}
133170
if (readOnly.length) {
134171
body.push(
135172
'',
136-
`⛔ **${readOnly.length}** release-owned page(s) ${editable.length ? 'also ' : ''}reference the affected code. These are **read-only**:`,
173+
`⛔ **${readOnly.length}** release-owned page(s) ${editable.length ? 'also ' : ''}name something this change touched. These are **read-only**:`,
137174
'',
138175
readOnly.map(row).join('\n'),
139176
'',
@@ -144,9 +181,13 @@ jobs:
144181
);
145182
}
146183
}
184+
body.push(...limitsBlock);
147185
body.push(
148186
'',
149-
'> Advisory only. To re-verify, run the `docs-accuracy-audit` workflow scoped to these files:',
187+
'> Advisory only, and a **precision-first** one (#9192): a page is listed because it names a',
188+
'> symbol, wire route or SDK method this diff touched — not because it mentions a changed',
189+
'> package. Each row says which anchor put it there, so a wrong row is reportable rather than',
190+
'> merely annoying. To re-verify, run the `docs-accuracy-audit` workflow scoped to these files:',
150191
'> `node scripts/docs-audit/affected-docs.mjs origin/' + baseRef + '` → pass the list as `args.docs`.',
151192
);
152193
body = body.join('\n');

scripts/docs-audit/README.md

Lines changed: 120 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ The system has four parts, layered cheapest-and-earliest first:
99

1010
## 1. `affected-docs.mjs` — change → docs mapping (the linchpin)
1111

12-
Maps a set of `packages/**` changes to the hand-written docs that reference the
13-
affected packages, so an audit can be scoped to what actually changed.
12+
Maps a set of `packages/**` changes to the hand-written docs that **name something the
13+
change touched**, so an audit can be scoped to what actually changed.
1414

1515
```bash
1616
# docs affected by changes on this branch vs origin/main
@@ -22,15 +22,114 @@ node scripts/docs-audit/affected-docs.mjs --json origin/main
2222
# every hand-written doc (full audit scope)
2323
node scripts/docs-audit/affected-docs.mjs --all
2424

25-
# pin the change classifiers + package-root derivation (needs no repo state; CI runs this before the mapping)
25+
# pin the classifiers, package-root and anchor derivations (needs no repo state; CI runs this before the mapping)
2626
node scripts/docs-audit/affected-docs.mjs --self-test
2727
```
2828

29-
Heuristic: a doc is *affected* by a changed package `P` if it mentions `P`'s npm
30-
name (`@objectstack/<x>`) or repo path (`P`'s directory, e.g.
31-
`packages/services/service-automation`). Over-inclusion is preferred over misses; the
32-
periodic **full** audit (part 4) is the backstop for docs that describe a package
33-
without naming it.
29+
**Derivation (#9192): a doc is *affected* when it NAMES something the change touched.**
30+
Not when it mentions the changed package — that predicate is a dependency-graph proxy
31+
answering a semantic question, and it was measured wrong in *both* directions on PR #9191
32+
(three read verbs in `@objectstack/metadata-protocol`): 3 pages listed of which 1 was
33+
relevant, while the 2 pages that actually document the changed surface —
34+
`api/client-sdk.mdx` and `kernel/contracts/metadata-service.mdx` — were absent, because
35+
they document it through the **SDK** surface, which does not depend on the implementing
36+
package at all.
37+
38+
Over-inclusion is not free, and that is the correction. A wrong-both-ways advisory trains
39+
its reader to skip it, and then it fails on the PR where it is right — the same bill
40+
exclusion 1 below already paid. The derivation is therefore **precision-first**: a shorter
41+
right list beats a longer noisy one.
42+
43+
Three anchor kinds, each exact:
44+
45+
| anchor | what it is | how it is derived |
46+
|:--|:--|:--|
47+
| `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 |
48+
| `route` | a wire path the change touched | a path literal on a changed line, plus every route whose **registrar handler** references a changed symbol |
49+
| `sdk` | the client method bound to an anchor route | the declared `route``client` rows in the repo's route ledgers |
50+
51+
The `route` and `sdk` hops are what carry the derivation across the surface boundary the
52+
package graph cannot cross: `auditMetaItem` (changed) → `GET /api/v1/meta/:type/:name/audit`
53+
(`rest-server.ts` registrar) → `meta.getAudit` (`rest-route-ledger.ts`) → the token
54+
`api/client-sdk.mdx` actually contains.
55+
56+
**A local variable is not documentable surface.** That one rule is what drops the measured
57+
false positive: `const singular = request.type;` inside a method body is not an anchor, so
58+
`kernel/services-checklist.mdx` — whose only `singular` is a service *slot name* — is no
59+
longer listed. A `const` object **is** a container (its keys are metadata property names,
60+
which docs do name); a function body is not.
61+
62+
### Two guards, and both publish what they removed
63+
64+
The first build of this derivation was, on some PRs, *noisier* than the proxy it replaced
65+
(134 rows where the old tool gave 26). Two guards fixed that, and both run **before** the
66+
route bridge — a name left in the set does not merely add a noisy row, it mints noisy route
67+
and SDK anchors from every registrar handler that mentions it:
68+
69+
1. **Shape** — an anchor must be code-shaped (camelCase / PascalCase / snake_case /
70+
dotted). `label`, `object`, `start`, `locale` and `sections` all arrived as real
71+
declarations and matched 82, 113, 43, 13 and 10 of 178 pages; confining them to code
72+
spans does not help, because those words live in code spans too. Reported as
73+
`weakAnchorsDropped`. The recall cost is a genuinely lowercase export (`parse`, `mask`).
74+
2. **Corpus share** — an anchor matching more than 15% of the corpus is a hub term, not an
75+
identifier. `ObjectQL` is code-shaped, genuinely changed, and named by 59 of 178 pages;
76+
it cannot tell an author which page to re-read. Reported as `overbroadAnchors`, with the
77+
count that condemned it.
78+
79+
Plus a cap on the route bridge itself: a symbol wired into more than 3 routes is a
80+
cross-cutting helper, and "which routes mention this name" then answers *every* route.
81+
Reported as `crossCuttingSymbols`. `SCREAMING_SNAKE` constants are kept out of the bridge
82+
entirely — a data table is consulted by handlers, it is not their implementation.
83+
84+
### What it cannot see is reported, never implied
85+
86+
`anchorlessChanges` lists changed files that yielded no anchor at all; a non-empty value
87+
means the list is incomplete **by a known amount**, and an empty `docs` beside it must
88+
never be read as "no page documents this change". The superseded coarse set is still
89+
computed and emitted as `packageMentionDocs`, labelled — an audit that deliberately wants
90+
the wide net can still ask for it, and keeping it visible is how a reader tells a *narrow*
91+
list from a *blind* one. The PR comment renders all of this in a collapsed section, because
92+
the failure #9192 records was never the tool lying — it was the tool never signalling its
93+
own limits at the point of use.
94+
95+
### Measured, before and after
96+
97+
Ten real PRs, each re-derived at its own merge base with its own docs corpus. `docs` rows:
98+
99+
| PR / commit | old (package-mention) | new (anchor) |
100+
|:--|--:|--:|
101+
| #9191 — the three metadata read verbs (the filing card's specimen) | 4 | **3** |
102+
| `0668f02a6` fix(rest): closed `ErrorCode` union on the error responder | 26 | 14 |
103+
| `75b7c240a` feat(spec): `master_detail` + `controlled_by_parent` | 113 | 32 |
104+
| `07ad42463` fix(cli): `os meta resync` skip-count explanation | 22 | **0** |
105+
| `7a537ce90` feat(spec): strict top-level stack keys | 113 | 13 |
106+
| `445ae4deb` fix(auth): auth emails follow the deployment locale | 13 | 3 |
107+
| `30b1c636a` feat(spec): register 9 REST wire codes | 113 | 4 |
108+
| `650cd3daa` fix(objectql): delete-cascade registry reads | 14 | **0** |
109+
| `3851f87f0` feat(spec,plugin-security): partial field masking | 116 | 19 |
110+
| `d5156b965` refactor(metadata-protocol): drop dead `objects` tolerances | 4 | 4 |
111+
112+
The #9191 row reads 4 where the filing card says "the bot listed three pages": `docs` is
113+
the full set and the comment partitions `content/docs/releases/v9.mdx` into its own
114+
read-only section (#6893), so 3 editable rows + 1 release-owned row = 4.
115+
116+
On #9191 the change is qualitative, not just smaller: all three previously-listed pages
117+
are gone and the two pages the filing card measured as *missing* are back, each with the
118+
anchor that put it there (`getAudit`/`getReferences` for `client-sdk.mdx`, `getHistory`
119+
for `metadata-service.mdx`).
120+
121+
The two zeroes are the honest shape of the trade, not a bug: `07ad42463` derives
122+
`MetaResync` and `resyncSkipExplanationLine`, and no hand-written page names either, so the
123+
run says so and points at the coarse set — where the old tool's 22 rows were every page
124+
mentioning `@objectstack/cli`. A CLI **command name** (`os meta resync`) is exactly the
125+
recall class the shape guard costs us: it is a lowercase word, so it cannot anchor.
126+
127+
**Cost** (the card's open question): the anchor derivation reads the same 178-page corpus
128+
the old one did, plus the 18 route-registrar/ledger sources (~875 KB) and one `git show`
129+
per changed file per side. Measured end-to-end on the ten PRs above, `node affected-docs.mjs`
130+
went from 85-195 ms to 114-582 ms. The heaviest case is the widest diff; every case stays
131+
well under a second, against a job that already spends seconds checking out the repo and
132+
setting up Node. It is the right default for every PR.
34133

35134
**How a changed file maps to its package:** the package root is the **deepest ancestor
36135
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:
88187

89188
The excluded counts are reported in the summary line and as `testFilesSkipped` /
90189
`scriptFilesSkipped` / `devOnlyManifestsSkipped` in `--json`, so the narrowing is never
91-
silent. `--self-test` pins the classifiers *and* the package-root derivation against
92-
paths that must and must not match (`commands/test.ts` is implementation;
190+
silent. `--self-test` pins the classifiers, the package-root derivation *and* the anchor
191+
derivation against inputs that must and must not match (`commands/test.ts` is implementation;
93192
`foo.conformance.test.ts` is not; a container directory must never come out as a package
94193
root; `dependencies` is never dev-only).
95194

@@ -196,9 +295,17 @@ out of an in-memory copy and requires that check to go red.
196295
## 2. CI gate — `.github/workflows/docs-drift-check.yml`
197296

198297
On any PR that touches `packages/**`, runs `affected-docs.mjs` against the base branch
199-
and posts/updates a single advisory PR comment listing the docs that reference the
200-
changed code. **Never fails the build** — it only flags drift at the source, before it
201-
lands on `main`. Reviewers (or an on-demand audit run) decide whether to re-verify.
298+
and posts/updates a single advisory PR comment listing the docs that name something the
299+
change touched — each row carrying **the anchor that put it there**, so a wrong row is
300+
reportable rather than merely annoying. **Never fails the build** — it only flags drift at
301+
the source, before it lands on `main`. Reviewers (or an on-demand audit run) decide whether
302+
to re-verify.
303+
304+
The comment also carries a collapsed **"What this run could not see"** section:
305+
anchorless files, cross-cutting symbols, over-broad anchors, and the coarse
306+
package-mention count. That is the point-of-use half of #9192 — every one of the three
307+
derived-list failures in that shift was caught only because a dev widened the probe past
308+
what the tool offered, never because the tool signalled its own limits where it was read.
202309

203310
### The comment forks release-owned pages into a read-only section (#6893)
204311

0 commit comments

Comments
 (0)