Repository navigation
fix(docs-api): anchor the typedoc exclude to the project root - #1723
Conversation
Closes #1722. `pnpm run docs:api` exited 0, printed "markdown generated at ./docs/api", raised no errors and no warnings — and wrote exactly ONE file instead of 3546. With cleanOutputDir: true, the other 3545 were deleted first. A command that reported success left a mass deletion staged. typedoc matches `exclude` against ABSOLUTE paths, and the pattern was `**/test/**`. Any checkout with a directory segment named `test` anywhere above it therefore excluded its own `src/lib/`, the entry point converted to an empty module, and only the module README survived. That is not an exotic path. `workforge create -t test -n <name>` puts the worktree under a directory named for the branch type, so every worktree opened for test work sat on it. Anchored to `./test/**`, which cannot match an ancestor of the repository. Testing — from a worktree at neurolink-fork/test/anchor-typedoc-excludes, i.e. reproducing the exact condition: before: Ran 1 total deferred tasks There are 1 pages to write. no error/warning summary at all git status --porcelain docs/api -> 3546 files (3545 D, 1 M) after: Ran 768 total deferred tasks There are 3546 pages to write. Found 0 errors and 95 warnings git status --porcelain docs/api -> 0 files Zero drift is also what proves the root `test/` directory is still excluded: if the anchored pattern had stopped matching it, docs/api would have GAINED pages rather than matching release byte for byte. Confirmed through `pnpm run docs:api` itself, not a --exclude flag override. `**/node_modules/**` is deliberately left alone: a nested node_modules is real under pnpm and must stay excluded, and a repository checked out beneath a directory named node_modules is not a case worth trading that for. CI never saw this. GitHub checks out at /home/runner/work/neurolink/neurolink, which has no matching segment, so the docs/api currency gate always computed the right answer and would have failed any PR that committed the deletions. The cost was entirely local and entirely quiet, which is why it is also recorded in CLAUDE.md next to the other "exit 0 is not evidence" incidents. lint 0 errors · prettier clean on both files
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe Typedoc configuration now anchors the test-directory exclusion to the project root. The documentation records the previous path-dependent failure, its observed output, and the corrected pattern. ChangesTypedoc path exclusion fix
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The documentation command will retain API pages in test-named worktrees while continuing to exclude the root test directory. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Fix is correct and well-verified. Approving — one non-blocking MINOR suggestion about a regression guard.
| "exclude": [ | ||
| "**/node_modules/**", | ||
| "**/test/**", | ||
| "./test/**", |
There was a problem hiding this comment.
MINOR — the fix is correct, but this bug class has no automated guard.
**/test/** → ./test/** is the right fix: typedoc matches exclude globs against absolute paths, so the old bare pattern matched any checkout whose absolute path contained a test/ segment and silently excluded all of src/lib/. Anchoring to ./test/** resolves against the project root and only matches the repo's own test/. Verified against the run log in the PR body (3545 files restored) and consistent with the docs/api currency gate in .github/workflows/ci.yml still passing with no committed regeneration.
The gap: CI checks out at /home/runner/work/neurolink/neurolink (no test segment), so no automated check can ever exercise the path that broke. Per the repo's own documented lesson in this PR's CLAUDE.md section — "a command that exits 0 is not evidence it did anything" — this is a silent-failure class. Consider a deterministic guard, e.g. a script/hook that runs pnpm run docs:api from a synthetic checkout path containing a test/ segment and asserts page count > 1, so the regression is caught when reintroduced rather than only by a developer who happens to sit under a test directory. Non-blocking.
There was a problem hiding this comment.
Fixed in #1895: validate:all now runs a guard that rejects an unanchored directory glob in the Typedoc excludes (other than node_modules).
|
APPROVE — the typedoc VerdictThe change addresses issue #1722 precisely. Findings
What I checked and found clean
No CRITICAL/MAJOR issues. Merging is safe. |
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Posting the MINOR regression-guard note as an inline comment; the APPROVE verdict stands from the earlier review.
|
Duplicate summary removed — superseded by the canonical summary above. No separate action required; see the |
|
Now 5/5 green on the same head First attempt, Three data points on the identical commit:
And Filed as #1724, because the failure message is actively misleading: it says "nothing here waits on a network, so this is a hang in the code under test, not a slow upstream", which rules out the cause that actually applies. Offline excludes a slow upstream; it does not exclude a slow runner, and this suite's whole subject is wall-clock timers. |
|
🎉 This PR is included in version 12.14.19 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes the review threads left open on merged PRs against the CI workflows, the repo's gate scripts and a few dev tools. Each change is the smallest one that closes its finding. Behaviour changes are covered by a new suite (test/continuous-test-suite-tooling-scripts.ts, `pnpm run test:tooling-scripts`) and by additions to the provider-structure and provider-descriptors suites; every new test was run red against the unfixed source first. Workflows - ci.yml: persist-credentials: false on the seven checkouts that never push, semantic-release-validation keeps its token (T3790294038, #1335). The permissions comment names that job as the one contents: write exception (T3858986829-a, #1552). The pinned suite counts and the 373-assertion figure are gone (T3869180755-f1, #1580). The new tooling-scripts suite is added to extended-suites; its weight (100) is an estimate, not a CI median. - release.yml: the ffmpeg note no longer says build-check gates this workflow (T3810295618-a, #1360). - single-commit-enforcement.yml: one SKIP_RE shared by both greps, printf instead of echo, and the guidance names the push/pull_request workflows rather than "every workflow" (T3813387872-printf-regex, T3813416696-overstated-guidance, #1364). Config and lint docs - config/models.json: Opus 4.5 uses the real snapshot id 20251101 for anthropic, bedrock and vertex instead of the 20251124 launch date (T3816077440, #1375). provider-structure now checks every Claude id in the file against the model enums. - eslint-rules/index.cjs: header lists e2e-tests-only, no-inline-secret-regex, provider-typed-errors and provider-base-class (T3801758166-1, #1344). Scripts - build-validations.ts: fails when typedoc.json carries an unanchored `**/<dir>/**` exclude, which drops every file under a checkout whose path contains that directory (T4042344752-guard, #1723). - check-banned-deps.ts: scans each file as a whole, so import(), require() and `from` followed by a specifier on the next line are found, and a `//` inside a string no longer hides the rest of the line (T3956062753, #1662). Files in the repo root and .mts/.cts are scanned too (T3956062775, #1662). - check-shipped-types.ts: the declarations under dist/ must equal the set the source tree emits, so a partial or stale build above the 100-file floor fails (PF-T3927528338, #1627). A wildcard export is matched against the whole pattern, including a `*` in a directory component (T3931686738-wildcard-match, #1632). - codex-replay-listener.ts: the tool-call script names `replay_tool` instead of `exec`, which Codex declares as a custom tool and which raised a Fatal "incompatible payload" error (F1-T4087477953-custom-tool-shape, #1783); reproduced and cleared against codex-cli 0.160.0. --requests counts served /responses turns, so a 404 probe cannot shut the listener down first (F2-T4087477985-requests-limit-counts-404s, #1783). - commit-validation.ts: execFileSync("git", [...]) instead of a shell string; behaviour unchanged (T3838161513-b, #1499). - migration-symbol-diff.mjs: this/super-rooted paths keep their full name, and tagged templates, obj["name"](), super() and import() are tracked; the header says it follows calls (T3835058026-residual, PF-T3833252257, #1448). - tools/automation/environmentManager.ts: credential-free providers count as configured only when the .env sets one of their variables, the score no longer divides by the size of the catalog, and the report lists the configured providers plus one count instead of every missing one (T3792794348, T3792807279, #1337). Not done, on purpose - The skip-checks trailer in the single-commit grep (optional in the finding). - Checkouts in workflows other than ci.yml: the findings named only ci.yml. - migration-symbol-diff still does not record a function passed by reference (`items.forEach(handler)`); the header now says so. Pre-existing, not touched: test:dynamic fails its five live cases without provider credentials, identically with config/models.json reverted.
2 participants
Closes #1722.
pnpm run docs:apiexited 0, printedmarkdown generated at ./docs/api, raised no errors and no warnings — and wrote exactly one file instead of 3546.cleanOutputDir: truehad already deleted the other 3545. A command that reported success left a mass deletion staged in the working tree.Cause
typedoc matches
excludeagainst absolute paths, and the pattern was**/test/**. Any checkout with a directory segment namedtestanywhere above it therefore excluded its ownsrc/lib/; the entry point converted to an empty module and only the module README survived.That is not an exotic path —
workforge create -t test -n <name>places the worktree under a directory named for the branch type, so every worktree opened for test work sat on this."exclude": [ "**/node_modules/**", - "**/test/**", + "./test/**", "**/*.test.ts", "**/*.spec.ts" ],./test/**cannot match an ancestor of the repository.Testing — from the failing condition, not next to it
Built in a worktree at
neurolink-fork/test/anchor-typedoc-excludes, so the bug reproduces before the change and is gone after it:deferred taskspages to writegit status --porcelain docs/apiD, 1MFound 0 errors and 95 warningsZero drift is also what proves the root
test/directory is still excluded: if the anchored pattern had stopped matching it,docs/apiwould have gained pages rather than matching release byte for byte. Confirmed throughpnpm run docs:apiitself, not a--excludeflag override.lint0 errors ·prettier --checkclean on both files.Deliberately not touched
**/node_modules/**keeps its pattern. A nestednode_modulesis real under pnpm and must stay excluded; a repository checked out beneath a directory namednode_modulesis not a case worth trading that for.Why CI never caught it
GitHub checks out at
/home/runner/work/neurolink/neurolink, which has no matching segment — so thedocs/apicurrency gate always computed the right answer, and would have failed any PR that committed the deletions. The cost was entirely local and entirely quiet: two worktrees at the same commit, same lockfile, identicaltsc --listFilesoutput of 4847 files, disagreeing by 3545 files, with neither a reinstall nor a rebuild changing anything.That is why the incident is also recorded in
CLAUDE.md, in the reproducible-generator section, right after "a command that exits 0 is not evidence it did anything" — this is the sharper version, where the artifact was written and is still wrong.Summary by CodeRabbit
Bug Fixes
testdirectory.Documentation