feat(todo.decision-accumulation-finding): flag accepted-decision accumulation - #517
Conversation
…mulation Accepted decisions accrete on a node forever and nothing prompts use of the supersession machinery cairn already has. Add a deterministic scan check counting accepted decisions per node and emitting Info CAIRN_DECISION_ACCUMULATION (CA039) above a threshold, default 10, overridable with the `decision_accumulation_threshold` config key. Success criterion: `cairn lint --json` reports one Info CAIRN_DECISION_ACCUMULATION per node over the threshold (cairn.root, cairn.kernel.cli, cairn.ui today) and `cairn remediate` returns a `consolidate_decisions` action naming them. Consolidation stays judgment work: the remediation prescribes authoring one superseding decision, and names `supersedes` as the canonical link direction (the parser never reads `superseded_by`, the caveat the todo carried). The new match arm pushed src/query_api/handlers/remediate.rs past the 500-line gate, so its test module moves to a sibling remediate/tests.rs.
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds configurable decision-accumulation scanning, CA039 findings, consolidation remediation actions, updated copy and documentation, refreshed metadata, and unit/integration tests covering configured and default thresholds. ChangesDecision accumulation workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectConfig
participant load_project
participant check_decision_accumulation
participant remediate_actions_raw
ProjectConfig->>load_project: configured threshold
load_project->>check_decision_accumulation: graph, artefacts, threshold
check_decision_accumulation->>load_project: CAIRN_DECISION_ACCUMULATION finding
load_project->>remediate_actions_raw: scan result
remediate_actions_raw->>remediate_actions_raw: create consolidate_decisions action
Possibly related PRs
Poem
🚥 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 |
Correctness lens: - A node repeated inside one decision's `nodes:` list counted twice; it now counts once. - The `decision_accumulation_threshold` branch matched at any indentation, so nested prose inside `rules:` could set the global threshold. Gated to top-level, with a regression test. - CA039 was already promised to `CAIRN_SOURCE_SHA256_UNEXPECTED` by the blocked todo.source-tracked-verification-mode; that todo now claims CA040. Simplicity lens: - Document the config key in the spec's config example, not just in the recognised-key list. - Correct the test module comment (it claimed strict-scan coverage the test does not exercise) and drop the exit flag no caller read. - Trim two comments that restated policy or overstated a parser convention. Also fixes a temp-directory collision observed under parallel test load: both CA038 tests and both CA039 tests derived their root from `SystemTime::now()` alone, which is not guaranteed to differ between threads in one binary, so one test could see the other's fixture. An atomic counter now separates them.
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 `@tests/decision_accumulation_remediation.rs`:
- Around line 50-57: Update the run helper to check out.status immediately after
Command::output returns and return an error when the CLI exits unsuccessfully,
before decoding or combining stdout and stderr; preserve the existing
successful-output behavior.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro Plus
Run ID: 320e9c4f-75c5-4b5d-910b-e35c3a5ab035
📒 Files selected for processing (17)
docs/design-system/copy.tomldocs/registries/error-codes.mddocs/spec.mdmap.jsonmeta/contracts/kernel.scanner.mdmeta/todos/todo.decision-accumulation-finding.mdmeta/todos/todo.source-tracked-verification-mode.mdsrc/query_api/handlers/remediate.rssrc/query_api/handlers/remediate/tests.rssrc/scanner/checks.rssrc/scanner/config/mod.rssrc/scanner/config/parse.rssrc/scanner/config/tests.rssrc/scanner/mod.rssrc/scanner/tests.rstests/artefact_filename_remediation.rstests/decision_accumulation_remediation.rs
- The remediation guidance was incomplete and self-defeating: listing stale decisions in a new decision's `supersedes` field does not change their status, so the finding would survive the fix and CAIRN_DECISION_SUPERSEDES_STATUS would join it. Both copy surfaces now require setting each target to `status: superseded` as well, and say why. - The default threshold of 10 had no regression cover: every positive assertion used an explicit config value, so disabling the fallback would have gone unnoticed. `default_threshold_is_ten` exercises both sides of the compiled-in boundary with no config file. - The integration helper swallowed a nonzero exit and concatenated stderr into the JSON it parsed, so a negative assertion could pass on a crashed invocation. It now fails on a nonzero exit and returns stdout alone. - Both fixture helpers use `tempfile::TempDir` instead of a clock-derived path, which removes the cross-process collision the counter still allowed and cleans up on unwind.
* docs(meta): four todos for streamlining the dev loop, measured Shapes the maintainer's four items into native todos, each carrying the evidence a transcript and artefact audit of 2026-07-28 produced, including the parts that contradict the original framing. - todo.decision-ratification-tiers (cairn.kernel.artefacts): `local` versus `binding` tiers, tier validated from the graph for container span and supersession plus a declared `affects:` list checked at the hook against the diff, self acceptance gated on two independent clean lens reports on the final SHA, `ratified_by: machine` for audit, and one narrow widening candidate. - todo.lint-selection-folding (cairn.kernel.scanner): typed `defers:` parking for Info findings only, Info non-selecting while `scan --strict` is green, and the accumulation question left open rather than answered with a configurable count. - todo.greenfield-completion-eval (cairn.root): reuse the archived runner and receipts, add briefs with a required-outcome oracle, a build lifecycle, a scripted persona with a clarification budget, and baseline-delta scoring at release time. - todo.local-gate-attestation (cairn.root): pin the toolchain and reconcile the two gate invocations first, then treat the signed receipt as a conditional second phase with a stated trust model. Corrections the audit forced, recorded in the bodies: - `CAIRN_DECISION_ACCUMULATION` did not regenerate node to node. #517 added the check and reported cli 20, root 23, ui 14 at once; #518 to #520 serialised a pre-existing set. The surviving concern is that three nodes now sit at exactly the flat threshold of 10. - Arguing about Info selectability cost about 5% of loop elapsed time, not the bulk of it, though all 14 iterations re-litigated it. The expensive effect was which units got selected: four finding-selected iterations spent 22% of the day on bookkeeping. - CI is not the bottleneck it was assumed to be: median all-checks-green was 159s against roughly 114s for the warm local gate set. CI is also not hermetic, so it could not validate a receipt today. - Two clean review lenses are not equivalent to a maintainer signature: later passes and CodeRabbit caught defects the pairs missed on #506, #511, and #521, and one lens produced a proven false blocker on #506. * docs(meta): point the four todos at the audit inventory on PR #523 * docs(meta): bind ratification receipts to typed reviews and a candidate commit * docs(meta): bind ratification receipts to a squash-safe manifest hash * docs(meta): make lens identity, allowlist matching, and surfaces precise
Accepted decisions accrete on a node forever and nothing prompts use of the
supersession machinery cairn already has. Add a deterministic scan check
counting accepted decisions per node and emitting Info
CAIRN_DECISION_ACCUMULATION (CA039) above a threshold, default 10,
overridable with the
decision_accumulation_thresholdconfig key.Success criterion:
cairn lint --jsonreports one InfoCAIRN_DECISION_ACCUMULATION per node over the threshold (cairn.root,
cairn.kernel.cli, cairn.ui today) and
cairn remediatereturns aconsolidate_decisionsaction naming them.Consolidation stays judgment work: the remediation prescribes authoring one
superseding decision, and names
supersedesas the canonical link direction(the parser never reads
superseded_by, the caveat the todo carried).The new match arm pushed src/query_api/handlers/remediate.rs past the
500-line gate, so its test module moves to a sibling remediate/tests.rs.