fix(decisions): list reads the decisions directory, not INDEX.md (sc-2332) - #599
Merged
Merged
Conversation
…2332)
`guard-decisions list` printed `No decisions recorded.` in any checkout whose `<decisionsDir>` held committed axis files but no git-tracked `INDEX.md`, while `query` and `categories` saw every axis in that same directory. It was not an empty answer — it was a positive claim of emptiness for a repo that had decided.
`skills/brainstorming/SKILL.md:96` tells an agent to run `list` to find a prior ruling before recording a new one. A false "nothing decided" there mints a duplicate axis for an already-ruled question. devkit's own `INDEX.md` is committed and complete (76/76), which is why self-host never saw this: the harm was consumer-side.
## Cause
`cmdList` gated on `existsSync(<decisionsDir>/INDEX.md)` and never read `decisionsDir` at all. Since `INDEX.md` is upserted one row at a time by `add --target` and full-rebuilt only by `amend --target`, it is routinely absent *or* merely incomplete — the 23-of-86 case measured under the governing Target. So the fix removes the gate rather than adding a missing-file fallback.
This finishes a migration already ruled on. `decision-retrieval-candidate-set` (Target 2026-07-25, Scope `gate-engine/decisions/**`) demoted `INDEX.md` to a rendered view; six other readers already enumerate the directory. `cmdList` was the last holdout. Its Rejected option (b) forecloses repairing `INDEX.md` on write, so `list` stays read-only.
## Change
`renderSpine()` in `recall/full-print.mts`: `loadAxisRows` → narrow cells → `renderIndex`. Placed there per that file's own documented precedent for `printRanked` — `decisions.mts` is at 498 of its 500-line cap.
The narrowing is the whole subtlety. `loadAxisRows` is a *retrieval* shape: `why` is the axis's full Context and `updated` is the last date anywhere in the body, notes included, while both `INDEX.md` writers clamp `why` through `whyHook` and record the **Target's** date. Rendering the rows raw grew this repo's own spine 71,292 → 114,634 bytes (+61%, 76/76 `why` cells) and moved 37/76 `updated` cells onto note dates, collapsing the `liveRulingId`-vs-`updated` staleness signal that axis records as load-bearing. With the narrowing, `list` output is **byte-identical to the committed `INDEX.md` on all 76 rows**.
## Tests
Ten cases in `__tests__/cli.test.mts`, all driven through the real bin, asserting against the fixture's own file set — never against `loadAxisRows` (tautological post-fix) or `query` (ranks, caps at `--top 5`, and lazily writes `.decisions/`, destroying the fixture precondition).
- no `INDEX.md` → every axis listed; incomplete `INDEX.md` → the omitted axis still listed; orphan row whose file is gone → omitted
- Target-less legacy axis kept as a well-formed row (the case `amend`'s `regenerateIndex` silently drops)
- `why` clamped to a hook; `updated` from the Target, not a later note — on both the `target:` and `entry:` branches
- a ruling containing `|` still renders 4 cells; an empty axis file renders a blank row instead of throwing
- `list` under a broken bundle names the outage and never claims the log is empty — `list` parses markdown now, so the sc-2692 contract became load-bearing here, and an outage rendering as an empty log would re-open this bug by a second door
Both date tests were mutation-verified: dropping the narrowing fails them.
## Acceptance criteria
AC1 and AC3 as written. AC2 ("name the command that builds the index") is **vacuously satisfied, not dropped** — `list` depends on no index now, and no devkit command rebuilds `INDEX.md`, so that hint would name a remediation the user cannot perform.
Out of scope: `skills/brainstorming/SKILL.md:96` keeps calling `list` — a corrected `list` is exactly what that call site needs. Moving it to `query` belongs to `decision-log-informs-before-work`'s Scope.
Closes sc-2332.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe ChangesDirectory-sourced decision listing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The decision list now reliably shows committed decision files even when INDEX.md is missing or stale, while preserving formatted output and read-only behavior. No merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant cmdList
participant renderSpine
participant loadAxisRows
participant renderIndex
cmdList->>renderSpine: renderSpine(paths(cwd))
renderSpine->>loadAxisRows: Load rows from the decisions directory
loadAxisRows-->>renderSpine: Return axis rows
renderSpine->>renderIndex: Render rows with dates and why text
renderIndex-->>renderSpine: Return formatted spine
renderSpine-->>cmdList: Return spine or null
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
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.
guard-decisions listprintedNo decisions recorded.in any checkout whose<decisionsDir>held committed axis files but no git-trackedINDEX.md, whilequeryandcategoriessaw every axis in that same directory. It was not an empty answer — it was a positive claim of emptiness for a repo that had decided.skills/brainstorming/SKILL.md:96tells an agent to runlistto find a prior ruling before recording a new one. A false "nothing decided" there mints a duplicate axis for an already-ruled question. devkit's ownINDEX.mdis committed and complete (76/76), which is why self-host never saw this: the harm was consumer-side.Cause
cmdListgated onexistsSync(<decisionsDir>/INDEX.md)and never readdecisionsDirat all. SinceINDEX.mdis upserted one row at a time byadd --targetand full-rebuilt only byamend --target, it is routinely absent or merely incomplete — the 23-of-86 case measured under the governing Target. So the fix removes the gate rather than adding a missing-file fallback.This finishes a migration already ruled on.
decision-retrieval-candidate-set(Target 2026-07-25, Scopegate-engine/decisions/**) demotedINDEX.mdto a rendered view; six other readers already enumerate the directory.cmdListwas the last holdout. Its Rejected option (b) forecloses repairingINDEX.mdon write, soliststays read-only.Change
renderSpine()inrecall/full-print.mts:loadAxisRows→ narrow cells →renderIndex. Placed there per that file's own documented precedent forprintRanked—decisions.mtsis at 498 of its 500-line cap.The narrowing is the whole subtlety.
loadAxisRowsis a retrieval shape:whyis the axis's full Context andupdatedis the last date anywhere in the body, notes included, while bothINDEX.mdwriters clampwhythroughwhyHookand record the Target's date. Rendering the rows raw grew this repo's own spine 71,292 → 114,634 bytes (+61%, 76/76whycells) and moved 37/76updatedcells onto note dates, collapsing theliveRulingId-vs-updatedstaleness signal that axis records as load-bearing. With the narrowing,listoutput is byte-identical to the committedINDEX.mdon all 76 rows.Tests
Ten cases in
__tests__/cli.test.mts, all driven through the real bin, asserting against the fixture's own file set — never againstloadAxisRows(tautological post-fix) orquery(ranks, caps at--top 5, and lazily writes.decisions/, destroying the fixture precondition).INDEX.md→ every axis listed; incompleteINDEX.md→ the omitted axis still listed; orphan row whose file is gone → omittedamend'sregenerateIndexsilently drops)whyclamped to a hook;updatedfrom the Target, not a later note — on both thetarget:andentry:branches|still renders 4 cells; an empty axis file renders a blank row instead of throwinglistunder a broken bundle names the outage and never claims the log is empty —listparses markdown now, so the sc-2692 contract became load-bearing here, and an outage rendering as an empty log would re-open this bug by a second doorBoth date tests were mutation-verified: dropping the narrowing fails them.
Acceptance criteria
AC1 and AC3 as written. AC2 ("name the command that builds the index") is vacuously satisfied, not dropped —
listdepends on no index now, and no devkit command rebuildsINDEX.md, so that hint would name a remediation the user cannot perform.Out of scope:
skills/brainstorming/SKILL.md:96keeps callinglist— a correctedlistis exactly what that call site needs. Moving it toquerybelongs todecision-log-informs-before-work's Scope.Closes sc-2332.
Summary by CodeRabbit
listcommand now reflects decisions found in the decisions directory, even whenINDEX.mdis missing or outdated.