Skip to content

fix(brownfield,cli): name the review step after init --from-code - #609

Merged
George-RD merged 1 commit into
mainfrom
loop/todo.brownfield-init-review-handoff
Aug 7, 2026
Merged

fix(brownfield,cli): name the review step after init --from-code#609
George-RD merged 1 commit into
mainfrom
loop/todo.brownfield-init-review-handoff

Conversation

@George-RD

Copy link
Copy Markdown
Collaborator

todo.brownfield-init-review-handoff (node cairn.brownfield).

Problem

The non-apply branch of cairn init --from-code was a dead end. It returned one sentence, ignored --json, and the generated proposal presented a source-file-count confidence score with nothing to say it is not an architectural claim. At that moment the agent pack and the cairn-dev skill are not installed yet, so the CLI is the only surface that can name the semantic review the whole brownfield model depends on. An agent entering through the CLI could apply a plausible but structurally wrong hierarchy without ever learning a review step existed.

Success criterion

cargo test proves the human ladder leads with review before apply, that --json returns an envelope whose next_actions are the same steps in the same order as the human output, and that the generated proposal.md carries the review checklist and the confidence disclaimer.

Change

  • Copy. New [init.from-code] table in docs/design-system/copy.toml: done, next-steps-header, and one ordered next-actions list. The human ladder and the JSON array are both rendered from that single list, so the two orderings cannot drift.
  • JSON contract. brownfield_init_report honours --json on the non-apply branch, emitting schema_version, command, change_id, change_path, applied: false, and next_actions. One flag no longer means two behaviours on one command.
  • Proposal. write_change appends a review checklist to every generated proposal: path ownership, grouping, edges, contracts, currency, hierarchy, plus a plain statement that confidence scores source-file count and is not a claim of architectural correctness, pointing at docs/brownfield.md.
  • Plan. todo.brownfield-one-step-first-map's 2026-07-12 resolution note covered the greenfield init.next-steps keys only; its body now records that the from-code branch has its own ladder (acceptance item 4).

Verification

  • cargo test: 1661 lib + all integration suites pass, including the two new CLI regression tests and the new write_change proposal test.
  • cargo clippy --all-targets --all-features -- -D warnings: clean.
  • cairn scan --strict: exit 0 (30 standing Info findings, unchanged).
  • cairn hook all: exit 0.
  • scripts/check-file-sizes.sh, scripts/check-voice-markers.sh: clean.
  • Smoke test on a fresh temp repo: human ladder renders six numbered steps with review first; --json returns a valid envelope whose next_actions match byte for byte; proposal.md carries the checklist.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added a review-before-apply workflow for init --from-code, with ordered next steps for reviewing, applying, wiring, scanning, onboarding, and validating changes.
    • Added confidence guidance and curation checks to generated proposals.
    • Added structured JSON results, including change details, application status, and next actions.
  • Documentation

    • Updated guidance and completion messages to explain the new initialization review flow.
  • Bug Fixes

    • Improved consistency between human-readable and JSON initialization results.

Walkthrough

The brownfield flow now adds review guidance to generated proposals and reports ordered follow-up actions for non-apply cairn init --from-code in human-readable and JSON output.

Changes

Brownfield initialization review flow

Layer / File(s) Summary
Proposal review checklist
src/brownfield/mod.rs
Generated proposals include confidence limits and six review items. Tests verify checklist content and placement.
Init next-action reporting
src/cli/mod.rs, docs/design-system/copy.toml, meta/todos/*, map.json
Non-apply initialization renders ordered next actions and returns matching structured JSON data. Copy keys, source ranges, and todo metadata are updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant run as CLI run
  participant report as Brownfield next-action reporting
  participant copy as copy.toml
  participant output as Human or JSON output
  run->>report: Execute non-apply init --from-code
  report->>copy: Load init.from-code copy keys
  copy-->>report: Return ordered next-action text
  report->>output: Render change details and next_actions
Loading

Possibly related PRs

Poem

A rabbit checks each proposed line,
Then hops through actions in a sign.
Human or JSON, the path is clear,
Review comes first, then apply draws near.
Six checklist steps now guard the way.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the brownfield CLI change and the review step added to init --from-code.
Description check ✅ Passed The description directly explains the non-apply flow, JSON contract, proposal checklist, planning update, and verification results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch loop/todo.brownfield-init-review-handoff

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

todo.brownfield-init-review-handoff. The non-apply branch of
`cairn init --from-code` was a dead end: it printed one sentence, ignored
`--json` entirely, and the generated proposal presented a source-file-count
confidence score with nothing to say it is not an architectural claim. At that
point the agent pack and the cairn-dev skill are not installed yet, so the CLI
is the only thing that can name the semantic review the brownfield model
depends on.

Success criterion: cargo test proves the human ladder leads with review before
apply, that `--json` returns an envelope whose next_actions are the same steps
in the same order, and that the generated proposal.md carries the review
checklist and the confidence disclaimer.

- New `init.from-code` copy: `done`, `next-steps-header`, and a single ordered
  `next-actions` list. Human output and the JSON envelope both render that one
  list, so the two orderings cannot drift.
- `brownfield_init_report` honours `--json` on the non-apply branch, emitting
  schema_version, change_id, change_path, applied=false, and next_actions. One
  flag no longer means two behaviours on one command.
- `write_change` appends a review checklist to every generated proposal,
  covering path ownership, grouping, edges, contracts, currency, and hierarchy,
  and states plainly that confidence is a structural signal.
- todo.brownfield-one-step-first-map's 2026-07-12 resolution note covered the
  greenfield next-steps keys only; its body now records what this unit closed.
@George-RD
George-RD force-pushed the loop/todo.brownfield-init-review-handoff branch from 2af8e8a to e6233b2 Compare August 7, 2026 19:04
@George-RD

Copy link
Copy Markdown
Collaborator Author

Pre-submit two-lens review applied (correctness + simplicity, run in parallel).

Fixed:

  • Silent copy-key fallback (must-fix). copy::lookup returns the key itself on a miss, so a renamed init.from-code.done or next-steps-header would have printed the literal key at exit 0 on the P1 cold-start path with no test catching it (only next-actions failed loudly on its own). Added test_init_from_code_copy_keys_resolve, following the existing flag_and_usage_copy_keys_resolve guard pattern.
  • Envelope dialect (recommended). The first draft emitted a flat {schema_version, command, ...} payload while the --apply branch of the same command emits {command, status, data} via archive. Shipping a third dialect on the very command whose defect was "one flag, two behaviours" is self-defeating, so the non-apply envelope now uses the same family, with the trailing newline the other envelopes carry.

Dropped, with reasons:

  • ladder_steps split_once(". ") fragility: the reviewer confirmed it fails safe (a future ". " in an action breaks the equality assertion loudly) and the real anti-drift guarantee is that both sides derive from one Vec.
  • refine's proposal getting no checklist and its own hardcoded done-string: a different command with a different emitter and no confidence column. Out of this unit's scope.
  • change_path lacking a trailing slash: a path value should not carry one; the slash in done is prose.
  • Renaming REVIEW_CHECKLIST and trimming doc-comments: cosmetic, not worth a churn commit.

Both lenses confirmed all three acceptance items are met by the code, not just by test self-consistency.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/cli/mod.rs`:
- Around line 122-130: Add the required schema_version field to the non-apply
JSON envelope in the init command alongside command and status, using the
documented contract value. Update
test_cli_init_from_code_json_next_actions_match_human_output to assert the
emitted schema_version while preserving the existing next_actions assertions.
🪄 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: CHILL

Plan: Pro Plus

Run ID: cc109bb6-9828-4439-a3b5-a31adc0d24e2

📥 Commits

Reviewing files that changed from the base of the PR and between 4202a97 and e6233b2.

📒 Files selected for processing (6)
  • docs/design-system/copy.toml
  • map.json
  • meta/todos/todo.brownfield-init-review-handoff.md
  • meta/todos/todo.brownfield-one-step-first-map.md
  • src/brownfield/mod.rs
  • src/cli/mod.rs

Comment thread src/cli/mod.rs
@George-RD
George-RD merged commit 3185410 into main Aug 7, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant