Adopt the canonical pm-ops docstring gate and document the remaining surface - #60
Merged
Conversation
pm-context had no docstring gate, so six exported declarations carried no documentation and nothing would have caught a seventh. Wires in the analyzer published as `pm-ops/docstrings` through a thin launcher — the analyzer is imported, never vendored — pinned exactly at 2026.8.8, as a required CI step and part of `release:check`. Documented `CommandError` and its `exitCode`, the `PmItem` and `ContextPackOptions` interfaces, and the `MARKDOWN_SECTIONS` / `AGENT_SECTIONS` constants. Writing the last two required reading the renderers rather than the constant lists, and the difference matters: `neighborhood` and `neighbors` look like two spellings of one section, but the markdown renderer matches each exactly and they emit different content — the relationship edges and the neighbouring items. Only the agent renderer takes aliases, via a second `include` parameter, where `actions`/`nextactions` select `next-actions` and `activity` selects `recent`. Added `scripts/docstring-gate.ts` to `coverageGate.sources` so the launcher is held to the same floor as the rest of the package instead of sitting outside the gate. Coverage stays at 100/100/100 with it included. Tracked as pm-context-ylx4.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai full review |
Summary by CodeRabbit
WalkthroughAdded a ChangesDocstring coverage gate
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant npm as npm run docstring
participant Gate as scripts/docstring-gate.ts
participant Analyzer as analyzeDocstringCoverage
CI->>npm: Run the docstring check
npm->>Gate: Invoke the launcher
Gate->>Analyzer: Analyze the repository root
Analyzer-->>Gate: Return violations or success
Gate-->>CI: Write output and set process.exitCode
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
|
Reviewer's GuideAdds a canonical docstring gate launcher based on pm-ops/docstrings, wires it into CI and release checks, and documents previously undocumented public surface (errors, interfaces, and section constants) while providing behavioral tests for the gate. Sequence diagram for the new docstring gate invocation in CIsequenceDiagram
actor CI
participant npm_script_docstring as npm_run_docstring
participant node_process as node
participant docstring_gate_module as scripts_docstring_gate_ts
participant pm_ops_docstrings as pm_ops_docstrings
CI->>npm_script_docstring: run docstring
npm_script_docstring->>node_process: node scripts/docstring-gate.ts
node_process->>docstring_gate_module: isMainInvocation(process.argv, import.meta.url)
docstring_gate_module-->>node_process: true
node_process->>docstring_gate_module: main(process.argv.slice(2))
docstring_gate_module->>docstring_gate_module: runGate(root)
docstring_gate_module->>pm_ops_docstrings: analyzeDocstringCoverage({ root })
pm_ops_docstrings-->>docstring_gate_module: report
docstring_gate_module-->>node_process: GateResult
node_process->>CI: exitCode / stdout / stderr
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Greptile SummaryThe PR adopts the canonical
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/docstring-gate.ts | Adds a thin CLI launcher that invokes the shared analyzer, formats violations, and fails closed on invalid invocation state. |
| test/docstring-gate.test.ts | Covers successful and failing scans, empty roots, stream and exit-code behavior, direct invocation, and symlink resolution. |
| package.json | Pins the analyzer dependency and integrates the gate into release and coverage checks. |
| .github/workflows/ci.yml | Runs the docstring gate as a required CI step after type checking. |
| index.ts | Adds documentation for the previously undocumented exported API surface. |
Reviews (2): Last reviewed commit: "fix(gate): propagate an unresolvable ent..." | Re-trigger Greptile
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/docstring-gate.ts`:
- Around line 128-135: The isMainInvocation function currently treats
realpathSync resolution failures as a non-main invocation, allowing the gate to
exit successfully. Make resolution failures propagate or produce a distinct
failure state that the top-level selector converts to a non-zero exit, while
preserving false for genuinely different entry paths; update the corresponding
docstring-gate tests to assert the failure behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b749deb9-ad19-4873-a206-654310a16114
⛔ Files ignored due to path filters (5)
dist/index.d.tsis excluded by!**/dist/**dist/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapdist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.agents/pm/history/pm-context-ylx4.jsonl.agents/pm/tasks/pm-context-ylx4.toon.github/workflows/ci.ymlCHANGELOG.mdindex.tspackage.jsonscripts/docstring-gate.tstest/docstring-gate.test.ts
`isMainInvocation` caught a `realpathSync` failure and returned false, so the top-level selector called the no-op placeholder and `npm run docstring` exited 0 having scanned nothing — a required release check reporting success without doing its job, which is the single failure this gate exists to prevent. The docstring and the test both described that as failing closed. They were using the phrase to mean "does not crash", which inverts it: the crash is the safe outcome and the silent skip is the dangerous one. Reaching this case requires `argv[1]` to stop resolving after Node has already loaded the file, so it means the environment is broken, and a broken environment must not quietly satisfy a gate. Removes the catch, states the throw in the docstring, and rewrites the test to assert the throw rather than the silent false. A genuinely different entry path still returns false, which is how a test import declines to run the gate. This copy now differs from the same launcher in the other repositories that run the canonical analyzer; propagating it is tracked separately. Found in review of #60.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopt the canonical docstring gate
pm-context was one of the last repositories without a docstring gate. Six exported declarations carried no documentation, and nothing would have caught a seventh.
scripts/docstring-gate.tsimportinganalyzeDocstringCoveragefrompm-ops/docstrings— the analyzer is imported, never vendored.pm-opspinned exactly at2026.8.8.continue-on-error) and part ofrelease:check.scripts/docstring-gate.tsadded tocoverageGate.sources, so the launcher is held to the same floor as the rest of the package rather than sitting outside the gate.This takes canonical adoption to 18 of 22 once pm-web #84 lands, leaving pm-csv, plus pm-cli (upstream) and pm-rust (Rust).
The documentation itself
Documented
CommandErrorand itsexitCode, thePmItemandContextPackOptionsinterfaces, and theMARKDOWN_SECTIONS/AGENT_SECTIONSconstants.Two of these could not be written from the constant lists alone, and the draft that was written that way was wrong:
MARKDOWN_SECTIONScontains bothneighborhoodandneighbors, which read like two spellings of one section. They are not. The markdown renderer matches every section name exactly, and the two emit different content —neighborhoodrenders the relationship edges ("Dependency Neighborhood"),neighborsrenders the neighbouring items ("Neighbor Items"). Selecting one does not include the other.Only the agent renderer accepts aliases, through a second
includeparameter:actionsandnextactionsboth selectnext-actions, andactivityselectsrecent.A docstring claiming the markdown names were interchangeable would have passed the gate — it enforces presence, not truth — and misled every caller who trusted it. Each claim here was checked against the renderer that implements it.
Verification
npm run docstringnpm run typechecknpm testnpm run coveragenpm run changelog:checkpm item
pm-context-ylx4Summary by Sourcery
Adopt the shared pm-ops docstring gate for pm-context and fully document the remaining exported surface and gate launcher behavior.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores:
Summary by cubic
Adopted the canonical docstring gate via
pm-ops/docstrings, documented the remaining public surface, and made the gate required in CI andrelease:check. Hardened the launcher to fail loudly on broken entry paths; coverage remains 100%.New Features
scripts/docstring-gate.tslauncher usingpm-ops/docstrings(imported, not vendored) andnpm run docstring.release:check; included the launcher incoverageGate.sources.CommandError(andexitCode),PmItem,ContextPackOptions,MARKDOWN_SECTIONS, andAGENT_SECTIONS.Bug Fixes
isMainInvocationinstead of skipping the gate, preventingnpm run docstringfrom exiting 0 without scanning; tests updated to assert the throw and symlinked entry handling.Written for commit 4d62939. Summary will update on new commits.