refactor: split roadmap inventory policy ownership - #300
Conversation
Summary by CodeRabbit
WalkthroughThe PR extracts roadmap accountability validation into a dedicated policy module. The inventory checker delegates to it. Tests enforce process contracts, ownership boundaries, and per-owner source budgets. Documentation records the completed decomposition. ChangesRoadmap policy ownership
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InventoryCLI
participant parseRoadmapInventory
participant validateArchitectureAccountability
InventoryCLI->>parseRoadmapInventory: Read roadmap and issue fixtures
parseRoadmapInventory->>validateArchitectureAccountability: Validate accountability Markdown
validateArchitectureAccountability-->>parseRoadmapInventory: Parsed tree or source-located error
parseRoadmapInventory-->>InventoryCLI: Inventory result and process output
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@docs/workflows/repository-maintenance/test-plan.md`:
- Around line 394-412: Update RM-15c and RM-15b in the test plan to use
“Requirements” consistently, move numeric measurements out of “Evidence type,”
and record exact paths plus executed test names:
scripts/check-roadmap-inventory.mjs, scripts/roadmap-accountability-policy.mjs,
scripts/roadmap-inventory-runner.mjs, “delegates roadmap policy to bounded pure
owners,” and “pins exact process bytes across the roadmap ownership seam.”
Reconcile the transport runner’s 249/250 measurement with the “meaningful
maintenance headroom” claim by adjusting the ceiling or claim while preserving
the stated budget behavior.
In `@ROADMAP.md`:
- Around line 193-197: Update the `#260` roadmap entry to remove the
already-delivered checker decomposition from its active backlog wording, leaving
only the remaining cursor-complete transport idea or explicitly noting that the
decomposition was delivered through `#257`. Keep the `#257` delivered entry and its
primary marker unchanged.
In `@scripts/check-roadmap-inventory.test.mjs`:
- Around line 1613-1628: Increase the roadmap transport runner line ceiling in
the budgets array to a reviewed value providing meaningful proportional
headroom, rather than the current 250-line limit. Update the corresponding
runner ceiling records in the README and test-plan documentation, including
RM-15c if the ceiling remains intentionally tight.
- Around line 1560-1565: Update sourceMeasurement to count top-level
function-valued bindings, including arrow functions and function expressions, in
addition to function declarations and class methods, so helper growth cannot
evade the budget. Then update the corresponding published helper measurements in
the maintenance README and test-plan documentation so the assertions in the
inventory tests remain consistent.
- Around line 1702-1714: Add digit boundaries to the interpolated measurement
values in the inventory and accountability RegExp assertions, so line and helper
counts match complete numeric tokens rather than substrings of larger numbers.
Update both patterns in the relevant assert.match checks while preserving their
existing text matching and error messages.
In `@scripts/roadmap-accountability-policy.mjs`:
- Around line 361-384: In the pipe-scanning logic, replace the local
precedingBackslashes parity loop with the existing isMarkdownEscaped helper.
Preserve the current behavior by continuing past escaped pipe characters and
only returning true for unescaped pipes outside inlineCodeRanges; remove the
now-unnecessary local counting code.
- Around line 18-19: Anchor the MARKDOWN_CHARACTER_REFERENCE regular expression
at the start so it only matches a character reference beginning at the probed
substring. Preserve the existing reference syntax and leave validateTextSource
behavior unchanged.
- Around line 187-233: Document on the exported
validateArchitectureAccountability seam that its fail callback must always throw
and never return, covering the invariant relied on by validateTextSource and the
other validation paths. Prefer a JSDoc contract on
validateArchitectureAccountability; alternatively, add explicit returns after
every fail call before subsequent values are dereferenced.
🪄 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: d08ae66a-b9cc-4c80-ad16-48e418d5a024
📒 Files selected for processing (7)
CHANGELOG.mdROADMAP.mddocs/workflows/repository-maintenance/README.mddocs/workflows/repository-maintenance/test-plan.mdscripts/check-roadmap-inventory.mjsscripts/check-roadmap-inventory.test.mjsscripts/roadmap-accountability-policy.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
docs/workflows/**/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Workflow README files must describe only implemented current truth; planned verification belongs in test-plan.md.
Files:
docs/workflows/repository-maintenance/README.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
New durable documentation pages must be linked from docs/README.md and follow the documentation corpus standard, including one primary reader job.
Files:
docs/workflows/repository-maintenance/README.mddocs/workflows/repository-maintenance/test-plan.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Use one logical change per commit and Conventional Commit prefixes such as feat, fix, docs, refactor, test, or chore.
Use runnable examples where practical, separate commands from expected output, omit shell prompts from copyable command blocks, warn before destructive or privileged commands, and provide useful visual alt text or nearby equivalents.
Treat prose metrics as editorial signals, not universal merge gates; hard gates concern links, examples, generated references, evidence, Markdown, whitespace, and contract coverage.
For documentation changes, run markdownlint-cli2, diff whitespace checks, internal-link checks, and documentation-citation checks.
Files:
docs/workflows/repository-maintenance/README.mdROADMAP.mdCHANGELOG.mddocs/workflows/repository-maintenance/test-plan.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not force-push, rebase, squash, or amend shared branches; make a new commit instead.
Do not claim work or verification is complete unless it was actually performed; report failures with their output.
Do not casually regenerate golden fixtures; golden changes must be deliberate, reviewable, and tied to a contract change.
Files:
docs/workflows/repository-maintenance/README.mdROADMAP.mdCHANGELOG.mddocs/workflows/repository-maintenance/test-plan.mdscripts/check-roadmap-inventory.mjsscripts/check-roadmap-inventory.test.mjsscripts/roadmap-accountability-policy.mjs
ROADMAP.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep roadmap anchors synchronized with goalpost and issue status, and do not describe unbuilt goalposts as existing.
Files:
ROADMAP.md
CHANGELOG.md
📄 CodeRabbit inference engine (AGENTS.md)
Update the changelog for release-visible changes.
Files:
CHANGELOG.md
docs/workflows/**/test-plan.md
📄 CodeRabbit inference engine (AGENTS.md)
Before implementation, record planned cases with stable IDs, requirements, explicit oracles, evidence types, and status; later record actual evidence and mark implemented.
Files:
docs/workflows/repository-maintenance/test-plan.md
🧠 Learnings (3)
📚 Learning: 2026-07-30T04:29:07.448Z
Learnt from: flyingrobots
Repo: flyingrobots/colorful-language PR: 279
File: docs/workflows/evidence-toolchains/README.md:0-0
Timestamp: 2026-07-30T04:29:07.448Z
Learning: In this repository’s GitHub Actions workflow documentation (docs/workflows/**/*.md), require action references to be pinned and documented consistently: (1) For non-Docker third-party actions, use the full 40-character commit-SHA pin (not a tag/short SHA). (2) For `docker://` actions, use an immutable `sha256` image-digest pin. (3) When reviewing/referencing actions, include release/review comments on the same line as the action reference, and keep comments/references consistent for the same action identity when it appears in multiple workflows.
Applied to files:
docs/workflows/repository-maintenance/README.mddocs/workflows/repository-maintenance/test-plan.md
📚 Learning: 2026-07-30T04:29:22.377Z
Learnt from: flyingrobots
Repo: flyingrobots/colorful-language PR: 279
File: docs/workflows/evidence-toolchains/test-plan.md:170-195
Timestamp: 2026-07-30T04:29:22.377Z
Learning: In any `docs/workflows/**/test-plan.md` durable test-plan document, record evidence using concrete, long-lived repository references: include exact repository paths plus the precise names of tests, fixtures, doctests, or commands that were executed. Avoid ephemeral references such as pull-request-specific CI URLs, locally generated/run-only artifacts, or other non-portable evidence that won’t be reproducible from the repo state later.
Applied to files:
docs/workflows/repository-maintenance/test-plan.md
📚 Learning: 2026-07-30T04:29:11.102Z
Learnt from: flyingrobots
Repo: flyingrobots/colorful-language PR: 279
File: scripts/check-coverage-policy.mjs:36-41
Timestamp: 2026-07-30T04:29:11.102Z
Learning: In this repository’s GitHub Actions workflow validation code, treat `scripts/check-dependency-update-policy.mjs` as the *only* source of mutable pin data. It should own full commit SHA or Docker digest pins, any release/comment text it depends on, and the identity→pin mapping across the workflow family. For `scripts/check-coverage-policy.mjs`, `scripts/check-repository-maintenance.mjs`, and any other `scripts/check-*.mjs` validators, only verify action identity, enforce full-SHA pin syntax, and apply their specific security/topology semantics—without duplicating mutable release SHA constants or identity→pin mappings. This prevents Dependabot/updates from becoming unsatisfiable across the workflow family.
Applied to files:
scripts/check-roadmap-inventory.mjsscripts/check-roadmap-inventory.test.mjs
🔇 Additional comments (16)
scripts/roadmap-accountability-policy.mjs (8)
30-57: LGTM!
59-94: LGTM!
96-136: LGTM!
138-185: LGTM!
235-285: LGTM!
287-341: LGTM!
386-643: LGTM!
274-279: 🗄️ Data Integrity & IntegrationNo change needed. The call form matches the current v2 packages:
micromark-extension-gfm-table2.1.1 exposesgfmTableas a factory, andmdast-util-gfm-table2.0.0 exposesgfmTableFromMarkdownas a factory.scripts/check-roadmap-inventory.mjs (1)
5-5: LGTM!Also applies to: 80-84
scripts/check-roadmap-inventory.test.mjs (5)
27-29: LGTM!
63-83: LGTM!
172-254: LGTM!
569-580: LGTM!Also applies to: 1525-1539
1631-1636: LGTM!Also applies to: 1669-1691
docs/workflows/repository-maintenance/test-plan.md (1)
365-373: LGTM!CHANGELOG.md (1)
12-18: LGTM!
1 participant
What changed
Extract architecture-accountability Markdown interpretation and diagnostics into a pure policy module, leaving primary-marker and issue-state reconciliation in the inventory owner and transport in the runner. Enforce separate line/helper ceilings with meaningful headroom and preserve exact process behavior.
Closes #257
Evidence
node --test scripts/check-roadmap-inventory.test.mjs: 85/85 passed, including exact stdout/stderr characterization and cross-owner ownership-budget mutations.PATH=/Users/james/.local/share/mise/installs/node/22.23.1/bin:$PATH bash scripts/release-prep.sh: passed the complete local release-preparation gate at09dc60a. The gate packaged and installed a VSIX only inside its local smoke harness; it did not publish any extension or crate.09dc60a; all 8 review threads are resolved.7f11c07; failing-evidence commit:63352cd; passing-implementation commit:9a69be8; review-hardening commit:09dc60a.Checklist
README/docs/topics/<topic>/README.md) describe only implemented behavior.CHANGELOG.mdandROADMAP.mdrecord the delivered slice.cargo fmt, warning-denyingcargo clippy, and locked workspace tests pass locally.