Adopt the mandatory docstring gate - #58
Merged
Merged
Conversation
Add a thin launcher (scripts/docstring-gate.ts) over the canonical
pm-ops/docstrings analyzer so this package enforces the same lexer-backed
docstring policy as the rest of the fleet, rather than a per-repo fork. The
analyzer is pinned exactly to pm-ops@2026.8.8 and imported from the published
pm-ops/docstrings subpath; it has no ignore list and fails closed on unknown
declaration forms.
The gate requires a JSDoc block that adds information beyond the identifier on
every exported declaration, every public member of an exported class, and every
non-exported function with a body over the threshold. This package had
twenty-six undocumented declarations in src/index.ts, spanning the Neo4j
runtime (config/load/driver/sync/destructive-query guard), graph construction,
multi-format export (cypher/mermaid/dot/graphml/plantuml), analysis
(criticalConnectors, explain types, item-id resolution) and the extension
entry point. Each now has a docstring written against its body — for example
that criticalConnectors analyses the dependency graph as UNDIRECTED, that
findDestructiveKeyword exempts "SET SESSION", and that the two number readers
(readNumberProperty vs toNumber) differ on whether a missing value is null or 0.
Wiring:
- package.json: pm-ops devDep (exact pin), `docstring` script, the step added
to release:check, and scripts/docstring-gate.ts listed in coverageGate.sources
(the scripts/ dir is a coverage skip-dir, mirroring prepare-merge-driver.ts).
- .github/workflows/ci.yml: a "Verify complete docstring coverage" step.
- test/docstring-gate.test.ts ports the pm-brief launcher suite.
- dist/ rebuilt: this package is installed by copying the repo, so the
compiled docstrings are committed alongside the source.
All quality gates green: typecheck, build, docstring (0 violations), coverage
(85.42 / 80.20 / 90.43 against 81 / 77 / 88), audit:prod, pack:dry-run,
changelog:check, and the full test suite (216 passing).
Tracked pm item: .agents/pm/tasks/pm-graph-k5ao.toon
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Summary by CodeRabbit
WalkthroughThe PR adds a mandatory docstring coverage gate. It documents declarations in ChangesDocstring coverage gate
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI test job
participant npm as npm run docstring
participant gate as docstring-gate.ts
participant analyzer as pm-ops/docstrings
CI->>npm: run docstring check
npm->>gate: start launcher
gate->>analyzer: analyze TypeScript docstrings
analyzer-->>gate: return coverage result
gate-->>CI: output result and exit status
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 GuideThis PR wires pm-graph into the fleet-wide mandatory docstring gate using the canonical pm-ops/docstrings analyzer, adds a thin launcher and test suite around it, integrates the gate into CI and release checks, and backfills JSDoc docstrings for previously undocumented public and key internal APIs so the gate passes with zero violations. Sequence diagram for the new docstring gate launcher and CI integrationsequenceDiagram
actor Dev
participant GitHubActions
participant npm
participant docstring_gate_ts as docstring_gate_main
participant pm_ops_docstrings as analyzeDocstringCoverage
Dev->>GitHubActions: push / PR
GitHubActions->>npm: run docstring
npm->>docstring_gate_ts: node scripts/docstring-gate.ts
docstring_gate_ts->>pm_ops_docstrings: analyzeDocstringCoverage({ root })
alt [violations > 0]
pm_ops_docstrings-->>docstring_gate_ts: report with violations
docstring_gate_ts-->>npm: exitCode = 1
docstring_gate_ts-->>npm: stderr "docstring-gate: n violation(s)…"
else [no violations]
pm_ops_docstrings-->>docstring_gate_ts: clean report
docstring_gate_ts-->>npm: exitCode = 0
docstring_gate_ts-->>npm: stdout "docstring-gate: files, declarations documented"
end
npm-->>GitHubActions: propagate exit code
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 `@src/index.ts`:
- Around line 2431-2443: Update the synchronization documentation for the
function containing the fullSync and currentIds logic to state that incremental
deletion of absent nodes occurs only when currentIds.size is greater than zero;
explicitly document that an empty graph leaves existing project nodes unchanged.
🪄 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: 4e8388ac-5904-413a-9328-f7db5f2a01c3
⛔ Files ignored due to path filters (4)
dist/index.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.agents/pm/history/pm-graph-k5ao.jsonl.agents/pm/tasks/pm-graph-k5ao.toon.github/workflows/ci.ymlpackage.jsonscripts/docstring-gate.tssrc/index.tstest/docstring-gate.test.ts
Greptile SummaryThis PR adopts the shared mandatory docstring gate and documents previously uncovered declarations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/docstring-gate.ts | Adds a testable launcher around the shared analyzer, including CLI output and direct-invocation handling. |
| test/docstring-gate.test.ts | Covers successful and failed analysis, stream handling, root selection, and entry-point detection. |
| package.json | Adds the exact-pinned analyzer dependency and integrates the gate into release and coverage configuration. |
| .github/workflows/ci.yml | Runs the new docstring gate in the main CI job. |
| src/index.ts | Adds documentation to previously undocumented declarations without changing executable behavior. |
| dist/index.js | Rebuilds the distributed JavaScript with corresponding documentation comments and unchanged runtime logic. |
| dist/index.d.ts | Publishes the newly added API documentation in generated type declarations. |
Reviews (3): Last reviewed commit: "Rebuild dist for the incremental-sync do..." | Re-trigger Greptile
CI verifies with changelog:check, which runs in --mode replace --all-release-tags. Only changelog:full generates in that mode, so a changelog produced by the prepend-mode changelog script leaves the check failing.
The docstring said an incremental sync DETACH DELETEs every node absent from the incoming graph. The pruning query is guarded by currentIds.size > 0, so an incremental sync of an empty graph deletes nothing and the stored project nodes survive. The guard is deliberate - an empty read is far more likely to be a failed or misconfigured scan than a project whose every item was deleted - so the docstring now states the exception and the reasoning, and points at fullSync as the way to empty a project on purpose. Reported by CodeRabbit on #58.
Owner
Author
|
Both docstring corrections pushed, each verified against the call site before editing. @greptileai review |
|
|
The docstring correction to syncNeo4j (documenting the empty-graph pruning exception) changed src/index.ts but the committed dist/ was not rebuilt, so the "committed dist matches a fresh build" gate drifted. Rebuilt with `npm run build`; only dist/index.js and its source map changed, both reflecting the docstring.
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.
Summary
Adopts the fleet's mandatory docstring gate in
pm-graph, mirroring the canonical wiring already shipping inpm-brief. The analyzer is not vendored or re-implemented — it is imported from the publishedpm-ops/docstringssubpath and pinned exactly topm-ops@2026.8.8(no^/~).A docstring gate enforces presence, never truth, so every violation was documented against its body rather than its name.
What changed
scripts/docstring-gate.ts— thin launcher exporting a purerunGate(root): {exitCode, stdout, stderr}plus a thinmain(args), ported frompm-brief.test/docstring-gate.test.ts— ports the pm-brief launcher suite.package.json—pm-opsdevDep (exact pin), adocstringscript,docstringwired intorelease:check, andscripts/docstring-gate.tsadded tocoverageGate.sources(thescripts/dir is a coverage skip-dir, mirroring the existingprepare-merge-driver.tsentry)..github/workflows/ci.yml— aVerify complete docstring coveragestep.src/index.ts(+ rebuiltdist/) — 26 previously-undocumented declarations now have JSDoc.Violations: 26 → 0
All in
src/index.ts, spanning: the Neo4j runtime (neo4jConfigured,neo4jMissingMessage,loadNeo4j,createDriver,syncNeo4j,findDestructiveKeyword), graph construction (relationshipTarget,graphFromItems,toNumber,readNumberProperty,itemNodeMap), multi-format export (cypherStatements,ExportFormat,EdgeFilter,renderMermaid,renderDot,renderExport,readExportOption), analysis (criticalConnectors,ExplainNeighbor,ExplainReport), item-id resolution (sharedPrefixLength,suggestItemIds,ambiguousItemIdError,resolveItemIdOrThrow), andactivate.A few non-obvious truths the docstrings now state:
criticalConnectorsbuilds symmetric adjacency — it analyzes the dependency graph as undirected.findDestructiveKeywordflagsSETbut exemptsSET SESSION(regex negative-lookahead).readNumberPropertyreturnsnullfor a missing value (so "no priority" ≠ 0) whiletoNumbercollapses missing to0for summing.syncNeo4jincremental mode deletes stale nodes absent from the current graph;fullSyncwipes first.Quality gates (all green)
npm run typechecknpm run buildnpm run docstring— 0 violationsnpm run coverage— 85.42 / 80.20 / 90.43 against thresholds 81 / 77 / 88 (no regression)npm run release:check(aggregate: typecheck, build, docstring, coverage, audit:prod, pack:dry-run, changelog:check)npm test— 216 passingpm item
Tracked in
pm-graph-k5ao.Summary by Sourcery
Adopt the shared docstring coverage gate for pm-graph and document all previously undocumented public APIs.
New Features:
Enhancements:
Build:
CI:
Tests:
Summary by cubic
Adopted the mandatory docstring gate for
pm-graphviapm-ops/docstrings, wired into CI andrelease:check. Documented all previously undocumented declarations and clarified that incremental Neo4j sync skips pruning when the incoming graph is empty; use full sync to wipe.New Features
scripts/docstring-gate.tslauncher and adocstringnpm script; integrated intorelease:checkand CI.test/docstring-gate.test.tsand includedscripts/docstring-gate.tsincoverageGate.sources.dist/to reflect the sync docstring update and updated CHANGELOG.Dependencies
pm-ops@2026.8.8(exact pin) and import frompm-ops/docstrings.Written for commit cbaad0d. Summary will update on new commits.