Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .changeset/docs-drift-nested-package-roots.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
---

Tooling only — no package changes, nothing to release.

The docs-drift mapper (`scripts/docs-audit/affected-docs.mjs`) derives a changed
file's package root from the filesystem — the deepest ancestor directory with a
`package.json` — instead of a regex that special-cased only `packages/plugins/*`.
Under the old derivation the 30 packages nested under the other six container
directories (`services/`, `connectors/`, `apps/`, `qa/`, `triggers/`, `adapters/`)
collapsed into their container, whose missing `package.json` left the npm name
unresolved and the npm-name matching arm dead: any doc that names
`@objectstack/service-automation` but never the repo path was a guaranteed miss
(#4162), the one direction this tool promises to avoid. No hardcoded container
list replaces the special case — that would fail the same way again on container
number eight (#3786's pattern).

- Verified against real history: on #4161's commit the drift comment attribution
changes from the directory name `packages/services` to
`@objectstack/service-automation`. A synthetic service-automation-only change
goes from 6 docs — all belonging to *other* services, matched via the coarse
`packages/services` path token, with `automation/flows.mdx` absent — to the 4
right ones, `flows.mdx` first among them.
- Second arm (from #4162's comment thread): `<packageRoot>/scripts/**` is
build/verification tooling and no longer counts as an implementation change
(#4183 flagged 106 docs for a diff whose only code change was a new check
script). Kept narrow: `package.json` and `src/scripts/**` stay counted.
Publication check done — no package ships runtime code from `scripts/`; three
plugins publish a lone `i18n-extract.config.ts` only for lack of a `files`
field. A scripts-only change now maps to 0 docs (was 106); the exclusion is
reported in the summary and as `scriptFilesSkipped` in `--json`, never silent.
- `--self-test` now pins the package-root derivation and the tooling-script
classifier too (32 cases, hermetic via an injected fake tree), closing the
guard-of-the-guard hole: the original self-test pinned only the test-file
matcher, so this bug was invisible to it. Includes the two invariants from the
issue: a container directory must never come out as a package root, and
`packages/x/package.json` is implementation while `packages/x/scripts/y.ts`
is not.
9 changes: 5 additions & 4 deletions .github/workflows/docs-drift-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ 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.
# The mapper excludes test files and package tooling scripts (neither can make
# an implementation doc stale) and derives package roots from the filesystem.
# Self-test first, so a regression that widened an exclusion into dropping real
# implementation changes — or collapsed nested packages into their container
# again (#4162) — fails loudly here instead of quietly skewing this comment.
- name: Self-test the change → docs mapper
run: node scripts/docs-audit/affected-docs.mjs --self-test

Expand Down
50 changes: 36 additions & 14 deletions scripts/docs-audit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,46 @@ 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)
# pin the change classifiers + package-root derivation (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).
name (`@objectstack/<x>`) 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.

**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
list of container directories. (The mapper once special-cased only
`packages/plugins/*`; the 30 packages nested under the other six containers collapsed
into `packages/services` et al., whose missing `package.json` disabled the npm-name
matching arm entirely, so a doc naming `@objectstack/service-automation` but not the
repo path was a guaranteed miss — #4162.) A deleted package falls back to the coarse
`packages/<x>` token, which still substring-matches any doc naming the deleted path.

**Two exclusions:** change classes that cannot make an implementation-accuracy doc
stale are dropped before the changed-package roots are derived:

1. **Test files** (`*.test.*` / `*.spec.*` at any depth, plus `__tests__` /
`__mocks__` / `__fixtures__`): a test observes behaviour rather than defining it —
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.
2. **Package tooling scripts** (`<packageRoot>/scripts/**`): build/verification
tooling, not the runtime behaviour docs describe (#4183 flagged 106 docs for a diff
whose only code change was a new check script). Narrow on purpose: `src/scripts/**`
is runtime code and stays counted, and so does `package.json` — exports/deps
changes ARE implementation. No package publishes runtime code from `scripts/`
(checked against every `files` allowlist; three plugins ship a lone
`i18n-extract.config.ts` only for lack of a `files` field).

The excluded counts are reported in the summary line and as `testFilesSkipped` /
`scriptFilesSkipped` 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; `foo.conformance.test.ts` is not; a
container directory must never come out as a package root).

**And one deliberate non-exclusion:** `packages/*/CHANGELOG.md` stays counted, even though
release notes define behaviour no more than a test does. Extending the exclusion there
Expand Down
Loading
Loading