-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem
The BMAD code-review workflow (workflows/4-implementation/code-review/instructions.xml) is designed to be adversarial and mandates finding 3-10 issues per review:
<critical>Find 3-10 specific issues in every review minimum - no lazy "looks good" reviews</critical>
...
<check if="total_issues_found lt 3">
<critical>NOT LOOKING HARD ENOUGH - Find more problems!</critical>
<action>Find at least 3 more specific, actionable issues</action>
</check>This incentivizes padding findings with noise to hit the quota. There's no step to validate whether findings are actually meaningful.
Concrete Example
During review of Story 2.1, the workflow flagged:
LOW - File List incomplete: Story File List doesn't include
sprint-status.yamlmodification
This is garbage. sprint-status.yaml is a workflow tracking file that gets modified by every story transition. It's infrastructure, not a story deliverable. Listing it in every story's File List would add zero signal and maximum noise.
The workflow found this "issue" because it was hunting for problems to hit the minimum count, not because it's actually meaningful.
Proposed Fix
Add a validation step after generating findings:
<step n="3b" goal="Validate findings against engineering judgment">
<action>For EACH finding, ask:
1. Would a senior engineer actually care about this?
2. Is this about implementation artifacts or workflow infrastructure?
3. Does fixing this improve the codebase or just satisfy a checklist?
</action>
<action>DISCARD findings that are:
- Workflow/infrastructure noise (tracking files, status updates)
- Pedantic style issues with no functional impact
- "Technically correct but nobody cares" observations
</action>
<action>Only report findings that survive this filter</action>
</step>The adversarial posture is valuable for catching real issues. But without a bullshit filter, it just generates noise.
Location
.bmad/bmm/workflows/4-implementation/code-review/instructions.xml