Skip to content

fix(docs-audit): exclude benchmarks and non-underscore test/ dirs from isTestFile - #12965

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-11857-registrar-istestfile
Aug 28, 2026
Merged

fix(docs-audit): exclude benchmarks and non-underscore test/ dirs from isTestFile#12965
os-elon merged 1 commit into
mainfrom
claude/issue-11857-registrar-istestfile

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Part of #11857 — this is the card's cheaper half (B) only. #11857 stays open: its half A
(the evidence-based registrar route) is a contract question and is not implemented here.

Scope

The card names one self-contained defect: "isTestFile excludes neither test/fixtures/*.ts
nor *.bench.ts."
That is what this PR fixes, as a predicate over the convention rather
than as the two path literals that exposed it.

Not in this PR, deliberately: admitting any non-test packages/**/*.ts whose masked
source yields a route tail (half A). That redefines "registrar" to include packages/spec Zod
contract declarations, and it moves a published figure other records cite. Every number half A
quotes — 45 to 59 client-bound rows, the storage ledger closing, 12 to 20 registrar files,
43 to 82 tails — belongs to half A and none of it is claimed here.

The control does NOT reproduce, and that is the first thing to read

The card's recipe requires reproducing 12 registrar files / 43 route tails on the unmodified
tree before any delta means anything. On origin/main at b1a987e4a it does not:

control readingcard (a6eca9223)today (b1a987e4a)
registrar files scanned1214
route tails produced4344
client-bound reachable4547

Cause, measured not guessed. Two files landed since a6eca9223 whose basenames contain
routes, so REGISTRAR_FILE_RE admits them. Both are confirmed absent at the card's ref by
git cat-file -e:

  • packages/spec/src/migrations/entries/retired-keys/18.kernel__Manifest__contributes.routes.ts
  • packages/spec/src/migrations/entries/semantic/18.plugin-manifest-contributes-routes-retired.ts

They are ADR-0049 retirement entries, not registration sites. The exact split of the +1 tail and
+2 reachable between them was not measured — isolating it needs a probe outside this half's
dispatched scope. Filed as a separate finding issue rather than pursued here.

What the predicate now excludes

Two arms, both written over the convention:

  1. .bench. joins .test. / .spec. in the infix arm.
  2. Non-underscore test/ and tests/ directories join __tests__ / __mocks__ /
    __fixtures__ — the same convention, spelled without underscores. Six packages use it, and
    test/fixtures/ falls out of it by shape rather than by being named.

Swept across the tree with the predicate sliced verbatim out of the working-tree source (not
re-implemented), against 4907 packages/**/*.ts files:

count
excluded by the old predicate2844
excluded by the new predicate2872
newly excluded28
newly admitted (must be zero)0

The 28 are 2 benchmarks and 26 files under a non-underscore test/ directory — 13 of them
packages/qa/dogfood/test/fixtures/*.ts, which is the card's first instance, reached by shape.

Both controls fired in the same run: the slice agrees with a known test file and a known
implementation file, and a deliberately over-wide stand-in predicate newly excludes 1906 —
so the diff arithmetic is demonstrably able to return a large non-zero.

Did the live numbers move? Partly — and the published figure did not

The dispatching assumption was that this half is verdict-neutral on today's tree. That is
right for the convention route and wrong for the ceiling.
Full --bridge-coverage diff,
before against after, is one line:

  • registrar files 14, route tails 44, ledger files 11, rows 290 of 290, client-bound 219 of 219,
    reachable 47, unreachable 172, and the whole per-ledger table — byte-identical.
  • cause breakdown (remediable 14 / no in-repo registrar 56 / undecided 102) — byte-identical.
  • the ceiling moved: 82-tail ceiling becomes 79.

The 3 ceiling tails removed are attributable in full, and they are exactly the two files the
card named as false positives:

filetails it was contributing to the ceiling
packages/qa/dogfood/test/fixtures/endpoint-policy-fixture.ts2 (/api/v1/apps/e8policy/public-notes, /api/v1/apps/e8policy/private-notes)
packages/spec/src/benchmark.bench.ts1 (/api/benchmark)

No other file among the 28 declares a path:. This is a correctness gain, not a loss: the
ceiling is meant to measure the headroom a better recognizer could reach, and a benchmark and a
test fixture were inflating it by 3 with paths no registrar serves.

The published 45 is untouched by this PR. It reads 47 both before and after — this change
moves it by zero. Note for #10534 and #9572 separately that the figure they quote drifted 45 to
47 on main on its own, through the two migration entries above and not through anything here.

packages/spec/src/conversions/registry.ts — the card's third false positive — is not
excluded by this predicate, which is the correct outcome: its cause is different (a
connector-action input inside an automation fixture, not a route), no isTestFile change
reaches it, and it is not admitted today anyway.

Reverse verification

Both arms were ablated from the committed state, one at a time, each confirming the mutation
landed on disk by marker count before reading any result, each restored under a
trap ... EXIT INT TERM with restoration proved by git diff HEAD empty plus a blob-hash match
against HEAD (81bb2690a41c3798650213841db0e196e5a5b8c3), never by an exit code.

ablationself-test
bench arm reverted — the bench alternative dropped from the infix armRED, 4 named cases
the test/ + tests/ directory arm deleted outrightRED, 8 named cases
restoredGREEN, 474 cases

The load-bearing red is this one, which is the production failure mode itself:

✗ self-test "...file matching REGISTRAR_FILE_RE reaches the registrar list":
packages/foo/test/fixtures/stub-route.ts → expected walkSourceFiles=false, got true

The two walk fixtures are non-vacuous by construction, in the same way the existing #11866
block documents: neither carries a .test. / .spec. infix, so under the previous predicate
both were walked, and stub-route.ts then matched REGISTRAR_FILE_RE and let a test fixture
contribute production route tails.

Over-widening is pinned too

An exclusion that silently widens drops real implementation changes — the failure selfTest's
own docblock names first, and isTestFile feeds the PHASE-1 change classifier as well as the
walk. So the classes deliberately left admitted are pinned admitted: *.testkit.ts (8 files,
shipped harness code, the same class as packages/qa/src/testing.ts which was already pinned as
implementation), *.fixture.ts / *.fixtures.ts, *.pin.ts, the testkit/ and testing/
directories, and a bare fixtures/ or mocks/ directory outside test/ (zero population
today, so admitting it would be speculative reach). The reasoning is recorded in the docblock so
a later reader sees a decision rather than an oversight.

Changeset

Derived, not inherited: the diff is 1 file, scripts/docs-audit/affected-docs.mjs, with zero
packages/** paths and no published-package change, so this PR releases nothing. Route taken is
the skip-changeset label, which pr-automation.yml reads at lines 294 and 589 — not an
empty-frontmatter changeset, which check-empty-changeset.mjs rejects for new files.

Gates — all run at d3de352724, exit codes captured before any pipe

Derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
(no STALE TREE; provenance line confirms commit d3de352724). 15 path-matched plus 2
convention-triggered for a gate-script edit; all 17 exit 0, plus this script's own --self-test.

0 pnpm check:agent-test-spelling 0 node scripts/check-ci-filter-parity.mjs
0 pnpm check:bash32-floor 0 node scripts/check-cross-package-test-inputs.mjs
0 pnpm check:cli-command-ids 0 node scripts/check-self-test-wired.mjs
0 pnpm check:cross-package-test-inputs 0 node scripts/docs-audit/check-affected-docs.mjs
0 pnpm check:docs-audit-scope 0 node scripts/docs-audit/check-drift-comment.mjs
0 pnpm check:entry-guard 0 node scripts/pm/bare-root-worklist.mjs --self-test
0 pnpm check:parse-guard 0 pnpm check:pm-dispatch-gates
0 pnpm check:pm-governed-merges 0 node scripts/docs-audit/affected-docs.mjs --self-test
0 pnpm check:pnpm-filter-targets 0 node scripts/check-nul-bytes.mjs
0 pnpm check:watch-hint-literal

Verdict lines, quoted from the gates themselves rather than from $?:

  • ✓ affected-docs self-test: 474 cases pass.
  • ✓ check-self-test-wired: every one of the 143 script(s) CI runs that ship a --self-test has that self-test run by CI.
  • ✓ check-drift-comment: 56 cases pass across 5 fixture diff(s).
  • check-nul-bytes: OK (scanned 7188 text file(s) ... no raw ASCII control bytes).
  • check-affected-docs prints the 79-tail ceiling line, confirming CI sees the same reading.

This script has no separate vitest suite — --self-testis its suite, and
check-self-test-wired.mjs is the gate that enforces that wiring. Both are green above.

Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw

Generated by Claude Code


Generated by Claude Code

…m isTestFile
`isTestFile` covered `__tests__/` / `__mocks__/` / `__fixtures__/` and the
`.test.` / `.spec.` infixes, but neither the non-underscore `test/` / `tests/`
spelling of the same directory convention nor `*.bench.*`. Six packages use the
non-underscore spelling, so `test/fixtures/*.ts` and benchmarks were reaching
both call sites: the PHASE-1 change classifier, which treated them as
implementation changes, and `walkSourceFiles`, which fed them into the
`--bridge-coverage` ceiling population.
Fixed as a predicate over the convention, not as a path list: two arms, plus
self-test cases pinning the classes deliberately left admitted (`*.testkit.ts`,
`*.fixture(s).ts`, `*.pin.ts`, `testkit/`) so the exclusion cannot widen by
drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPrUz21stTFhJRUirdc4yw
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fc8a33935ce9abd9f9519af4327a78fac696f853packageMentionDocs.

@os-elonos-elon added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 28, 2026 — with Claude
@os-elon
os-elon marked this pull request as ready for review August 28, 2026 11:27
@os-elon
os-elon enabled auto-merge August 28, 2026 11:28
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — armed (draft:false, then auto-merge). PM seat domain:devx @ objectstack (#6023), session session_01CPrUz21stTFhJRUirdc4yw, round R25.

Entry qualification: every check. 33 runs at d3de352724 — 24 success, 9 skipped, 0 failure, 0 pending.

Scope held exactly.Part of #11857, ⛔ never Fixes — half A (admitting any packages/**/*.ts whose masked source yields a route tail, which redefines "registrar" to include Zod contract declarations) is a contract question and stays open for a ruling. Part-of PR must not also close its card passed, which is the mechanical confirmation.

Both of my Zone 2 assumptions were falsified, and the second one inverts the result

1. ⛔ The card's control does not reproduce, and that is a finding, not a nuisance. Today's origin/main reads 14 registrar files / 44 tails / 47 reachable, against the card's 12 / 43 / 45. The dev did not shrug and proceed — it found the cause: REGISTRAR_FILE_RE matches any basename containingroute, and two ADR-0049 retirement entries whose basenames carry "routes" landed after the card's ref a6eca9223. Both confirmed absent at that ref by git cat-file -e, so the attribution is measured, not inferred.

The published 45 that #10534 and #9572 both quote has already drifted to 47 on main, with nothing recording it. That is precisely the "moves a published figure" hazard the card raised for half A — and it has already happened, by a different mechanism, unnoticed. Filed as #12966.

2. "Verdict-neutral" was right for the convention route and wrong for the ceiling — and the difference is a correctness gain. The full --bridge-coverage output is byte-identical before and after except one line: the ceiling falls 82 → 79. Those 3 tails are attributable in full to exactly the two files the card named as false positives. So a benchmark and a test fixture had been inflating the headroom measure with paths no registrar serves. I briefed this as "probably a no-op whose value is that half A becomes cleanly decidable later" — measured, it is a small correctness fix in its own right.

The fix is over the convention, not over the two paths that exposed it

.bench. joins .test./.spec. in the infix arm, and non-underscore test//tests/ directories join __tests__/__mocks__/__fixtures__ — so test/fixtures/ is reached by shape. ⛔ The hardcoded-pair fix I refused in Zone 1 was refused for a reason and the dev did not take it.

The population sweep carries both controls, which is what makes its numbers readings: the predicate was sliced verbatim out of the shipped source rather than re-implemented; a positive control agrees with a known test file and a known implementation file; and a deliberately over-wide stand-in predicate newly excludes 1906, proving the diff arithmetic can return a large non-zero. Result: old excludes 2844, new excludes 2872, newly excluded 28, newly admitted 0. Only 2 of the 28 declare a path, and they are exactly the 3 tails the ceiling lost.

Ablation, one arm at a time, from the committed state, each mutation proved on disk by marker count before any result was read and each restore proved by state (git diff HEAD empty and blob hash equal to HEAD:PATH), never by an exit code: bench arm reverted → 4 named cases red; test/tests dir arm deleted → 8 named cases red, including the production failure mode itself — packages/foo/test/fixtures/stub-route.ts → expected walkSourceFiles=false, got true. Restored: ✓ affected-docs self-test: 474 cases pass.

packages/spec/src/conversions/registry.ts is not excluded by this predicate — correct, and correctly noticed rather than claimed as a win: it is a different cause (a connector-action input, not a route) and is not admitted today anyway.

All 18 derived gates green (15 path-matched + 2 convention-triggered + check-nul-bytes), exit codes captured before any pipe. The gate-script-edit obligation is satisfied by --self-testbeing this script's suite — git grep finds no separate vitest file — with check-self-test-wired.mjs green as the gate that enforces that wiring.

⚠️ Carried, now 31 for 31: arming echo reports method: MERGE while SQUASH was requested. Cosmetic.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-elon@claude