ldy26098
/
BMAD-METHOD
mirror of https://github.com/bmad-code-org/BMAD-METHOD.git
1
0
Fork 0
Code Packages Projects Releases Wiki Activity

refactor(code-review): simplify triage to decision-needed/patch/defer/dismiss 

Replace 5-bucket classification (intent_gap, bad_spec, patch, defer, reject)
with 4 pragmatic buckets. Findings always written to story file first.
Decision-needed findings gate patch handling — resolve ambiguity before fixing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Verkhovsky 2026-03-19 00:39:45 -06:00
parent 027738121f
commit adafd1187b
2 changed files with 30 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/bmm-skills/4-implementation/bmad-code-review/steps/step-03-triage.md
View File

@ -30,19 +30,18 @@
- Set `source` to the merged sources (e.g., `blind+edge`). - Set `source` to the merged sources (e.g., `blind+edge`).
3. **Classify** each finding into exactly one bucket: 3. **Classify** each finding into exactly one bucket:
- **intent_gap** -- The spec/intent is incomplete; cannot resolve from existing information. Only possible if `{review_mode}` = `"full"`. - **decision_needed** -- There is an ambiguous choice that requires human input. The code cannot be correctly patched without knowing the user's intent. Only possible if `{review_mode}` = `"full"`.
- **bad_spec** -- The spec should have prevented this; spec is wrong or ambiguous. Only possible if `{review_mode}` = `"full"`. - **patch** -- Code issue that is fixable without human input. The correct fix is unambiguous.
- **patch** -- Code issue that is trivially fixable without human input. Just needs a code change.
- **defer** -- Pre-existing issue not caused by the current change. Real but not actionable now. - **defer** -- Pre-existing issue not caused by the current change. Real but not actionable now.
- **reject** -- Noise, false positive, or handled elsewhere. - **dismiss** -- Noise, false positive, or handled elsewhere.
If `{review_mode}` = `"no-spec"` and a finding would otherwise be `intent_gap` or `bad_spec`, reclassify it as `patch` (if code-fixable) or `defer` (if not). If `{review_mode}` = `"no-spec"` and a finding would otherwise be `decision_needed`, reclassify it as `patch` (if the fix is unambiguous) or `defer` (if not).
4. **Drop** all `reject` findings. Record the reject count for the summary. 4. **Drop** all `dismiss` findings. Record the dismiss count for the summary.
5. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping rejects AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review. 5. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping dismissed AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review.
6. If zero findings remain after dropping rejects and no layers failed, note clean review. 6. If zero findings remain after dropping dismissed and no layers failed, note clean review.
## NEXT ## NEXT

58
src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md
View File

@ -7,64 +7,52 @@ deferred_work_file: '{implementation_artifacts}/deferred-work.md'
## RULES ## RULES
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}` - YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
- Deferred findings are handled automatically — no user input needed. - Always write findings to the story file before offering action choices.
- Patch findings require user choice before acting. - Decision-needed findings must be resolved before handling patches.
- Intent gap and bad spec findings require a conversation — suggest options but let the user decide.
## INSTRUCTIONS ## INSTRUCTIONS
### 1. Clean review shortcut ### 1. Clean review shortcut
If zero findings remain after triage (all rejected or none raised): state that and end the workflow. If zero findings remain after triage (all dismissed or none raised): state that and end the workflow.
### 2. Handle deferred findings automatically ### 2. Write findings to the story file
If `defer` findings exist: If `{spec_file}` exists and contains a Tasks/Subtasks section, append a `### Review Findings` subsection. Write all findings in this order:
1. Append each to `{deferred_work_file}` under a heading `## Deferred from: code review ({date})`. If `{spec_file}` is set, include its basename in the heading (e.g., `code review of story-3.3 (2026-03-18)`). One bullet per finding with severity and description. 1. **Decision needed** findings (unchecked):
2. If `{spec_file}` exists and contains a Tasks/Subtasks section, append each as a checked-off item: `- [ ] [Review][Decision] {Title} — {Detail}`
`- [x] [AI-Review][Defer] Description [file:line]`
3. Announce: "**{N} deferred findings** written to deferred-work.md and marked complete in the story file."
### 3. Present remaining findings 2. **Patch** findings (unchecked):
`- [ ] [Review][Patch] {Title} [{file}:{line}]`
Group and present in this order (include a section only if findings exist): 3. **Defer** findings (checked off, marked deferred):
`- [x] [Review][Defer] {Title} [{file}:{line}] — deferred, pre-existing`
- **Intent Gaps**: "These findings suggest the captured intent is incomplete." Also append each `defer` finding to `{deferred_work_file}` under a heading `## Deferred from: code review ({date})`. If `{spec_file}` is set, include its basename in the heading (e.g., `code review of story-3.3 (2026-03-18)`). One bullet per finding with description.
- List each with title + detail.
- **Bad Spec**: "These findings suggest the spec should be amended." ### 3. Present summary
- List each with title + detail + suggested spec amendment.
- **Patch**: "These are fixable code issues:" Announce what was written:
- List each with title + detail + location (if available).
Summary line: **X** intent_gap, **Y** bad_spec, **Z** patch, **W** defer (auto-handled), **R** rejected as noise. > **Code review complete.** {D} decision-needed, {P} patch, {W} deferred, {R} dismissed as noise.
> Findings written to the review findings section in `{spec_file}`.
### 4. Handle intent gap and bad spec findings ### 4. Resolve decision-needed findings
If `intent_gap` or `bad_spec` findings exist, initiate a conversation: If `decision_needed` findings exist, present each one with its detail and the options available. The user must decide — the correct fix is ambiguous without their input. Walk through each finding (or batch related ones) and get the user's call. Once resolved, each becomes a `patch`, `defer`, or is dismissed.
1. Present each finding with its detail and the specific spec section it relates to.
2. For each finding (or as a batch if they relate to the same spec section), suggest resolution options:
- **Downgrade to patch** — reclassify as a `patch` finding and handle it with the other patches in step 5. Cheapest option — avoids re-running dev story or create story.
- **Patch the spec** — amend the specific section in `{spec_file}` to address the gap or fix the bad spec language, then continue with implementation.
- **Reset to ready-for-dev** — update the story status back to ready-for-dev so the spec can be reworked and code regenerated from the corrected spec. Most expensive — triggers another full dev cycle.
- **Dismiss** — the finding is not actionable or the spec is correct as-is.
3. For findings that trace upstream of the story file (e.g., a PRD or architecture gap), note this explicitly: "This may originate upstream of the story — consider whether the PRD or architecture docs need a course correction."
4. Let the user decide. Do not auto-apply any spec changes.
### 5. Handle patch findings ### 5. Handle patch findings
If `patch` findings exist (including any downgraded from step 4), ask the user: If `patch` findings exist (including any resolved from step 4), ask the user:
> **How would you like to handle the {Z} patch findings?** > **How would you like to handle the {Z} patch findings?**
> 1. **Fix them automatically** — I will apply fixes now > 1. **Fix them automatically** — I will apply fixes now
> 2. **Create action items** — I will add them to the story file for later > 2. **Leave as action items** — they are already in the story file
> 3. **Show me details** — Let me walk through each one before deciding > 3. **Walk through each** — let me show details before deciding
- **Option 1**: Apply each fix. After all patches are applied, present a summary of changes made. - **Option 1**: Apply each fix. After all patches are applied, present a summary of changes made and check off the items in the story file.
- **Option 2**: Add a "Review Follow-ups (AI)" subsection to the Tasks/Subtasks section of `{spec_file}`. For each finding: `- [ ] [AI-Review][{Severity}] {Description} [{file}:{line}]` - **Option 2**: Done — findings are already written to the story.
- **Option 3**: Present each finding with full detail, diff context, and suggested fix. After walkthrough, re-offer options 1 and 2. - **Option 3**: Present each finding with full detail, diff context, and suggested fix. After walkthrough, re-offer options 1 and 2.
Workflow complete. Workflow complete.