fix(tooling): docs-drift stops flagging docs for tests-only changes (#4091)

`affected-docs.mjs` derived changed-package roots from every file under `packages/`,
tests included. A test observes behaviour rather than defining it, so it cannot make an
implementation-accuracy doc stale — yet every tests-only PR lit up its packages' whole
doc set. Three in a row did it (#4064, #4078 and one before), each flagging 6
packages/services docs for a diff with no production code.

This is the one place the mapper's deliberate over-inclusion actively hurt: a category
that is ALWAYS false teaches the reader to skip the comment, and then it fails on the PR
where it is right.

- Test files are dropped before the roots are derived: *.test.* / *.spec.* at any depth
  (covering .integration.test.ts and .conformance.test.ts) plus __tests__ / __mocks__ /
  __fixtures__.
- The narrowing is NOT silent — the excluded count rides the summary line and
  `testFilesSkipped` in --json.
- `--self-test` pins the matcher (the check-error-code-casing / check-route-envelope
  convention), run by the workflow before it computes the mapping.
- The workflow also triggers on the mapper's own paths, so editing it runs its own guard.
  CI surfaced that gap: on the first push the drift job never ran at all, which meant the
  self-test had been added but was structurally unreachable by the change most likely to
  break it.

Verified against real history: 46e86ba (tests only) → 0 docs, was 6; 4965bfa
(engine.ts + formula) → the same 11 docs as before; --all unchanged at 178. Swapping the
anchored match for a naive path.includes('test') fails 7 self-test cases, including
control-flow.zod.ts and latest.ts being swallowed as tests.

Author: os-zhuang

.changeset/docs-drift-skip-test-files.md

@@ -0,0 +1,28 @@
+---
+---
+
+Tooling only — no package changes, nothing to release.
+
+The docs-drift mapper (`scripts/docs-audit/affected-docs.mjs`) no longer treats a
+test-file change as a reason to flag docs. A test observes behaviour rather than
+defining it, so it cannot make an implementation-accuracy doc stale — yet every
+tests-only PR lit up its packages' whole doc set. Three in a row (#4064, #4078, and one
+before) each flagged 6 `packages/services` docs for a diff containing no production
+code.
+
+That is the one place the mapper's deliberate over-inclusion actively hurt: a comment a
+reader learns to skip stops doing its job on the PR where it is right.
+
+- Test files are dropped before the changed-package roots are derived: `*.test.*` /
+  `*.spec.*` at any depth, and anything under `__tests__` / `__mocks__` / `__fixtures__`.
+- The narrowing is **not silent** — the excluded count appears in the summary line and
+  as `testFilesSkipped` in `--json`.
+- A `--self-test` flag pins the matcher, following the convention of
+  `check-error-code-casing` / `check-route-envelope`, and the drift workflow now runs it
+  before computing the mapping. Verified to catch the dangerous direction: replacing the
+  anchored match with a naive substring check fails 7 cases, including
+  `control-flow.zod.ts` and `latest.ts` being swallowed as "tests".
+
+Verified against real history: the tests-only commit `46e86bad` (#4078) now maps to 0
+docs where it previously reported 6, while the implementation commit `4965bfac` (#4059)
+still maps to the same 11 docs it did before.

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

@@ -11,6 +11,13 @@ on:
     types: [opened, synchronize, reopened]
     paths:
       - 'packages/**'
+      # The mapper and this workflow, so a change to the guard runs the guard. Without
+      # these, editing `affected-docs.mjs` was the one change its own self-test could
+      # never see — a `packages/**`-only trigger means the tool is unguarded exactly
+      # when it is being modified. A PR that touches only these gets the benign
+      # "0 changed package(s) ✅" comment, which is the correct answer.
+      - 'scripts/docs-audit/**'
+      - '.github/workflows/docs-drift-check.yml'
 
 permissions:
   contents: read
@@ -34,6 +41,13 @@ jobs:
       - name: Fetch base branch
         run: git fetch --no-tags origin "${{ github.base_ref }}"
 
+      # The mapper excludes test files (a test cannot make an implementation doc
+      # stale). Self-test first, so a regression that widened the exclusion into
+      # dropping real implementation changes fails loudly here instead of turning
+      # this whole comment quietly empty.
+      - name: Self-test the change → docs mapper
+        run: node scripts/docs-audit/affected-docs.mjs --self-test
+
       - name: Compute affected docs
         id: affected
         run: |

scripts/docs-audit/README.md

@@ -21,13 +21,26 @@ 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
+
+# check the test-file matcher (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/<x>`) or repo path (`packages/<x>`). Over-inclusion is preferred
 over misses; the periodic **full** audit (part 4) is the backstop for docs that
 describe a package without naming it.
 
+**One exclusion:** changes to **test files** are dropped before the changed-package
+roots are derived. A test observes behaviour rather than defining it, so it cannot make
+an implementation-accuracy doc stale — yet counting them made every tests-only PR light
+up its packages' whole doc set, a class of finding that is always false. That is the one
+place over-inclusion actively hurt: a comment a reader learns to skip stops working on
+the PR where it is right. The count of excluded files is reported in the summary and as
+`testFilesSkipped` in `--json`, so the narrowing is never silent, and `--self-test`
+pins the matcher against paths that must and must not match (`commands/test.ts` is
+implementation; `foo.conformance.test.ts` is not).
+
 ## 2. CI gate — `.github/workflows/docs-drift-check.yml`
 
 On any PR that touches `packages/**`, runs `affected-docs.mjs` against the base branch

scripts/docs-audit/affected-docs.mjs

@@ -7,6 +7,7 @@
 //   node scripts/docs-audit/affected-docs.mjs [sinceRef]   # docs affected by changes since <sinceRef> (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    # check the test-file matcher (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).
@@ -15,6 +16,14 @@
 // npm name (`@objectstack/<x>`) or its repo path (`packages/<x>`). Over-inclusion is
 // intentionally preferred over misses; the periodic FULL audit is the backstop for
 // docs that describe a package without naming it.
+//
+// One exclusion, though: a change to a TEST file cannot make an implementation-accuracy
+// doc stale, because tests do not define behaviour — they observe it. Counting them made
+// every tests-only PR light up its packages' whole doc set (three in a row on #4064 /
+// #4078 / one before), which is a class of finding that is always false. A reader who
+// learns the comment is usually noise stops reading it, and then it fails to do its job
+// on the PR where it is right. So test files are dropped before deriving the changed
+// package roots; everything else stays deliberately over-inclusive.
 
 import { execSync } from 'node:child_process';
 import { readFileSync, readdirSync, existsSync, statSync } from 'node:fs';
@@ -26,6 +35,12 @@ const asJson = args.includes('--json');
 const all = args.includes('--all');
 const sinceRef = args.find((a) => !a.startsWith('--')) || 'origin/main';
 
+// Short-circuit before any git work — the self-test needs no repo state.
+if (args.includes('--self-test')) {
+  selfTest();
+  process.exit(0);
+}
+
 function sh(cmd) {
   return execSync(cmd, { cwd: repoRoot, stdio: ['ignore', 'pipe', 'ignore'] }).toString();
 }
@@ -61,9 +76,63 @@ try {
   changedFiles = sh(`git diff --name-only ${sinceRef} -- packages/`).split('\n').filter(Boolean);
 }
 
+/**
+ * A test file — it observes behaviour rather than defining it, so changing one cannot
+ * make an implementation-accuracy doc stale. Covers the repo's conventions: `*.test.*`
+ * / `*.spec.*` at any depth (including `.integration.test.ts` and `.conformance.test.ts`)
+ * plus anything under a `__tests__` / `__mocks__` / `__fixtures__` directory.
+ *
+ * Verify with `--self-test`.
+ */
+function isTestFile(path) {
+  return /(^|\/)__(tests|mocks|fixtures)__\//.test(path)
+    || /(^|\/)[^/]+\.(test|spec)\.[^/]+$/.test(path);
+}
+
+/**
+ * Check the test-file matcher against known-good and known-bad paths, so the
+ * exclusion cannot silently widen into dropping real implementation changes — the
+ * one way this optimisation could turn into a miss.
+ */
+function selfTest() {
+  const cases = [
+    // [path, isTest, label]
+    ['packages/services/service-automation/src/builtin/config-schemas.test.ts', true, 'plain .test.ts'],
+    ['packages/rest/src/package-envelope.conformance.test.ts', true, 'compound .conformance.test.ts'],
+    ['packages/services/service-automation/src/runas-grant-resolution.integration.test.ts', true, '.integration.test.ts'],
+    ['packages/spec/src/data/object.spec.ts', true, '.spec.ts'],
+    ['packages/foo/src/__tests__/helper.ts', true, 'helper inside __tests__'],
+    ['packages/foo/src/__mocks__/driver.ts', true, '__mocks__'],
+    ['packages/foo/src/__fixtures__/stack.json', true, '__fixtures__'],
+
+    ['packages/services/service-automation/src/engine.ts', false, 'implementation'],
+    ['packages/spec/src/automation/control-flow.zod.ts', false, 'a zod schema'],
+    ['packages/formula/src/validate.ts', false, 'implementation with a test-ish name'],
+    ['packages/cli/src/commands/test.ts', false, 'a command NAMED test is not a test file'],
+    ['packages/qa/src/testing.ts', false, 'testing.ts is implementation'],
+    ['packages/spec/src/latest.ts', false, 'no false positive on a bare name'],
+    ['packages/foo/src/tests-helper.ts', false, 'tests-helper is not __tests__'],
+  ];
+  let failed = 0;
+  for (const [path, want, label] of cases) {
+    const got = isTestFile(path);
+    if (got !== want) {
+      console.error(`  ✗ self-test "${label}": ${path} → expected isTestFile=${want}, got ${got}`);
+      failed++;
+    }
+  }
+  if (failed) {
+    console.error(`\n✗ affected-docs self-test failed (${failed} case(s)).`);
+    process.exit(1);
+  }
+  console.log(`✓ affected-docs self-test: ${cases.length} cases pass.`);
+}
+
+
 // collect package roots: packages/<x> and packages/plugins/<x>
 const pkgRoots = new Set();
-for (const f of changedFiles) {
+const implementationChanges = changedFiles.filter((f) => !isTestFile(f));
+for (const f of implementationChanges) {
   let m = f.match(/^(packages\/plugins\/[^/]+)\//) || f.match(/^(packages\/[^/]+)\//);
   if (m) pkgRoots.add(m[1]);
 }
@@ -91,11 +160,24 @@ for (const doc of handwritten) {
   if (hits.length) affected.push({ doc, via: [...new Set(hits)] });
 }
 
-emit(affected.map((a) => a.doc), changedPackages, `${affected.length} docs affected by ${changedPackages.length} changed package(s) since ${sinceRef}`, affected);
+// Report what was excluded rather than dropping it silently — a tool that quietly
+// narrows its own scope reads as "nothing to see here" when it means "I did not look".
+const testFilesSkipped = changedFiles.length - implementationChanges.length;
+const skipNote = testFilesSkipped > 0
+  ? ` (${testFilesSkipped} test file(s) excluded — tests cannot make an implementation doc stale)`
+  : '';
+
+emit(
+  affected.map((a) => a.doc),
+  changedPackages,
+  `${affected.length} docs affected by ${changedPackages.length} changed package(s) since ${sinceRef}${skipNote}`,
+  affected,
+  testFilesSkipped,
+);
 
-function emit(docList, changedPackages, summary, detail) {
+function emit(docList, changedPackages, summary, detail, testFilesSkipped = 0) {
   if (asJson) {
-    process.stdout.write(JSON.stringify({ summary, sinceRef: all ? null : sinceRef, changedPackages, docs: docList, detail: detail || null }, null, 2) + '\n');
+    process.stdout.write(JSON.stringify({ summary, sinceRef: all ? null : sinceRef, changedPackages, docs: docList, detail: detail || null, testFilesSkipped }, null, 2) + '\n');
   } else {
     process.stderr.write(`# ${summary}\n`);
     process.stdout.write(docList.join('\n') + (docList.length ? '\n' : ''));