From 460c27e29a562a787d1ea51b565e11df8d7cee5f Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 01:07:58 -0800 Subject: [PATCH 01/43] refactor(code-review): convert to sharded format with dual-phase review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert monolithic code-review workflow to step-file architecture: - workflow.md: Overview and initialization - step-01: Load story and discover git changes - step-02: Build review attack plan - step-03: Context-aware review (validates ACs, audits tasks) - step-04: Adversarial review (information-asymmetric diff review) - step-05: Consolidate findings (merge + deduplicate) - step-06: Resolve findings and update status Key features: - Dual-phase review: context-aware + context-independent adversarial - Information asymmetry: adversarial reviewer sees only diff, no story - Uses review-adversarial-general.xml via subagent (with fallbacks) - Findings consolidation with severity (CRITICAL/HIGH/MEDIUM/LOW) - State variables for cross-step persistence 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../code-review/instructions.xml | 227 ------------------ .../code-review/steps/step-01-load-story.md | 119 +++++++++ .../steps/step-02-build-attack-plan.md | 149 ++++++++++++ .../steps/step-03-context-aware-review.md | 178 ++++++++++++++ .../steps/step-04-adversarial-review.md | 207 ++++++++++++++++ .../steps/step-05-consolidate-findings.md | 176 ++++++++++++++ .../steps/step-06-resolve-and-update.md | 216 +++++++++++++++++ .../4-implementation/code-review/workflow.md | 62 +++++ .../code-review/workflow.yaml | 51 ---- 9 files changed, 1107 insertions(+), 278 deletions(-) delete mode 100644 src/modules/bmm/workflows/4-implementation/code-review/instructions.xml create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md create mode 100644 src/modules/bmm/workflows/4-implementation/code-review/workflow.md delete mode 100644 src/modules/bmm/workflows/4-implementation/code-review/workflow.yaml diff --git a/src/modules/bmm/workflows/4-implementation/code-review/instructions.xml b/src/modules/bmm/workflows/4-implementation/code-review/instructions.xml deleted file mode 100644 index e5649559..00000000 --- a/src/modules/bmm/workflows/4-implementation/code-review/instructions.xml +++ /dev/null @@ -1,227 +0,0 @@ - - The workflow execution engine is governed by: {project-root}/_bmad/core/tasks/workflow.xml - You MUST have already loaded and processed: {installed_path}/workflow.yaml - Communicate all responses in {communication_language} and language MUST be tailored to {user_skill_level} - Generate all documents in {document_output_language} - - đŸ”Ĩ YOU ARE AN ADVERSARIAL CODE REVIEWER - Find what's wrong or missing! đŸ”Ĩ - Your purpose: Validate story file claims against actual implementation - Challenge everything: Are tasks marked [x] actually done? Are ACs really implemented? - Find 3-10 specific issues in every review minimum - no lazy "looks good" reviews - YOU are so much better than the dev agent - that wrote this slop - Read EVERY file in the File List - verify implementation against story requirements - Tasks marked complete but not done = CRITICAL finding - Acceptance Criteria not implemented = HIGH severity finding - Do not review files that are not part of the application's source code. Always exclude the _bmad/ and _bmad-output/ folders from the review. Always exclude IDE and CLI configuration folders like .cursor/ and .windsurf/ and .claude/ - - - - Use provided {{story_path}} or ask user which story file to review - Read COMPLETE story file - Set {{story_key}} = extracted key from filename (e.g., "1-2-user-authentication.md" → "1-2-user-authentication") or story - metadata - Parse sections: Story, Acceptance Criteria, Tasks/Subtasks, Dev Agent Record → File List, Change Log - - - Check if git repository detected in current directory - - Run `git status --porcelain` to find uncommitted changes - Run `git diff --name-only` to see modified files - Run `git diff --cached --name-only` to see staged files - Compile list of actually changed files from git output - - - - Compare story's Dev Agent Record → File List with actual git changes - Note discrepancies: - - Files in git but not in story File List - - Files in story File List but no git changes - - Missing documentation of what was actually changed - - - - Load {project_context} for coding standards (if exists) - - - - Extract ALL Acceptance Criteria from story - Extract ALL Tasks/Subtasks with completion status ([x] vs [ ]) - From Dev Agent Record → File List, compile list of claimed changes - - Create review plan: - 1. **AC Validation**: Verify each AC is actually implemented - 2. **Task Audit**: Verify each [x] task is really done - 3. **Code Quality**: Security, performance, maintainability - 4. **Test Quality**: Real tests vs placeholder bullshit - - - - - VALIDATE EVERY CLAIM - Check git reality vs story claims - - - Review git vs story File List discrepancies: - 1. **Files changed but not in story File List** → MEDIUM finding (incomplete documentation) - 2. **Story lists files but no git changes** → HIGH finding (false claims) - 3. **Uncommitted changes not documented** → MEDIUM finding (transparency issue) - - - - Create comprehensive review file list from story File List and git changes - - - For EACH Acceptance Criterion: - 1. Read the AC requirement - 2. Search implementation files for evidence - 3. Determine: IMPLEMENTED, PARTIAL, or MISSING - 4. If MISSING/PARTIAL → HIGH SEVERITY finding - - - - For EACH task marked [x]: - 1. Read the task description - 2. Search files for evidence it was actually done - 3. **CRITICAL**: If marked [x] but NOT DONE → CRITICAL finding - 4. Record specific proof (file:line) - - - - For EACH file in comprehensive review list: - 1. **Security**: Look for injection risks, missing validation, auth issues - 2. **Performance**: N+1 queries, inefficient loops, missing caching - 3. **Error Handling**: Missing try/catch, poor error messages - 4. **Code Quality**: Complex functions, magic numbers, poor naming - 5. **Test Quality**: Are tests real assertions or placeholders? - - - - NOT LOOKING HARD ENOUGH - Find more problems! - Re-examine code for: - - Edge cases and null handling - - Architecture violations - - Documentation gaps - - Integration issues - - Dependency problems - - Git commit message quality (if applicable) - - Find at least 3 more specific, actionable issues - - - - - Categorize findings: HIGH (must fix), MEDIUM (should fix), LOW (nice to fix) - Set {{fixed_count}} = 0 - Set {{action_count}} = 0 - - **đŸ”Ĩ CODE REVIEW FINDINGS, {user_name}!** - - **Story:** {{story_file}} - **Git vs Story Discrepancies:** {{git_discrepancy_count}} found - **Issues Found:** {{high_count}} High, {{medium_count}} Medium, {{low_count}} Low - - ## 🔴 CRITICAL ISSUES - - Tasks marked [x] but not actually implemented - - Acceptance Criteria not implemented - - Story claims files changed but no git evidence - - Security vulnerabilities - - ## 🟡 MEDIUM ISSUES - - Files changed but not documented in story File List - - Uncommitted changes not tracked - - Performance problems - - Poor test coverage/quality - - Code maintainability issues - - ## đŸŸĸ LOW ISSUES - - Code style improvements - - Documentation gaps - - Git commit message quality - - - What should I do with these issues? - - 1. **Fix them automatically** - I'll update the code and tests - 2. **Create action items** - Add to story Tasks/Subtasks for later - 3. **Show me details** - Deep dive into specific issues - - Choose [1], [2], or specify which issue to examine: - - - Fix all HIGH and MEDIUM issues in the code - Add/update tests as needed - Update File List in story if files changed - Update story Dev Agent Record with fixes applied - Set {{fixed_count}} = number of HIGH and MEDIUM issues fixed - Set {{action_count}} = 0 - - - - Add "Review Follow-ups (AI)" subsection to Tasks/Subtasks - For each issue: `- [ ] [AI-Review][Severity] Description [file:line]` - Set {{action_count}} = number of action items created - Set {{fixed_count}} = 0 - - - - Show detailed explanation with code examples - Return to fix decision - - - - - - - Set {{new_status}} = "done" - Update story Status field to "done" - - - Set {{new_status}} = "in-progress" - Update story Status field to "in-progress" - - Save story file - - - - Set {{current_sprint_status}} = "enabled" - - - Set {{current_sprint_status}} = "no-sprint-tracking" - - - - - Load the FULL file: {sprint_status} - Find development_status key matching {{story_key}} - - - Update development_status[{{story_key}}] = "done" - Save file, preserving ALL comments and structure - ✅ Sprint status synced: {{story_key}} → done - - - - Update development_status[{{story_key}}] = "in-progress" - Save file, preserving ALL comments and structure - 🔄 Sprint status synced: {{story_key}} → in-progress - - - - ⚠ī¸ Story file updated, but sprint-status sync failed: {{story_key}} not found in sprint-status.yaml - - - - - ℹī¸ Story status updated (no sprint tracking configured) - - - **✅ Review Complete!** - - **Story Status:** {{new_status}} - **Issues Fixed:** {{fixed_count}} - **Action Items Created:** {{action_count}} - - {{#if new_status == "done"}}Code review complete!{{else}}Address the action items and continue development.{{/if}} - - - - \ No newline at end of file diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md new file mode 100644 index 00000000..50660b45 --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -0,0 +1,119 @@ +--- +name: 'step-01-load-story' +description: 'Load story file, discover git changes, establish review context' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-01-load-story.md' +nextStepFile: '{workflow_path}/steps/step-02-build-attack-plan.md' +--- + +# Step 1: Load Story and Discover Changes + +**Goal:** Load story file, extract all sections, discover actual git changes for comparison. + +--- + +## STATE VARIABLES (capture now, persist throughout) + +These variables MUST be set in this step and available to all subsequent steps: + +- `{story_path}` - Path to the story file being reviewed +- `{story_key}` - Story identifier (e.g., "1-2-user-authentication") +- `{story_file_list}` - Files claimed in story's Dev Agent Record → File List +- `{git_changed_files}` - Files actually changed according to git +- `{git_discrepancies}` - Differences between story claims and git reality + +--- + +## EXECUTION SEQUENCE + +### 1. Identify Story + +**If `{story_path}` provided by user:** + +- Use the provided path directly + +**If NOT provided:** + +- Ask user which story file to review +- Wait for response before proceeding + +### 2. Load Story File + +- Read COMPLETE story file from `{story_path}` +- Extract `{story_key}` from filename (e.g., "1-2-user-authentication.md" → "1-2-user-authentication") or story metadata + +### 3. Parse Story Sections + +Extract and store: + +- **Story**: Title, description, status +- **Acceptance Criteria**: All ACs with their requirements +- **Tasks/Subtasks**: All tasks with completion status ([x] vs [ ]) +- **Dev Agent Record → File List**: Claimed file changes +- **Change Log**: History of modifications + +Set `{story_file_list}` = list of files from Dev Agent Record → File List + +### 4. Discover Git Changes + +Check if git repository exists: + +**If Git repo detected:** + +```bash +git status --porcelain +git diff --name-only +git diff --cached --name-only +``` + +Compile `{git_changed_files}` = union of modified, staged, and new files + +**If NOT a Git repo:** + +Set `{git_changed_files}` = "NO_GIT" + +### 5. Cross-Reference Story vs Git + +Compare `{story_file_list}` with `{git_changed_files}`: + +Set `{git_discrepancies}` with categories: + +- **files_in_git_not_story**: Files changed in git but not in story File List +- **files_in_story_not_git**: Files in story File List but no git changes +- **uncommitted_undocumented**: Uncommitted changes not tracked in story + +### 6. Load Context + +- Load `{project_context}` if exists (`**/project-context.md`) +- Invoke `discover_inputs` protocol to load architecture, UX, epic docs as needed + +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-02-build-attack-plan.md`" + +--- + +## SUCCESS METRICS + +- `{story_path}` identified and loaded +- `{story_key}` extracted +- All story sections parsed +- `{story_file_list}` compiled from Dev Agent Record +- `{git_changed_files}` discovered via git commands +- `{git_discrepancies}` calculated +- Context documents loaded +- Explicit NEXT directive provided + +## FAILURE MODES + +- Proceeding without story file loaded +- Missing `{story_key}` extraction +- Not parsing all story sections +- Skipping git change discovery +- Not calculating discrepancies +- No explicit NEXT directive at step completion diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md new file mode 100644 index 00000000..3b442899 --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md @@ -0,0 +1,149 @@ +--- +name: 'step-02-build-attack-plan' +description: 'Extract ACs and tasks, create comprehensive review plan for both phases' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-02-build-attack-plan.md' +nextStepFile: '{workflow_path}/steps/step-03-context-aware-review.md' +--- + +# Step 2: Build Review Attack Plan + +**Goal:** Extract all reviewable items from story and create attack plan for both review phases. + +--- + +## AVAILABLE STATE + +From previous step: + +- `{story_path}` - Path to the story file +- `{story_key}` - Story identifier +- `{story_file_list}` - Files claimed in story +- `{git_changed_files}` - Files actually changed (git) +- `{git_discrepancies}` - Differences between claims and reality + +--- + +## STATE VARIABLES (capture now) + +- `{acceptance_criteria}` - All ACs extracted from story +- `{tasks_with_status}` - All tasks with their [x] or [ ] status +- `{comprehensive_file_list}` - Union of story files + git files +- `{review_attack_plan}` - Structured plan for both phases + +--- + +## EXECUTION SEQUENCE + +### 1. Extract Acceptance Criteria + +Parse all Acceptance Criteria from story: + +``` +{acceptance_criteria} = [ + { id: "AC1", requirement: "...", testable: true/false }, + { id: "AC2", requirement: "...", testable: true/false }, + ... +] +``` + +Note any ACs that are vague or untestable. + +### 2. Extract Tasks with Status + +Parse all Tasks/Subtasks with completion markers: + +``` +{tasks_with_status} = [ + { id: "T1", description: "...", status: "complete" ([x]) or "incomplete" ([ ]) }, + { id: "T1.1", description: "...", status: "complete" or "incomplete" }, + ... +] +``` + +Flag any tasks marked complete [x] for verification. + +### 3. Build Comprehensive File List + +Merge `{story_file_list}` and `{git_changed_files}`: + +``` +{comprehensive_file_list} = union of: + - Files in story Dev Agent Record + - Files changed according to git + - Deduped and sorted +``` + +Exclude from review: + +- `_bmad/`, `_bmad-output/` +- `.cursor/`, `.windsurf/`, `.claude/` +- IDE/editor config files + +### 4. Create Review Attack Plan + +Structure the `{review_attack_plan}`: + +``` +PHASE 1: Context-Aware Review (Step 3) +├── Git vs Story Discrepancies +│ └── {git_discrepancies} items +├── AC Validation +│ └── {acceptance_criteria} items to verify +├── Task Completion Audit +│ └── {tasks_with_status} marked [x] to verify +└── Code Quality Review + └── {comprehensive_file_list} files to review + +PHASE 2: Adversarial Asymmetric Review (Step 4) +└── Context-independent diff review + └── Diff constructed from git changes + └── Story file EXCLUDED from reviewer context + └── Pure code quality assessment +``` + +### 5. Preview Attack Plan + +Present to user (brief summary): + +``` +**Review Attack Plan** + +**Story:** {story_key} +**ACs to verify:** {count} +**Tasks marked complete:** {count} +**Files to review:** {count} +**Git discrepancies detected:** {count} + +Proceeding with dual-phase review... +``` + +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-03-context-aware-review.md`" + +--- + +## SUCCESS METRICS + +- All ACs extracted with testability assessment +- All tasks extracted with completion status +- Comprehensive file list built (story + git) +- Exclusions applied correctly +- Attack plan structured for both phases +- Summary presented to user +- Explicit NEXT directive provided + +## FAILURE MODES + +- Missing AC extraction +- Not capturing task completion status +- Forgetting to merge story + git files +- Not excluding IDE/config directories +- Skipping attack plan structure +- No explicit NEXT directive at step completion diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md new file mode 100644 index 00000000..a36589e1 --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md @@ -0,0 +1,178 @@ +--- +name: 'step-03-context-aware-review' +description: 'Story-aware validation: verify ACs, audit task completion, check git discrepancies' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-03-context-aware-review.md' +nextStepFile: '{workflow_path}/steps/step-04-adversarial-review.md' +--- + +# Step 3: Context-Aware Review + +**Goal:** Perform story-aware validation - verify AC implementation, audit task completion, review code quality with full story context. + +VALIDATE EVERY CLAIM - Check git reality vs story claims +You KNOW the story requirements - use that knowledge to find gaps + +--- + +## AVAILABLE STATE + +From previous steps: + +- `{story_path}`, `{story_key}` +- `{story_file_list}`, `{git_changed_files}`, `{git_discrepancies}` +- `{acceptance_criteria}`, `{tasks_with_status}` +- `{comprehensive_file_list}`, `{review_attack_plan}` + +--- + +## STATE VARIABLE (capture now) + +- `{context_aware_findings}` - All findings from this phase + +Initialize `{context_aware_findings}` as empty list. + +--- + +## EXECUTION SEQUENCE + +### 1. Git vs Story Discrepancies + +Review `{git_discrepancies}` and create findings: + +| Discrepancy Type | Severity | +| --- | --- | +| Files changed but not in story File List | MEDIUM | +| Story lists files but no git changes | HIGH | +| Uncommitted changes not documented | MEDIUM | + +For each discrepancy, add to `{context_aware_findings}`: + +``` +{ + id: "CAF-{n}", + source: "git-discrepancy", + severity: "...", + description: "...", + evidence: "file: X, git says: Y, story says: Z" +} +``` + +### 2. Acceptance Criteria Validation + +For EACH AC in `{acceptance_criteria}`: + +1. Read the AC requirement +2. Search implementation files in `{comprehensive_file_list}` for evidence +3. Determine status: IMPLEMENTED, PARTIAL, or MISSING +4. If PARTIAL or MISSING → add HIGH severity finding + +Add to `{context_aware_findings}`: + +``` +{ + id: "CAF-{n}", + source: "ac-validation", + severity: "HIGH", + description: "AC {id} not fully implemented: {details}", + evidence: "Expected: {ac}, Found: {what_was_found}" +} +``` + +### 3. Task Completion Audit + +For EACH task marked [x] in `{tasks_with_status}`: + +1. Read the task description +2. Search files for evidence it was actually done +3. **CRITICAL**: If marked [x] but NOT DONE → CRITICAL finding +4. Record specific proof (file:line) if done + +Add to `{context_aware_findings}` if false: + +``` +{ + id: "CAF-{n}", + source: "task-audit", + severity: "CRITICAL", + description: "Task marked complete but not implemented: {task}", + evidence: "Searched: {files}, Found: no evidence of {expected}" +} +``` + +### 4. Code Quality Review (Context-Aware) + +For EACH file in `{comprehensive_file_list}`: + +Review with STORY CONTEXT (you know what was supposed to be built): + +- **Security**: Missing validation for AC-specified inputs? +- **Performance**: Story mentioned scale requirements met? +- **Error Handling**: Edge cases from AC covered? +- **Test Quality**: Tests actually verify ACs or just placeholders? +- **Architecture Compliance**: Follows patterns in architecture doc? + +Add findings to `{context_aware_findings}` with appropriate severity. + +### 5. Minimum Finding Check + +If total findings < 3, NOT LOOKING HARD ENOUGH + +Re-examine for: + +- Edge cases not covered by implementation +- Documentation gaps +- Integration issues with other components +- Dependency problems +- Comments missing for complex logic + +--- + +## PHASE 1 SUMMARY + +Present context-aware findings: + +``` +**Phase 1: Context-Aware Review Complete** + +**Findings:** {count} +- CRITICAL: {count} +- HIGH: {count} +- MEDIUM: {count} +- LOW: {count} + +Proceeding to Phase 2: Adversarial Review... +``` + +Store `{context_aware_findings}` for consolidation in step 5. + +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-04-adversarial-review.md`" + +--- + +## SUCCESS METRICS + +- All git discrepancies reviewed and findings created +- Every AC checked for implementation evidence +- Every [x] task verified with proof +- Code quality reviewed with story context +- Minimum 3 findings (push harder if not) +- `{context_aware_findings}` populated +- Phase summary presented +- Explicit NEXT directive provided + +## FAILURE MODES + +- Accepting "looks good" with < 3 findings +- Not verifying [x] tasks with actual evidence +- Missing AC validation +- Ignoring git discrepancies +- Not storing findings for consolidation +- No explicit NEXT directive at step completion diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md new file mode 100644 index 00000000..408fff56 --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -0,0 +1,207 @@ +--- +name: 'step-04-adversarial-review' +description: 'Context-independent adversarial diff review via subagent - no story knowledge' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-04-adversarial-review.md' +nextStepFile: '{workflow_path}/steps/step-05-consolidate-findings.md' +--- + +# Step 4: Adversarial Review (Information Asymmetric) + +**Goal:** Perform context-independent adversarial review of code changes. Reviewer sees ONLY the diff - no story, no ACs, no context about WHY changes were made. + +Reviewer has FULL repo access but NO knowledge of WHY changes were made +DO NOT include story file in prompt - asymmetry is about intent, not visibility +This catches issues a fresh reviewer would find that story-biased review might miss + +--- + +## AVAILABLE STATE + +From previous steps: + +- `{story_path}`, `{story_key}` +- `{git_changed_files}` - Files changed according to git +- `{context_aware_findings}` - Findings from Phase 1 + +--- + +## STATE VARIABLE (capture now) + +- `{diff_output}` - Complete diff of changes +- `{asymmetric_findings}` - Findings from adversarial review + +--- + +## EXECUTION SEQUENCE + +### 1. Construct Diff + +Build complete diff of all changes for this story. + +**Determine diff source:** + +If uncommitted changes exist for story files: + +```bash +git diff +git diff --cached +``` + +If story work is already committed, find story-related commits: + +```bash +# Find commits that reference this story +git log --oneline --all --grep="{story_key}" --format="%H" +# Or find recent commits touching story files +git log --oneline -10 -- {story_file_list} +``` + +Then construct diff: + +```bash +git diff {first_story_commit}^..HEAD -- {files} +``` + +**Include in `{diff_output}`:** + +- All modified tracked files +- All new files created for this story +- Full content for new files + +**EXCLUDE from `{diff_output}`:** + +- The story file itself (`{story_path}`) - CRITICAL for asymmetry +- `_bmad/`, `_bmad-output/` directories +- `.cursor/`, `.windsurf/`, `.claude/` directories + +### 2. Invoke Adversarial Review + +Use information asymmetry: separate context from review + +**Execution Hierarchy (try in order):** + +**Option A: Subagent (Preferred)** + +If Task tool available with subagent capability: + +```xml + + Review {diff_output} using {project-root}/_bmad/core/tasks/review-adversarial-general.xml + +``` + +The subagent: + +- Has FULL read access to the repository +- Receives ONLY `{diff_output}` as context +- Does NOT know story requirements, ACs, or intent +- Reviews code purely on technical merit + +**Option B: CLI Fallback** + +If subagent not available but CLI available: + +```bash +# Pipe diff to adversarial review task +cat {diff_file} | claude --task {adversarial_review_task} +``` + +**Option C: Inline Execution** + +If neither available, load `review-adversarial-general.xml` and execute inline: + +1. Load task file +2. Adopt adversarial persona +3. Review `{diff_output}` with zero story context +4. Generate findings + +### 3. Process Adversarial Findings + +Capture findings from adversarial review. + +**If zero findings returned:** + +HALT - Zero findings is suspicious. Re-analyze or ask for guidance. + +**For each finding:** + +Assign severity: + +- CRITICAL: Security vulnerabilities, data loss risks +- HIGH: Logic errors, missing error handling +- MEDIUM: Performance issues, code smells +- LOW: Style, documentation + +Assign validity: + +- REAL: Genuine issue to address +- NOISE: False positive (explain why) +- UNDECIDED: Needs human judgment + +Create `{asymmetric_findings}` list: + +``` +{ + id: "AAF-{n}", + source: "adversarial-review", + severity: "...", + validity: "...", + description: "...", + location: "file:line (if applicable)" +} +``` + +### 4. Phase 2 Summary + +Present adversarial findings: + +``` +**Phase 2: Adversarial Review Complete** + +**Reviewer Context:** Pure diff review (no story knowledge) +**Findings:** {count} +- CRITICAL: {count} +- HIGH: {count} +- MEDIUM: {count} +- LOW: {count} + +**Validity Assessment:** +- Real issues: {count} +- Noise/false positives: {count} +- Needs judgment: {count} + +Proceeding to findings consolidation... +``` + +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-05-consolidate-findings.md`" + +--- + +## SUCCESS METRICS + +- Diff constructed from correct source (uncommitted or commits) +- Story file excluded from diff +- Subagent invoked with proper isolation (or fallback used) +- Adversarial review executed +- Findings captured with severity and validity +- `{asymmetric_findings}` populated +- Phase summary presented +- Explicit NEXT directive provided + +## FAILURE MODES + +- Including story file in diff (breaks asymmetry) +- Skipping adversarial review entirely +- Accepting zero findings without halt +- Not using subagent when available +- Missing severity/validity classification +- Not storing findings for consolidation +- No explicit NEXT directive at step completion diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md new file mode 100644 index 00000000..0ae3f418 --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md @@ -0,0 +1,176 @@ +--- +name: 'step-05-consolidate-findings' +description: 'Merge and deduplicate findings from both review phases' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-05-consolidate-findings.md' +nextStepFile: '{workflow_path}/steps/step-06-resolve-and-update.md' +--- + +# Step 5: Consolidate Findings + +**Goal:** Merge findings from context-aware review (Phase 1) and adversarial review (Phase 2), deduplicate, and present unified findings table. + +--- + +## AVAILABLE STATE + +From previous steps: + +- `{story_path}`, `{story_key}` +- `{context_aware_findings}` - Findings from Phase 1 (step 3) +- `{asymmetric_findings}` - Findings from Phase 2 (step 4) + +--- + +## STATE VARIABLE (capture now) + +- `{consolidated_findings}` - Merged, deduplicated findings + +--- + +## EXECUTION SEQUENCE + +### 1. Merge All Findings + +Combine both finding lists: + +``` +all_findings = {context_aware_findings} + {asymmetric_findings} +``` + +### 2. Deduplicate Findings + +Identify duplicates (same underlying issue found by both phases): + +**Duplicate Detection Criteria:** + +- Same file + same line range +- Same issue type (e.g., both about error handling in same function) +- Overlapping descriptions + +**Resolution Rule:** + +Keep the MORE DETAILED version: + +- If context-aware finding has AC reference → keep that +- If adversarial finding has better technical detail → keep that +- When in doubt, keep context-aware (has more context) + +Mark duplicates as merged: + +``` +{ + id: "CF-{n}", + merged_from: ["CAF-3", "AAF-2"], + kept_version: "CAF-3", + reason: "Context-aware version includes AC reference" +} +``` + +### 3. Normalize Severity + +Apply consistent severity scale: + +| Severity | Icon | Criteria | +| --- | --- | --- | +| CRITICAL | RED | Security vuln, data loss, tasks marked done but not, broken core functionality | +| HIGH | ORANGE | Missing AC implementation, logic errors, missing critical error handling | +| MEDIUM | YELLOW | Performance issues, incomplete features, documentation gaps | +| LOW | GREEN | Code style, minor improvements, suggestions | + +### 4. Filter Noise + +Review adversarial findings marked as NOISE: + +- If clearly false positive (e.g., style preference, not actual issue) → exclude +- If questionable → keep with UNDECIDED validity +- If context reveals it's actually valid → upgrade to REAL + +**Do NOT filter:** + +- Any CRITICAL or HIGH severity +- Any context-aware findings (they have story context) + +### 5. Create Consolidated Table + +Build `{consolidated_findings}`: + +```markdown +| ID | Severity | Source | Description | Location | +|----|----------|--------|-------------|----------| +| CF-1 | CRITICAL | task-audit | Task 3 marked [x] but not implemented | src/auth.ts | +| CF-2 | HIGH | ac-validation | AC2 partially implemented | src/api/*.ts | +| CF-3 | HIGH | adversarial | Missing error handling in API calls | src/api/client.ts:45 | +| CF-4 | MEDIUM | git-discrepancy | File changed but not in story | src/utils.ts | +| CF-5 | LOW | adversarial | Magic number should be constant | src/config.ts:12 | +``` + +### 6. Present Consolidated Findings + +```markdown +**Consolidated Code Review Findings** + +**Story:** {story_key} + +**Summary:** +- Total findings: {count} +- CRITICAL: {count} +- HIGH: {count} +- MEDIUM: {count} +- LOW: {count} + +**Deduplication:** {merged_count} duplicate findings merged + +--- + +## Findings by Severity + +### CRITICAL (Must Fix) +{list critical findings with full details} + +### HIGH (Should Fix) +{list high findings with full details} + +### MEDIUM (Consider Fixing) +{list medium findings} + +### LOW (Nice to Fix) +{list low findings} + +--- + +**Phase Sources:** +- Context-Aware (Phase 1): {count} findings +- Adversarial (Phase 2): {count} findings +``` + +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-06-resolve-and-update.md`" + +--- + +## SUCCESS METRICS + +- All findings merged from both phases +- Duplicates identified and resolved (kept more detailed) +- Severity normalized consistently +- Noise filtered appropriately (but not excessively) +- Consolidated table created +- `{consolidated_findings}` populated +- Summary presented to user +- Explicit NEXT directive provided + +## FAILURE MODES + +- Missing findings from either phase +- Not detecting duplicates (double-counting issues) +- Inconsistent severity assignment +- Filtering real issues as noise +- Not storing consolidated findings +- No explicit NEXT directive at step completion diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md new file mode 100644 index 00000000..9b2c51bf --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md @@ -0,0 +1,216 @@ +--- +name: 'step-06-resolve-and-update' +description: 'Present findings, fix or create action items, update story and sprint status' + +workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' +thisStepFile: '{workflow_path}/steps/step-06-resolve-and-update.md' +--- + +# Step 6: Resolve Findings and Update Status + +**Goal:** Present findings to user, handle resolution (fix or action items), update story file and sprint status. + +--- + +## AVAILABLE STATE + +From previous steps: + +- `{story_path}`, `{story_key}` +- `{consolidated_findings}` - Merged findings from step 5 +- `sprint_status` = `{implementation_artifacts}/sprint-status.yaml` + +--- + +## STATE VARIABLES (capture now) + +- `{fixed_count}` - Number of issues fixed +- `{action_count}` - Number of action items created +- `{new_status}` - Final story status + +--- + +## EXECUTION SEQUENCE + +### 1. Present Resolution Options + +```markdown +**Code Review Findings for {user_name}** + +**Story:** {story_key} +**Total Issues:** {consolidated_findings.count} + +{consolidated_findings_table} + +--- + +**What should I do with these issues?** + +**[1] Fix them automatically** - I'll update the code and tests +**[2] Create action items** - Add to story Tasks/Subtasks for later +**[3] Walk through** - Discuss each finding individually +**[4] Show details** - Deep dive into specific issues + +Choose [1], [2], [3], [4], or specify which issue (e.g., "CF-3"): +``` + +### 2. Handle User Choice + +**Option [1]: Fix Automatically** + +1. For each CRITICAL and HIGH finding: + - Apply the fix in the code + - Add/update tests if needed + - Record what was fixed +2. Update story Dev Agent Record → File List if files changed +3. Add "Code Review Fixes Applied" entry to Change Log +4. Set `{fixed_count}` = number of issues fixed +5. Set `{action_count}` = 0 (LOW findings can become action items) + +**Option [2]: Create Action Items** + +1. Add "Review Follow-ups (AI)" subsection to Tasks/Subtasks +2. For each finding: + ``` + - [ ] [AI-Review][{severity}] {description} [{location}] + ``` +3. Set `{action_count}` = number of action items created +4. Set `{fixed_count}` = 0 + +**Option [3]: Walk Through** + +For each finding in order: + +1. Present finding with full context and code snippet +2. Ask: **[f]ix now / [s]kip / [d]iscuss more** +3. If fix: Apply fix immediately, increment `{fixed_count}` +4. If skip: Note as acknowledged, optionally create action item +5. If discuss: Provide more detail, repeat choice +6. Continue to next finding + +After all processed, summarize what was fixed/skipped. + +**Option [4]: Show Details** + +1. Present expanded details for specific finding(s) +2. Return to resolution choice + +### 3. Determine Final Status + +Evaluate completion: + +**If ALL conditions met:** + +- All CRITICAL issues fixed +- All HIGH issues fixed or have action items +- All ACs verified as implemented + +Set `{new_status}` = "done" + +**Otherwise:** + +Set `{new_status}` = "in-progress" + +### 4. Update Story File + +1. Update story Status field to `{new_status}` +2. Add review notes to Dev Agent Record: + +```markdown +## Senior Developer Review (AI) + +**Date:** {date} +**Reviewer:** AI Code Review + +**Findings Summary:** +- CRITICAL: {count} ({fixed}/{action_items}) +- HIGH: {count} ({fixed}/{action_items}) +- MEDIUM: {count} +- LOW: {count} + +**Resolution:** {approach_taken} + +**Files Modified:** {list if fixes applied} +``` + +3. Update Change Log: + +```markdown +- [{date}] Code review completed - {outcome_summary} +``` + +4. Save story file + +### 5. Sync Sprint Status + +Check if `{sprint_status}` file exists: + +**If exists:** + +1. Load `{sprint_status}` +2. Find `{story_key}` in development_status +3. Update status to `{new_status}` +4. Save file, preserving ALL comments and structure + +``` + Sprint status synced: {story_key} {new_status} +``` + +**If not exists or key not found:** + +``` + Sprint status sync skipped (no sprint tracking or key not found) +``` + +### 6. Completion Output + +```markdown +** Code Review Complete!** + +**Story:** {story_key} +**Final Status:** {new_status} +**Issues Fixed:** {fixed_count} +**Action Items Created:** {action_count} + +{if new_status == "done"} +Code review passed! Story is ready for final verification. +{else} +Address the action items and run another review cycle. +{endif} + +--- + +**Next Steps:** +- Commit changes (if fixes applied) +- Run tests to verify fixes +- Address remaining action items (if any) +- Mark story complete when all items resolved +``` + +--- + +## WORKFLOW COMPLETE + +This is the final step. The Code Review workflow is now complete. + +--- + +## SUCCESS METRICS + +- Resolution options presented clearly +- User choice handled correctly +- Fixes applied cleanly (if chosen) +- Action items created correctly (if chosen) +- Story status determined correctly +- Story file updated with review notes +- Sprint status synced (if applicable) +- Completion summary provided + +## FAILURE MODES + +- Not presenting resolution options +- Fixing without user consent +- Not updating story file +- Wrong status determination (done when issues remain) +- Not syncing sprint status when it exists +- Missing completion summary diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md new file mode 100644 index 00000000..46c3bdeb --- /dev/null +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -0,0 +1,62 @@ +--- +name: code-review +description: 'Perform an ADVERSARIAL Senior Developer code review with dual-phase analysis: context-aware story validation plus context-independent adversarial diff review. Finds 3-10 specific problems in every story. NEVER accepts "looks good" - must find minimum issues and can auto-fix with user approval.' +--- + +# Code Review Workflow + +**Goal:** Execute a comprehensive two-phase code review that validates story claims AND performs context-independent adversarial analysis. + +**Your Role:** You are an elite senior developer performing adversarial review. Challenge everything. Verify claims against reality. Find what's wrong or missing. + +--- + +## WORKFLOW ARCHITECTURE + +This uses **step-file architecture** for focused execution: + +- Each step loads fresh to combat "lost in the middle" +- State persists via variables: `{story_path}`, `{story_key}`, `{context_aware_findings}`, `{asymmetric_findings}` +- Dual-phase review: context-aware (step 3) + adversarial asymmetric (step 4) + +--- + +## INITIALIZATION + +### Configuration Loading + +Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: + +- `user_name`, `communication_language`, `user_skill_level`, `document_output_language` +- `planning_artifacts`, `implementation_artifacts` +- `date` as system-generated current datetime + +### Paths + +- `installed_path` = `{project-root}/_bmad/bmm/workflows/4-implementation/code-review` +- `project_context` = `**/project-context.md` (load if exists) +- `sprint_status` = `{implementation_artifacts}/sprint-status.yaml` +- `validation` = `{installed_path}/checklist.md` + +### Related Tasks + +- `adversarial_review_task` = `{project-root}/_bmad/core/tasks/review-adversarial-general.xml` + +--- + +## CRITICAL DIRECTIVES + +YOU ARE AN ADVERSARIAL CODE REVIEWER - Find what's wrong or missing! +Your purpose: Validate story file claims against actual implementation +Challenge everything: Are tasks marked [x] actually done? Are ACs really implemented? +Find 3-10 specific issues in every review minimum - no lazy "looks good" reviews +Read EVERY file in the File List - verify implementation against story requirements +Tasks marked complete but not done = CRITICAL finding +Acceptance Criteria not implemented = HIGH severity finding +Exclude `_bmad/`, `_bmad-output/`, `.cursor/`, `.windsurf/`, `.claude/` from review + +--- + +## EXECUTION + +Load and execute `steps/step-01-load-story.md` to begin the workflow. diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.yaml b/src/modules/bmm/workflows/4-implementation/code-review/workflow.yaml deleted file mode 100644 index 9e66b932..00000000 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.yaml +++ /dev/null @@ -1,51 +0,0 @@ -# Review Story Workflow -name: code-review -description: "Perform an ADVERSARIAL Senior Developer code review that finds 3-10 specific problems in every story. Challenges everything: code quality, test coverage, architecture compliance, security, performance. NEVER accepts `looks good` - must find minimum issues and can auto-fix with user approval." -author: "BMad" - -# Critical variables from config -config_source: "{project-root}/_bmad/bmm/config.yaml" -user_name: "{config_source}:user_name" -communication_language: "{config_source}:communication_language" -user_skill_level: "{config_source}:user_skill_level" -document_output_language: "{config_source}:document_output_language" -date: system-generated -planning_artifacts: "{config_source}:planning_artifacts" -implementation_artifacts: "{config_source}:implementation_artifacts" -output_folder: "{implementation_artifacts}" -sprint_status: "{implementation_artifacts}/sprint-status.yaml" - -# Workflow components -installed_path: "{project-root}/_bmad/bmm/workflows/4-implementation/code-review" -instructions: "{installed_path}/instructions.xml" -validation: "{installed_path}/checklist.md" -template: false - -variables: - # Project context - project_context: "**/project-context.md" - story_dir: "{implementation_artifacts}" - -# Smart input file references - handles both whole docs and sharded docs -# Priority: Whole document first, then sharded version -# Strategy: SELECTIVE LOAD - only load the specific epic needed for this story review -input_file_patterns: - architecture: - description: "System architecture for review context" - whole: "{planning_artifacts}/*architecture*.md" - sharded: "{planning_artifacts}/*architecture*/*.md" - load_strategy: "FULL_LOAD" - ux_design: - description: "UX design specification (if UI review)" - whole: "{planning_artifacts}/*ux*.md" - sharded: "{planning_artifacts}/*ux*/*.md" - load_strategy: "FULL_LOAD" - epics: - description: "Epic containing story being reviewed" - whole: "{planning_artifacts}/*epic*.md" - sharded_index: "{planning_artifacts}/*epic*/index.md" - sharded_single: "{planning_artifacts}/*epic*/epic-{{epic_num}}.md" - load_strategy: "SELECTIVE_LOAD" - -standalone: true -web_bundle: false From 8b6a053d2e949ace24b9f278f9b63ac6e22da0b5 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 01:41:20 -0800 Subject: [PATCH 02/43] fix(code-review): simplify diff exclusion to implementation_artifacts only --- .../code-review/steps/step-04-adversarial-review.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index 408fff56..c77a0721 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -66,16 +66,10 @@ git diff {first_story_commit}^..HEAD -- {files} **Include in `{diff_output}`:** -- All modified tracked files +- All modified tracked files (except files in `{implementation_artifacts}` - asymmetry requires hiding intent) - All new files created for this story - Full content for new files -**EXCLUDE from `{diff_output}`:** - -- The story file itself (`{story_path}`) - CRITICAL for asymmetry -- `_bmad/`, `_bmad-output/` directories -- `.cursor/`, `.windsurf/`, `.claude/` directories - ### 2. Invoke Adversarial Review Use information asymmetry: separate context from review From 6d1d7d0e72ede2b4e6df0c50c2e105c7f02170bb Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 02:12:02 -0800 Subject: [PATCH 03/43] fix(adversarial-review): add tech-spec exclusion and read-only notes --- .../code-review/steps/step-04-adversarial-review.md | 2 ++ .../quick-dev/steps/step-05-adversarial-review.md | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index c77a0721..945d3849 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -70,6 +70,8 @@ git diff {first_story_commit}^..HEAD -- {files} - All new files created for this story - Full content for new files +**Note:** Do NOT `git add` anything - this is read-only inspection. + ### 2. Invoke Adversarial Review Use information asymmetry: separate context from review diff --git a/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md b/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md index 2a366dbd..a5c63422 100644 --- a/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md +++ b/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md @@ -51,7 +51,11 @@ Use best-effort diff construction: ### Capture as {diff_output} -Merge all changes into `{diff_output}`. +**Include in `{diff_output}`:** + +- All modified tracked files (except `{tech_spec_path}` if tech-spec mode - asymmetry requires hiding intent) +- All new files created during this workflow +- Full content for new files **Note:** Do NOT `git add` anything - this is read-only inspection. @@ -92,6 +96,7 @@ With findings in hand, load `step-06-resolve-findings.md` for user to choose res ## SUCCESS METRICS - Diff constructed from baseline_commit +- Tech-spec excluded from diff when in tech-spec mode (information asymmetry) - New files included in diff - Task invoked with diff as input - Findings received @@ -100,6 +105,7 @@ With findings in hand, load `step-06-resolve-findings.md` for user to choose res ## FAILURE MODES - Missing baseline_commit (can't construct accurate diff) +- Including tech_spec_path in diff when in tech-spec mode (breaks asymmetry) - Not including new untracked files in diff - Invoking task without providing diff input - Accepting zero findings without questioning From f5d949b922aa980e040c958748e7244299f6d846 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 03:04:56 -0800 Subject: [PATCH 04/43] feat(dev-story): capture baseline commit for code-review diff --- .../steps/step-04-adversarial-review.md | 33 ++++++++++--------- .../dev-story/instructions.xml | 11 +++++++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index 945d3849..1727c33d 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -22,13 +22,14 @@ nextStepFile: '{workflow_path}/steps/step-05-consolidate-findings.md' From previous steps: - `{story_path}`, `{story_key}` -- `{git_changed_files}` - Files changed according to git +- `{file_list}` - Files listed in story's File List section - `{context_aware_findings}` - Findings from Phase 1 --- ## STATE VARIABLE (capture now) +- `{baseline_commit}` - From story file Dev Agent Record - `{diff_output}` - Complete diff of changes - `{asymmetric_findings}` - Findings from adversarial review @@ -40,29 +41,29 @@ From previous steps: Build complete diff of all changes for this story. -**Determine diff source:** +**Step 1a: Read baseline from story file** -If uncommitted changes exist for story files: +Extract `Baseline Commit` from the story file's Dev Agent Record section. + +- If found and not "NO_GIT": use as `{baseline_commit}` +- If "NO_GIT" or missing: proceed to fallback + +**Step 1b: Construct diff (with baseline)** + +If `{baseline_commit}` is a valid commit hash: ```bash -git diff -git diff --cached +git diff {baseline_commit} -- ':!{implementation_artifacts}' ``` -If story work is already committed, find story-related commits: +This captures all changes (committed + uncommitted) since dev-story started. -```bash -# Find commits that reference this story -git log --oneline --all --grep="{story_key}" --format="%H" -# Or find recent commits touching story files -git log --oneline -10 -- {story_file_list} -``` +**Step 1c: Fallback (no baseline)** -Then construct diff: +If no baseline available, review current state of files in `{file_list}`: -```bash -git diff {first_story_commit}^..HEAD -- {files} -``` +- Read each file listed in the story's File List section +- Review as full file content (not a diff) **Include in `{diff_output}`:** diff --git a/src/modules/bmm/workflows/4-implementation/dev-story/instructions.xml b/src/modules/bmm/workflows/4-implementation/dev-story/instructions.xml index 4fb70efe..3e49b378 100644 --- a/src/modules/bmm/workflows/4-implementation/dev-story/instructions.xml +++ b/src/modules/bmm/workflows/4-implementation/dev-story/instructions.xml @@ -219,6 +219,17 @@ ℹī¸ No sprint status file exists - story progress will be tracked in story file only Set {{current_sprint_status}} = "no-sprint-tracking" + + + + Capture current HEAD commit: `git rev-parse HEAD` + Store as {{baseline_commit}} + Write to story file Dev Agent Record: "**Baseline Commit:** {{baseline_commit}}" + + + Set {{baseline_commit}} = "NO_GIT" + Write to story file Dev Agent Record: "**Baseline Commit:** NO_GIT" + From b628eec9fd83a9cf900ade4976dab79e8aa7cbfb Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 04:07:23 -0800 Subject: [PATCH 05/43] refactor(code-review): simplify adversarial review task invocation --- .../steps/step-04-adversarial-review.md | 41 +++---------------- 1 file changed, 6 insertions(+), 35 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index 1727c33d..5db1f354 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -75,44 +75,15 @@ If no baseline available, review current state of files in `{file_list}`: ### 2. Invoke Adversarial Review -Use information asymmetry: separate context from review - -**Execution Hierarchy (try in order):** - -**Option A: Subagent (Preferred)** - -If Task tool available with subagent capability: +With `{diff_output}` constructed, invoke the review task. If possible, use information asymmetry: run this step, and only it, in a separate subagent or process with read access to the project, but no context except the `{diff_output}`. ```xml - - Review {diff_output} using {project-root}/_bmad/core/tasks/review-adversarial-general.xml - +Review {diff_output} using {project-root}/_bmad/core/tasks/review-adversarial-general.xml ``` -The subagent: +**Platform fallback:** If task invocation not available, load the task file and execute its instructions inline, passing `{diff_output}` as the content. -- Has FULL read access to the repository -- Receives ONLY `{diff_output}` as context -- Does NOT know story requirements, ACs, or intent -- Reviews code purely on technical merit - -**Option B: CLI Fallback** - -If subagent not available but CLI available: - -```bash -# Pipe diff to adversarial review task -cat {diff_file} | claude --task {adversarial_review_task} -``` - -**Option C: Inline Execution** - -If neither available, load `review-adversarial-general.xml` and execute inline: - -1. Load task file -2. Adopt adversarial persona -3. Review `{diff_output}` with zero story context -4. Generate findings +The task should: review `{diff_output}` and return a list of findings. ### 3. Process Adversarial Findings @@ -186,7 +157,7 @@ Proceeding to findings consolidation... - Diff constructed from correct source (uncommitted or commits) - Story file excluded from diff -- Subagent invoked with proper isolation (or fallback used) +- Task invoked with diff as input - Adversarial review executed - Findings captured with severity and validity - `{asymmetric_findings}` populated @@ -198,7 +169,7 @@ Proceeding to findings consolidation... - Including story file in diff (breaks asymmetry) - Skipping adversarial review entirely - Accepting zero findings without halt -- Not using subagent when available +- Invoking task without providing diff input - Missing severity/validity classification - Not storing findings for consolidation - No explicit NEXT directive at step completion From b8eeb78cffbd5089134789074b4283d83ac16b35 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 04:13:46 -0800 Subject: [PATCH 06/43] refactor(adversarial-review): simplify severity/validity classification --- .../steps/step-04-adversarial-review.md | 19 ++----------------- .../steps/step-05-adversarial-review.md | 2 +- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index 5db1f354..1a2485ec 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -89,24 +89,9 @@ The task should: review `{diff_output}` and return a list of findings. Capture findings from adversarial review. -**If zero findings returned:** +**If zero findings:** HALT - this is suspicious. Re-analyze or ask for guidance. -HALT - Zero findings is suspicious. Re-analyze or ask for guidance. - -**For each finding:** - -Assign severity: - -- CRITICAL: Security vulnerabilities, data loss risks -- HIGH: Logic errors, missing error handling -- MEDIUM: Performance issues, code smells -- LOW: Style, documentation - -Assign validity: - -- REAL: Genuine issue to address -- NOISE: False positive (explain why) -- UNDECIDED: Needs human judgment +Evaluate severity (Critical, High, Medium, Low) and validity (Real, Noise, Undecided). Create `{asymmetric_findings}` list: diff --git a/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md b/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md index a5c63422..5ce5ff7f 100644 --- a/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md +++ b/src/modules/bmm/workflows/bmad-quick-flow/quick-dev/steps/step-05-adversarial-review.md @@ -79,7 +79,7 @@ The task should: review `{diff_output}` and return a list of findings. Capture the findings from the task output. **If zero findings:** HALT - this is suspicious. Re-analyze or request user guidance. -Evaluate severity (Critical, High, Medium, Low) and validity (real, noise, undecided). +Evaluate severity (Critical, High, Medium, Low) and validity (Real, Noise, Undecided). DO NOT exclude findings based on severity or validity unless explicitly asked to do so. Order findings by severity. Number the ordered findings (F1, F2, F3, etc.). From 5fcdae02b5790c4124e1a166c5f7258a1a5b4a74 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 05:33:21 -0800 Subject: [PATCH 07/43] refactor(code-review): defer finding IDs until consolidation --- .../steps/step-03-context-aware-review.md | 27 ++++----- .../steps/step-04-adversarial-review.md | 19 +++--- .../steps/step-05-consolidate-findings.md | 58 +++++++------------ 3 files changed, 43 insertions(+), 61 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md index a36589e1..2d7a3c7e 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md @@ -43,15 +43,14 @@ Review `{git_discrepancies}` and create findings: | Discrepancy Type | Severity | | --- | --- | -| Files changed but not in story File List | MEDIUM | -| Story lists files but no git changes | HIGH | -| Uncommitted changes not documented | MEDIUM | +| Files changed but not in story File List | Medium | +| Story lists files but no git changes | High | +| Uncommitted changes not documented | Medium | -For each discrepancy, add to `{context_aware_findings}`: +For each discrepancy, add to `{context_aware_findings}` (no IDs yet - assigned after merge): ``` { - id: "CAF-{n}", source: "git-discrepancy", severity: "...", description: "...", @@ -66,15 +65,14 @@ For EACH AC in `{acceptance_criteria}`: 1. Read the AC requirement 2. Search implementation files in `{comprehensive_file_list}` for evidence 3. Determine status: IMPLEMENTED, PARTIAL, or MISSING -4. If PARTIAL or MISSING → add HIGH severity finding +4. If PARTIAL or MISSING → add High severity finding Add to `{context_aware_findings}`: ``` { - id: "CAF-{n}", source: "ac-validation", - severity: "HIGH", + severity: "High", description: "AC {id} not fully implemented: {details}", evidence: "Expected: {ac}, Found: {what_was_found}" } @@ -86,16 +84,15 @@ For EACH task marked [x] in `{tasks_with_status}`: 1. Read the task description 2. Search files for evidence it was actually done -3. **CRITICAL**: If marked [x] but NOT DONE → CRITICAL finding +3. **Critical**: If marked [x] but NOT DONE → Critical finding 4. Record specific proof (file:line) if done Add to `{context_aware_findings}` if false: ``` { - id: "CAF-{n}", source: "task-audit", - severity: "CRITICAL", + severity: "Critical", description: "Task marked complete but not implemented: {task}", evidence: "Searched: {files}, Found: no evidence of {expected}" } @@ -137,10 +134,10 @@ Present context-aware findings: **Phase 1: Context-Aware Review Complete** **Findings:** {count} -- CRITICAL: {count} -- HIGH: {count} -- MEDIUM: {count} -- LOW: {count} +- Critical: {count} +- High: {count} +- Medium: {count} +- Low: {count} Proceeding to Phase 2: Adversarial Review... ``` diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index 1a2485ec..f77b2221 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -93,12 +93,11 @@ Capture findings from adversarial review. Evaluate severity (Critical, High, Medium, Low) and validity (Real, Noise, Undecided). -Create `{asymmetric_findings}` list: +Add each finding to `{asymmetric_findings}` (no IDs yet - assigned after merge): ``` { - id: "AAF-{n}", - source: "adversarial-review", + source: "adversarial", severity: "...", validity: "...", description: "...", @@ -115,15 +114,15 @@ Present adversarial findings: **Reviewer Context:** Pure diff review (no story knowledge) **Findings:** {count} -- CRITICAL: {count} -- HIGH: {count} -- MEDIUM: {count} -- LOW: {count} +- Critical: {count} +- High: {count} +- Medium: {count} +- Low: {count} **Validity Assessment:** -- Real issues: {count} -- Noise/false positives: {count} -- Needs judgment: {count} +- Real: {count} +- Noise: {count} +- Undecided: {count} Proceeding to findings consolidation... ``` diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md index 0ae3f418..69953215 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md @@ -57,53 +57,39 @@ Keep the MORE DETAILED version: - If adversarial finding has better technical detail → keep that - When in doubt, keep context-aware (has more context) -Mark duplicates as merged: - -``` -{ - id: "CF-{n}", - merged_from: ["CAF-3", "AAF-2"], - kept_version: "CAF-3", - reason: "Context-aware version includes AC reference" -} -``` +Note which findings were merged (for transparency in the summary). ### 3. Normalize Severity -Apply consistent severity scale: - -| Severity | Icon | Criteria | -| --- | --- | --- | -| CRITICAL | RED | Security vuln, data loss, tasks marked done but not, broken core functionality | -| HIGH | ORANGE | Missing AC implementation, logic errors, missing critical error handling | -| MEDIUM | YELLOW | Performance issues, incomplete features, documentation gaps | -| LOW | GREEN | Code style, minor improvements, suggestions | +Apply consistent severity scale (Critical, High, Medium, Low). ### 4. Filter Noise -Review adversarial findings marked as NOISE: +Review adversarial findings marked as Noise: - If clearly false positive (e.g., style preference, not actual issue) → exclude -- If questionable → keep with UNDECIDED validity -- If context reveals it's actually valid → upgrade to REAL +- If questionable → keep with Undecided validity +- If context reveals it's actually valid → upgrade to Real **Do NOT filter:** -- Any CRITICAL or HIGH severity +- Any Critical or High severity - Any context-aware findings (they have story context) -### 5. Create Consolidated Table +### 5. Sort and Number Findings + +Sort by severity (Critical → High → Medium → Low), then assign IDs: F1, F2, F3, etc. Build `{consolidated_findings}`: ```markdown | ID | Severity | Source | Description | Location | |----|----------|--------|-------------|----------| -| CF-1 | CRITICAL | task-audit | Task 3 marked [x] but not implemented | src/auth.ts | -| CF-2 | HIGH | ac-validation | AC2 partially implemented | src/api/*.ts | -| CF-3 | HIGH | adversarial | Missing error handling in API calls | src/api/client.ts:45 | -| CF-4 | MEDIUM | git-discrepancy | File changed but not in story | src/utils.ts | -| CF-5 | LOW | adversarial | Magic number should be constant | src/config.ts:12 | +| F1 | Critical | task-audit | Task 3 marked [x] but not implemented | src/auth.ts | +| F2 | High | ac-validation | AC2 partially implemented | src/api/*.ts | +| F3 | High | adversarial | Missing error handling in API calls | src/api/client.ts:45 | +| F4 | Medium | git-discrepancy | File changed but not in story | src/utils.ts | +| F5 | Low | adversarial | Magic number should be constant | src/config.ts:12 | ``` ### 6. Present Consolidated Findings @@ -115,10 +101,10 @@ Build `{consolidated_findings}`: **Summary:** - Total findings: {count} -- CRITICAL: {count} -- HIGH: {count} -- MEDIUM: {count} -- LOW: {count} +- Critical: {count} +- High: {count} +- Medium: {count} +- Low: {count} **Deduplication:** {merged_count} duplicate findings merged @@ -126,16 +112,16 @@ Build `{consolidated_findings}`: ## Findings by Severity -### CRITICAL (Must Fix) +### Critical (Must Fix) {list critical findings with full details} -### HIGH (Should Fix) +### High (Should Fix) {list high findings with full details} -### MEDIUM (Consider Fixing) +### Medium (Consider Fixing) {list medium findings} -### LOW (Nice to Fix) +### Low (Nice to Fix) {list low findings} --- From 0f708d0b89a8d6e44670062a806e2b317f77becd Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 18:28:51 -0800 Subject: [PATCH 08/43] refactor(core): shorten adversarial review task name --- src/core/tasks/review-adversarial-general.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tasks/review-adversarial-general.xml b/src/core/tasks/review-adversarial-general.xml index 6e5df408..3521157d 100644 --- a/src/core/tasks/review-adversarial-general.xml +++ b/src/core/tasks/review-adversarial-general.xml @@ -1,7 +1,7 @@ - + Cynically review content and produce findings From 25f93a3b6499a2a99dc9312c9186933cd1b26c31 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 20:59:58 -0800 Subject: [PATCH 09/43] refactor(code-review): simplify workflow.md --- .../4-implementation/code-review/workflow.md | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 46c3bdeb..afc4149d 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -1,23 +1,14 @@ --- name: code-review -description: 'Perform an ADVERSARIAL Senior Developer code review with dual-phase analysis: context-aware story validation plus context-independent adversarial diff review. Finds 3-10 specific problems in every story. NEVER accepts "looks good" - must find minimum issues and can auto-fix with user approval.' +description: 'Code review for dev-story output. Audits acceptance criteria against implementation, performs adversarial diff review, can auto-fix with approval. A different LLM than the implementer is recommended.' --- # Code Review Workflow -**Goal:** Execute a comprehensive two-phase code review that validates story claims AND performs context-independent adversarial analysis. - -**Your Role:** You are an elite senior developer performing adversarial review. Challenge everything. Verify claims against reality. Find what's wrong or missing. - ---- - -## WORKFLOW ARCHITECTURE - -This uses **step-file architecture** for focused execution: +## WORKFLOW ARCHITECTURE: STEP FILES - Each step loads fresh to combat "lost in the middle" - State persists via variables: `{story_path}`, `{story_key}`, `{context_aware_findings}`, `{asymmetric_findings}` -- Dual-phase review: context-aware (step 3) + adversarial asymmetric (step 4) --- From dadca29b093589aa66bdd780e0e3c5c4a26870ef Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 21:00:18 -0800 Subject: [PATCH 10/43] refactor(code-review): use installed_path variable in step files --- .../code-review/steps/step-01-load-story.md | 5 ++--- .../code-review/steps/step-02-build-attack-plan.md | 5 ++--- .../code-review/steps/step-03-context-aware-review.md | 5 ++--- .../code-review/steps/step-04-adversarial-review.md | 7 +++---- .../code-review/steps/step-05-consolidate-findings.md | 5 ++--- .../code-review/steps/step-06-resolve-and-update.md | 3 +-- 6 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 50660b45..861a5ece 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -2,9 +2,8 @@ name: 'step-01-load-story' description: 'Load story file, discover git changes, establish review context' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-01-load-story.md' -nextStepFile: '{workflow_path}/steps/step-02-build-attack-plan.md' +thisStepFile: '{installed_path}/steps/step-01-load-story.md' +nextStepFile: '{installed_path}/steps/step-02-build-attack-plan.md' --- # Step 1: Load Story and Discover Changes diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md index 3b442899..bae20e39 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md @@ -2,9 +2,8 @@ name: 'step-02-build-attack-plan' description: 'Extract ACs and tasks, create comprehensive review plan for both phases' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-02-build-attack-plan.md' -nextStepFile: '{workflow_path}/steps/step-03-context-aware-review.md' +thisStepFile: '{installed_path}/steps/step-02-build-attack-plan.md' +nextStepFile: '{installed_path}/steps/step-03-context-aware-review.md' --- # Step 2: Build Review Attack Plan diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md index 2d7a3c7e..4e5ff747 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md @@ -2,9 +2,8 @@ name: 'step-03-context-aware-review' description: 'Story-aware validation: verify ACs, audit task completion, check git discrepancies' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-03-context-aware-review.md' -nextStepFile: '{workflow_path}/steps/step-04-adversarial-review.md' +thisStepFile: '{installed_path}/steps/step-03-context-aware-review.md' +nextStepFile: '{installed_path}/steps/step-04-adversarial-review.md' --- # Step 3: Context-Aware Review diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index f77b2221..e0746abe 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -2,9 +2,8 @@ name: 'step-04-adversarial-review' description: 'Context-independent adversarial diff review via subagent - no story knowledge' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-04-adversarial-review.md' -nextStepFile: '{workflow_path}/steps/step-05-consolidate-findings.md' +thisStepFile: '{installed_path}/steps/step-04-adversarial-review.md' +nextStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' --- # Step 4: Adversarial Review (Information Asymmetric) @@ -78,7 +77,7 @@ If no baseline available, review current state of files in `{file_list}`: With `{diff_output}` constructed, invoke the review task. If possible, use information asymmetry: run this step, and only it, in a separate subagent or process with read access to the project, but no context except the `{diff_output}`. ```xml -Review {diff_output} using {project-root}/_bmad/core/tasks/review-adversarial-general.xml +Review {diff_output} using {adversarial_review_task} ``` **Platform fallback:** If task invocation not available, load the task file and execute its instructions inline, passing `{diff_output}` as the content. diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md index 69953215..97e4c0d3 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md @@ -2,9 +2,8 @@ name: 'step-05-consolidate-findings' description: 'Merge and deduplicate findings from both review phases' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-05-consolidate-findings.md' -nextStepFile: '{workflow_path}/steps/step-06-resolve-and-update.md' +thisStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' +nextStepFile: '{installed_path}/steps/step-06-resolve-and-update.md' --- # Step 5: Consolidate Findings diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md index 9b2c51bf..44988a54 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md @@ -2,8 +2,7 @@ name: 'step-06-resolve-and-update' description: 'Present findings, fix or create action items, update story and sprint status' -workflow_path: '{project-root}/_bmad/bmm/workflows/4-implementation/code-review' -thisStepFile: '{workflow_path}/steps/step-06-resolve-and-update.md' +thisStepFile: '{installed_path}/steps/step-06-resolve-and-update.md' --- # Step 6: Resolve Findings and Update Status From 7c914ae8b2d3e4e9202057dcd811f4c088489fc4 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 21:05:48 -0800 Subject: [PATCH 11/43] refactor(code-review): inline single-use adversarial task path --- .../code-review/steps/step-04-adversarial-review.md | 2 +- .../bmm/workflows/4-implementation/code-review/workflow.md | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md index e0746abe..96a82d8a 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md @@ -77,7 +77,7 @@ If no baseline available, review current state of files in `{file_list}`: With `{diff_output}` constructed, invoke the review task. If possible, use information asymmetry: run this step, and only it, in a separate subagent or process with read access to the project, but no context except the `{diff_output}`. ```xml -Review {diff_output} using {adversarial_review_task} +Review {diff_output} using {project-root}/_bmad/core/tasks/review-adversarial-general.xml ``` **Platform fallback:** If task invocation not available, load the task file and execute its instructions inline, passing `{diff_output}` as the content. diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index afc4149d..13274de9 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -29,10 +29,6 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - `sprint_status` = `{implementation_artifacts}/sprint-status.yaml` - `validation` = `{installed_path}/checklist.md` -### Related Tasks - -- `adversarial_review_task` = `{project-root}/_bmad/core/tasks/review-adversarial-general.xml` - --- ## CRITICAL DIRECTIVES From ac081a27e8ba8730f912cc0cb4b2453416a413d0 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 21:15:04 -0800 Subject: [PATCH 12/43] docs(code-review): clarify step file loading in workflow architecture --- .../bmm/workflows/4-implementation/code-review/workflow.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 13274de9..18318235 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -7,7 +7,8 @@ description: 'Code review for dev-story output. Audits acceptance criteria again ## WORKFLOW ARCHITECTURE: STEP FILES -- Each step loads fresh to combat "lost in the middle" +- This file (workflow.md) stays in context throughout +- Each step file is read just before processing (current step stays at end of context) - State persists via variables: `{story_path}`, `{story_key}`, `{context_aware_findings}`, `{asymmetric_findings}` --- From a8758b03932a83d7e256f825fd7e43695394c1c6 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 21:30:01 -0800 Subject: [PATCH 13/43] refactor(code-review): remove CRITICAL DIRECTIVES, add communication_language --- .../4-implementation/code-review/workflow.md | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 18318235..b6bb3c2a 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -23,6 +23,8 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - `planning_artifacts`, `implementation_artifacts` - `date` as system-generated current datetime +- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}` + ### Paths - `installed_path` = `{project-root}/_bmad/bmm/workflows/4-implementation/code-review` @@ -32,19 +34,6 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: --- -## CRITICAL DIRECTIVES - -YOU ARE AN ADVERSARIAL CODE REVIEWER - Find what's wrong or missing! -Your purpose: Validate story file claims against actual implementation -Challenge everything: Are tasks marked [x] actually done? Are ACs really implemented? -Find 3-10 specific issues in every review minimum - no lazy "looks good" reviews -Read EVERY file in the File List - verify implementation against story requirements -Tasks marked complete but not done = CRITICAL finding -Acceptance Criteria not implemented = HIGH severity finding -Exclude `_bmad/`, `_bmad-output/`, `.cursor/`, `.windsurf/`, `.claude/` from review - ---- - ## EXECUTION Load and execute `steps/step-01-load-story.md` to begin the workflow. From eae4ad46a165950ba0b9e6735a3885ded33596a4 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Sun, 4 Jan 2026 21:38:04 -0800 Subject: [PATCH 14/43] refactor(code-review): remove unused validation path and checklist --- .../4-implementation/code-review/checklist.md | 23 ------------------- .../4-implementation/code-review/workflow.md | 1 - 2 files changed, 24 deletions(-) delete mode 100644 src/modules/bmm/workflows/4-implementation/code-review/checklist.md diff --git a/src/modules/bmm/workflows/4-implementation/code-review/checklist.md b/src/modules/bmm/workflows/4-implementation/code-review/checklist.md deleted file mode 100644 index f213a6b9..00000000 --- a/src/modules/bmm/workflows/4-implementation/code-review/checklist.md +++ /dev/null @@ -1,23 +0,0 @@ -# Senior Developer Review - Validation Checklist - -- [ ] Story file loaded from `{{story_path}}` -- [ ] Story Status verified as reviewable (review) -- [ ] Epic and Story IDs resolved ({{epic_num}}.{{story_num}}) -- [ ] Story Context located or warning recorded -- [ ] Epic Tech Spec located or warning recorded -- [ ] Architecture/standards docs loaded (as available) -- [ ] Tech stack detected and documented -- [ ] MCP doc search performed (or web fallback) and references captured -- [ ] Acceptance Criteria cross-checked against implementation -- [ ] File List reviewed and validated for completeness -- [ ] Tests identified and mapped to ACs; gaps noted -- [ ] Code quality review performed on changed files -- [ ] Security review performed on changed files and dependencies -- [ ] Outcome decided (Approve/Changes Requested/Blocked) -- [ ] Review notes appended under "Senior Developer Review (AI)" -- [ ] Change Log updated with review entry -- [ ] Status updated according to settings (if enabled) -- [ ] Sprint status synced (if sprint tracking enabled) -- [ ] Story saved successfully - -_Reviewer: {{user_name}} on {{date}}_ diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index b6bb3c2a..30551b1a 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -30,7 +30,6 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - `installed_path` = `{project-root}/_bmad/bmm/workflows/4-implementation/code-review` - `project_context` = `**/project-context.md` (load if exists) - `sprint_status` = `{implementation_artifacts}/sprint-status.yaml` -- `validation` = `{installed_path}/checklist.md` --- From 64c32d8c8c05a3b3434c98e354ab965163bc455c Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 00:39:17 -0800 Subject: [PATCH 15/43] refactor(code-review): add web_bundle: false, use "Read and follow" wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add web_bundle: false to frontmatter (workflow needs file access) - Change "Load and execute" to "Read and follow" (clearer for LLMs) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../bmm/workflows/4-implementation/code-review/workflow.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 30551b1a..1d46e3fb 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -1,6 +1,7 @@ --- name: code-review description: 'Code review for dev-story output. Audits acceptance criteria against implementation, performs adversarial diff review, can auto-fix with approval. A different LLM than the implementer is recommended.' +web_bundle: false --- # Code Review Workflow @@ -35,4 +36,4 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: ## EXECUTION -Load and execute `steps/step-01-load-story.md` to begin the workflow. +Read and follow `steps/step-01-load-story.md` to begin the workflow. From ae9b83388cd6704383f2da6078beab65d2b9830d Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 01:09:38 -0800 Subject: [PATCH 16/43] refactor(code-review): reorder phases - adversarial first, context-aware second - Swap step-03 and step-04: adversarial review now runs before context-aware - Move discover_inputs from step-01 to step-04 (JIT loading) - Add input_file_patterns to workflow.md frontmatter - Adversarial runs lean (just diff + code), context-aware loads planning docs --- .../code-review/steps/step-01-load-story.md | 7 +++---- .../steps/step-02-build-attack-plan.md | 4 ++-- ...review.md => step-03-adversarial-review.md} | 10 +++++----- ...view.md => step-04-context-aware-review.md} | 16 +++++++++++----- .../4-implementation/code-review/workflow.md | 18 ++++++++++++++++++ 5 files changed, 39 insertions(+), 16 deletions(-) rename src/modules/bmm/workflows/4-implementation/code-review/steps/{step-04-adversarial-review.md => step-03-adversarial-review.md} (93%) rename src/modules/bmm/workflows/4-implementation/code-review/steps/{step-03-context-aware-review.md => step-04-context-aware-review.md} (89%) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 861a5ece..0c8fd8c9 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -82,10 +82,9 @@ Set `{git_discrepancies}` with categories: - **files_in_story_not_git**: Files in story File List but no git changes - **uncommitted_undocumented**: Uncommitted changes not tracked in story -### 6. Load Context +### 6. Load Project Context -- Load `{project_context}` if exists (`**/project-context.md`) -- Invoke `discover_inputs` protocol to load architecture, UX, epic docs as needed +- Load `{project_context}` if exists (`**/project-context.md`) for coding standards --- @@ -105,7 +104,7 @@ Set `{git_discrepancies}` with categories: - `{story_file_list}` compiled from Dev Agent Record - `{git_changed_files}` discovered via git commands - `{git_discrepancies}` calculated -- Context documents loaded +- `{project_context}` loaded if exists - Explicit NEXT directive provided ## FAILURE MODES diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md index bae20e39..6307e963 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md @@ -3,7 +3,7 @@ name: 'step-02-build-attack-plan' description: 'Extract ACs and tasks, create comprehensive review plan for both phases' thisStepFile: '{installed_path}/steps/step-02-build-attack-plan.md' -nextStepFile: '{installed_path}/steps/step-03-context-aware-review.md' +nextStepFile: '{installed_path}/steps/step-03-adversarial-review.md' --- # Step 2: Build Review Attack Plan @@ -124,7 +124,7 @@ Proceeding with dual-phase review... **CRITICAL:** When this step completes, explicitly state: -"**NEXT:** Loading `step-03-context-aware-review.md`" +"**NEXT:** Loading `step-03-adversarial-review.md`" --- diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md similarity index 93% rename from src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md rename to src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md index 96a82d8a..cb1e8b6c 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md @@ -1,12 +1,12 @@ --- -name: 'step-04-adversarial-review' +name: 'step-03-adversarial-review' description: 'Context-independent adversarial diff review via subagent - no story knowledge' -thisStepFile: '{installed_path}/steps/step-04-adversarial-review.md' -nextStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' +thisStepFile: '{installed_path}/steps/step-03-adversarial-review.md' +nextStepFile: '{installed_path}/steps/step-04-context-aware-review.md' --- -# Step 4: Adversarial Review (Information Asymmetric) +# Step 3: Adversarial Review (Information Asymmetric) **Goal:** Perform context-independent adversarial review of code changes. Reviewer sees ONLY the diff - no story, no ACs, no context about WHY changes were made. @@ -132,7 +132,7 @@ Proceeding to findings consolidation... **CRITICAL:** When this step completes, explicitly state: -"**NEXT:** Loading `step-05-consolidate-findings.md`" +"**NEXT:** Loading `step-04-context-aware-review.md`" --- diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md similarity index 89% rename from src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md rename to src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md index 4e5ff747..343fba08 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md @@ -1,12 +1,12 @@ --- -name: 'step-03-context-aware-review' +name: 'step-04-context-aware-review' description: 'Story-aware validation: verify ACs, audit task completion, check git discrepancies' -thisStepFile: '{installed_path}/steps/step-03-context-aware-review.md' -nextStepFile: '{installed_path}/steps/step-04-adversarial-review.md' +thisStepFile: '{installed_path}/steps/step-04-context-aware-review.md' +nextStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' --- -# Step 3: Context-Aware Review +# Step 4: Context-Aware Review **Goal:** Perform story-aware validation - verify AC implementation, audit task completion, review code quality with full story context. @@ -36,6 +36,12 @@ Initialize `{context_aware_findings}` as empty list. ## EXECUTION SEQUENCE +### 0. Load Planning Context (JIT) + +Invoke `discover_inputs` protocol to load architecture, UX, and epic documents. + +These provide the design context needed to validate AC implementation against system requirements. + ### 1. Git vs Story Discrepancies Review `{git_discrepancies}` and create findings: @@ -149,7 +155,7 @@ Store `{context_aware_findings}` for consolidation in step 5. **CRITICAL:** When this step completes, explicitly state: -"**NEXT:** Loading `step-04-adversarial-review.md`" +"**NEXT:** Loading `step-05-consolidate-findings.md`" --- diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 1d46e3fb..8fa0cfa5 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -2,6 +2,24 @@ name: code-review description: 'Code review for dev-story output. Audits acceptance criteria against implementation, performs adversarial diff review, can auto-fix with approval. A different LLM than the implementer is recommended.' web_bundle: false + +input_file_patterns: + architecture: + description: 'System architecture for review context' + whole: '{planning_artifacts}/*architecture*.md' + sharded: '{planning_artifacts}/*architecture*/*.md' + load_strategy: 'FULL_LOAD' + ux_design: + description: 'UX design specification (if UI review)' + whole: '{planning_artifacts}/*ux*.md' + sharded: '{planning_artifacts}/*ux*/*.md' + load_strategy: 'FULL_LOAD' + epics: + description: 'Epic containing story being reviewed' + whole: '{planning_artifacts}/*epic*.md' + sharded_index: '{planning_artifacts}/*epic*/index.md' + sharded_single: '{planning_artifacts}/*epic*/epic-{{epic_num}}.md' + load_strategy: 'SELECTIVE_LOAD' --- # Code Review Workflow From 0f18c4bcba0decf46d78add66ab5166002df0230 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 01:12:35 -0800 Subject: [PATCH 17/43] refactor(code-review): replace discover_inputs protocol with explicit file loading --- .../code-review/steps/step-04-context-aware-review.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md index 343fba08..90d295c3 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md @@ -38,7 +38,11 @@ Initialize `{context_aware_findings}` as empty list. ### 0. Load Planning Context (JIT) -Invoke `discover_inputs` protocol to load architecture, UX, and epic documents. +Load planning documents for AC validation against system design: + +- **Architecture**: `{planning_artifacts}/*architecture*.md` (or sharded: `{planning_artifacts}/*architecture*/*.md`) +- **UX Design**: `{planning_artifacts}/*ux*.md` (if UI review relevant) +- **Epic**: `{planning_artifacts}/*epic*/epic-{epic_num}.md` (the epic containing this story) These provide the design context needed to validate AC implementation against system requirements. From 9700da9dc666805eed2383b94c65694e288067ec Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 01:14:37 -0800 Subject: [PATCH 18/43] refactor(code-review): remove input_file_patterns from workflow.md to prevent context leak --- .../4-implementation/code-review/workflow.md | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md index 8fa0cfa5..1d46e3fb 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/workflow.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/workflow.md @@ -2,24 +2,6 @@ name: code-review description: 'Code review for dev-story output. Audits acceptance criteria against implementation, performs adversarial diff review, can auto-fix with approval. A different LLM than the implementer is recommended.' web_bundle: false - -input_file_patterns: - architecture: - description: 'System architecture for review context' - whole: '{planning_artifacts}/*architecture*.md' - sharded: '{planning_artifacts}/*architecture*/*.md' - load_strategy: 'FULL_LOAD' - ux_design: - description: 'UX design specification (if UI review)' - whole: '{planning_artifacts}/*ux*.md' - sharded: '{planning_artifacts}/*ux*/*.md' - load_strategy: 'FULL_LOAD' - epics: - description: 'Epic containing story being reviewed' - whole: '{planning_artifacts}/*epic*.md' - sharded_index: '{planning_artifacts}/*epic*/index.md' - sharded_single: '{planning_artifacts}/*epic*/epic-{{epic_num}}.md' - load_strategy: 'SELECTIVE_LOAD' --- # Code Review Workflow From 1f5700ea14848753f903743547ca8fe5c7fa4930 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 02:37:00 -0800 Subject: [PATCH 19/43] refactor(code-review): remove unused thisStepFile/nextStepFile from frontmatter --- .../4-implementation/code-review/steps/step-01-load-story.md | 3 --- .../code-review/steps/step-02-build-attack-plan.md | 3 --- .../code-review/steps/step-03-adversarial-review.md | 3 --- .../code-review/steps/step-04-context-aware-review.md | 3 --- .../code-review/steps/step-05-consolidate-findings.md | 3 --- .../code-review/steps/step-06-resolve-and-update.md | 2 -- 6 files changed, 17 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 0c8fd8c9..d5473c96 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -1,9 +1,6 @@ --- name: 'step-01-load-story' description: 'Load story file, discover git changes, establish review context' - -thisStepFile: '{installed_path}/steps/step-01-load-story.md' -nextStepFile: '{installed_path}/steps/step-02-build-attack-plan.md' --- # Step 1: Load Story and Discover Changes diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md index 6307e963..13907a12 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md @@ -1,9 +1,6 @@ --- name: 'step-02-build-attack-plan' description: 'Extract ACs and tasks, create comprehensive review plan for both phases' - -thisStepFile: '{installed_path}/steps/step-02-build-attack-plan.md' -nextStepFile: '{installed_path}/steps/step-03-adversarial-review.md' --- # Step 2: Build Review Attack Plan diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md index cb1e8b6c..45681fda 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md @@ -1,9 +1,6 @@ --- name: 'step-03-adversarial-review' description: 'Context-independent adversarial diff review via subagent - no story knowledge' - -thisStepFile: '{installed_path}/steps/step-03-adversarial-review.md' -nextStepFile: '{installed_path}/steps/step-04-context-aware-review.md' --- # Step 3: Adversarial Review (Information Asymmetric) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md index 90d295c3..ca145736 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md @@ -1,9 +1,6 @@ --- name: 'step-04-context-aware-review' description: 'Story-aware validation: verify ACs, audit task completion, check git discrepancies' - -thisStepFile: '{installed_path}/steps/step-04-context-aware-review.md' -nextStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' --- # Step 4: Context-Aware Review diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md index 97e4c0d3..2c310e5f 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md @@ -1,9 +1,6 @@ --- name: 'step-05-consolidate-findings' description: 'Merge and deduplicate findings from both review phases' - -thisStepFile: '{installed_path}/steps/step-05-consolidate-findings.md' -nextStepFile: '{installed_path}/steps/step-06-resolve-and-update.md' --- # Step 5: Consolidate Findings diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md index 44988a54..700032b7 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-06-resolve-and-update.md @@ -1,8 +1,6 @@ --- name: 'step-06-resolve-and-update' description: 'Present findings, fix or create action items, update story and sprint status' - -thisStepFile: '{installed_path}/steps/step-06-resolve-and-update.md' --- # Step 6: Resolve Findings and Update Status From 6886e3c8cd1000f31a902a27ee116d8b141e1bc6 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 02:54:00 -0800 Subject: [PATCH 20/43] refactor(code-review): clarify step-01 description and NO_GIT handling --- .../code-review/steps/step-01-load-story.md | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index d5473c96..34830796 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -1,12 +1,10 @@ --- name: 'step-01-load-story' -description: 'Load story file, discover git changes, establish review context' +description: "Compare story's file list against git changes" --- # Step 1: Load Story and Discover Changes -**Goal:** Load story file, extract all sections, discover actual git changes for comparison. - --- ## STATE VARIABLES (capture now, persist throughout) @@ -53,9 +51,11 @@ Set `{story_file_list}` = list of files from Dev Agent Record → File List ### 4. Discover Git Changes -Check if git repository exists: +Check if git repository exists. -**If Git repo detected:** +**If NOT a git repo:** Set `{git_changed_files}` = NO_GIT, `{git_discrepancies}` = NO_GIT. Skip to substep 6. + +**If git repo detected:** ```bash git status --porcelain @@ -63,11 +63,7 @@ git diff --name-only git diff --cached --name-only ``` -Compile `{git_changed_files}` = union of modified, staged, and new files - -**If NOT a Git repo:** - -Set `{git_changed_files}` = "NO_GIT" +Compile `{git_changed_files}` = union of modified, staged, and new files. ### 5. Cross-Reference Story vs Git From 2bd6e9df1b08c31aad85abfe891360e3065edd44 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 04:14:14 -0800 Subject: [PATCH 21/43] docs(code-review): clarify step-01 story identification algorithm - Fixed variable naming convention: backticks for names, curlies only for value substitution - Rewrote Identify Story section with explicit two-path algorithm (file path vs sprint_status search) - Added verification step for files not in sprint_status with user confirmation flow - Clarified matching priority order: exact key > full ID > partial > name > description - Made loopback instructions consistent and explicit (return to user prompt) - Improved git_discrepancies description from vague "differences" to concrete "mismatches" - Tested with 30+ test cases and fresh agent review - algorithm is clear and executable --- .../code-review/steps/step-01-load-story.md | 63 +++++++++++-------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 34830796..841e100a 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -11,11 +11,11 @@ description: "Compare story's file list against git changes" These variables MUST be set in this step and available to all subsequent steps: -- `{story_path}` - Path to the story file being reviewed -- `{story_key}` - Story identifier (e.g., "1-2-user-authentication") -- `{story_file_list}` - Files claimed in story's Dev Agent Record → File List -- `{git_changed_files}` - Files actually changed according to git -- `{git_discrepancies}` - Differences between story claims and git reality +- `story_path` - Path to the story file being reviewed +- `story_key` - Story identifier (e.g., "1-2-user-authentication") +- `story_file_list` - Files claimed in story's Dev Agent Record → File List +- `git_changed_files` - Files actually changed according to git +- `git_discrepancies` - Mismatches between `story_file_list` and `git_changed_files` --- @@ -23,19 +23,32 @@ These variables MUST be set in this step and available to all subsequent steps: ### 1. Identify Story -**If `{story_path}` provided by user:** +Ask user: "Which story would you like to review?" -- Use the provided path directly +**Try input as direct file path first:** +If input resolves to an existing file: + - Verify it's in `sprint_status` with status `review` or `done` + - If verified → set `story_path` to that file path + - If NOT verified → Warn user the file is not in sprint_status (or wrong status). Ask: "Continue anyway?" + - If yes → set `story_path` + - If no → return to user prompt (ask "Which story would you like to review?" again) -**If NOT provided:** +**Search sprint_status** (if input is not a direct file): +Search for stories with status `review` or `done`. Match by priority: +1. Exact story number (e.g., "1-2") +2. Exact story name/key (e.g., "1-2-user-auth-api") +3. Story name/title contains input +4. Story description contains input -- Ask user which story file to review -- Wait for response before proceeding +**Resolution:** +- **Single match**: Confident. Set `story_path`, proceed to substep 2 +- **Multiple matches**: Uncertain. Present all candidates to user. Wait for selection. Set `story_path`, proceed to substep 2 +- **No match**: Ask user to clarify or provide the full story path. Return to user prompt (ask "Which story would you like to review?" again) ### 2. Load Story File -- Read COMPLETE story file from `{story_path}` -- Extract `{story_key}` from filename (e.g., "1-2-user-authentication.md" → "1-2-user-authentication") or story metadata +- Read COMPLETE story file from {story_path} +- Extract `story_key` from filename (e.g., "1-2-user-authentication.md" → "1-2-user-authentication") or story metadata ### 3. Parse Story Sections @@ -47,13 +60,13 @@ Extract and store: - **Dev Agent Record → File List**: Claimed file changes - **Change Log**: History of modifications -Set `{story_file_list}` = list of files from Dev Agent Record → File List +Set `story_file_list` = list of files from Dev Agent Record → File List ### 4. Discover Git Changes Check if git repository exists. -**If NOT a git repo:** Set `{git_changed_files}` = NO_GIT, `{git_discrepancies}` = NO_GIT. Skip to substep 6. +**If NOT a git repo:** Set `git_changed_files` = NO_GIT, `git_discrepancies` = NO_GIT. Skip to substep 6. **If git repo detected:** @@ -63,13 +76,13 @@ git diff --name-only git diff --cached --name-only ``` -Compile `{git_changed_files}` = union of modified, staged, and new files. +Compile `git_changed_files` = union of modified, staged, and new files. ### 5. Cross-Reference Story vs Git -Compare `{story_file_list}` with `{git_changed_files}`: +Compare {story_file_list} with {git_changed_files}: -Set `{git_discrepancies}` with categories: +Set `git_discrepancies` with categories: - **files_in_git_not_story**: Files changed in git but not in story File List - **files_in_story_not_git**: Files in story File List but no git changes @@ -77,7 +90,7 @@ Set `{git_discrepancies}` with categories: ### 6. Load Project Context -- Load `{project_context}` if exists (`**/project-context.md`) for coding standards +- Load {project_context} if exists (**/project-context.md) for coding standards --- @@ -91,19 +104,19 @@ Set `{git_discrepancies}` with categories: ## SUCCESS METRICS -- `{story_path}` identified and loaded -- `{story_key}` extracted +- `story_path` identified and loaded +- `story_key` extracted - All story sections parsed -- `{story_file_list}` compiled from Dev Agent Record -- `{git_changed_files}` discovered via git commands -- `{git_discrepancies}` calculated -- `{project_context}` loaded if exists +- `story_file_list` compiled from Dev Agent Record +- `git_changed_files` discovered via git commands +- `git_discrepancies` calculated +- `project_context` loaded if exists - Explicit NEXT directive provided ## FAILURE MODES - Proceeding without story file loaded -- Missing `{story_key}` extraction +- Missing `story_key` extraction - Not parsing all story sections - Skipping git change discovery - Not calculating discrepancies From 060d5562a4767ca3833d0738bbd779adb74a81a1 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 04:24:29 -0800 Subject: [PATCH 22/43] docs(code-review): clarify fuzzy matching for story identification - Changed priority 1 from exact to resembles: handles format variations (1 2, 1.2, one-two, one thirty two) - Explicitly prevents false matches: 1-33 does not match 1-32 - Updated priority 3-4 to use resembles instead of contains: supports typos and TTS errors (paiment, passwd) - Added examples for number variations and compound spoken formats - Tested with agent validation: handles typos, format variations, misspellings correctly --- .../code-review/steps/step-01-load-story.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 841e100a..9a0a0e9c 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -35,10 +35,10 @@ If input resolves to an existing file: **Search sprint_status** (if input is not a direct file): Search for stories with status `review` or `done`. Match by priority: -1. Exact story number (e.g., "1-2") +1. Story number resembles input closely enough (e.g., "1-2" matches "1 2", "1.2", "one dash two", "one two"; "1-32" matches "one thirty two"). Do NOT match if numbers differ (e.g., "1-33" does not match "1-32") 2. Exact story name/key (e.g., "1-2-user-auth-api") -3. Story name/title contains input -4. Story description contains input +3. Story name/title resembles input closely enough +4. Story description resembles input closely enough **Resolution:** - **Single match**: Confident. Set `story_path`, proceed to substep 2 From 8fc7db7b97e54b85c33bdd308dba78ccb909caa3 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 05:33:27 -0800 Subject: [PATCH 23/43] refactor(code-review): remove implementation notes from step-01 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementation notes for the workflow should be collected in a dedicated implementation-notes.md file, not embedded in step files. This keeps each step focused and defers editorial comments to a separate tracking document. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 9a0a0e9c..f6a6ae11 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -13,6 +13,7 @@ These variables MUST be set in this step and available to all subsequent steps: - `story_path` - Path to the story file being reviewed - `story_key` - Story identifier (e.g., "1-2-user-authentication") +- `story_content` - Complete, unmodified file content from story_path (loaded in substep 2) - `story_file_list` - Files claimed in story's Dev Agent Record → File List - `git_changed_files` - Files actually changed according to git - `git_discrepancies` - Mismatches between `story_file_list` and `git_changed_files` @@ -47,8 +48,11 @@ Search for stories with status `review` or `done`. Match by priority: ### 2. Load Story File -- Read COMPLETE story file from {story_path} -- Extract `story_key` from filename (e.g., "1-2-user-authentication.md" → "1-2-user-authentication") or story metadata +**Load file content:** +Read the complete contents of {story_path} and assign to `story_content` WITHOUT filtering, truncating or summarizing. If {story_path} cannot be read: report the error to the user and HALT the workflow. + +**Extract story identifier:** +Verify the filename ends with `.md` extension. Remove `.md` to get `story_key` (e.g., "1-2-user-authentication.md" → "1-2-user-authentication"). If filename doesn't end with `.md` or the result is empty: report the error to the user and HALT the workflow. ### 3. Parse Story Sections @@ -121,3 +125,4 @@ Set `git_discrepancies` with categories: - Skipping git change discovery - Not calculating discrepancies - No explicit NEXT directive at step completion + From 18ac3c931a41f6dd392037a45a98c54e18a9424f Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 05:42:04 -0800 Subject: [PATCH 24/43] refactor(code-review): audit step-01 substeps and success/failure criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 01 audit findings: - Substep 3 was extracting items not needed by step-01 (ACs, tasks, changelog) Trimmed to only extract story_file_list (needed for git comparison) - Success/failure criteria now explicitly guard story_content completeness since downstream steps depend on the full file content - Removed "downstream" jargon in favor of "later steps" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index f6a6ae11..7a1b3f01 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -54,15 +54,9 @@ Read the complete contents of {story_path} and assign to `story_content` WITHOUT **Extract story identifier:** Verify the filename ends with `.md` extension. Remove `.md` to get `story_key` (e.g., "1-2-user-authentication.md" → "1-2-user-authentication"). If filename doesn't end with `.md` or the result is empty: report the error to the user and HALT the workflow. -### 3. Parse Story Sections +### 3. Extract File List from Story -Extract and store: - -- **Story**: Title, description, status -- **Acceptance Criteria**: All ACs with their requirements -- **Tasks/Subtasks**: All tasks with completion status ([x] vs [ ]) -- **Dev Agent Record → File List**: Claimed file changes -- **Change Log**: History of modifications +Extract `story_file_list` from the Dev Agent Record → File List section of the story file. Set `story_file_list` = list of files from Dev Agent Record → File List @@ -110,7 +104,7 @@ Set `git_discrepancies` with categories: - `story_path` identified and loaded - `story_key` extracted -- All story sections parsed +- `story_content` captured completely and unmodified - `story_file_list` compiled from Dev Agent Record - `git_changed_files` discovered via git commands - `git_discrepancies` calculated @@ -121,7 +115,7 @@ Set `git_discrepancies` with categories: - Proceeding without story file loaded - Missing `story_key` extraction -- Not parsing all story sections +- `story_content` incomplete or modified (breaks later steps) - Skipping git change discovery - Not calculating discrepancies - No explicit NEXT directive at step completion From 59c58b2e2c7d3ad462167998daed79ad77501b92 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 05:47:46 -0800 Subject: [PATCH 25/43] refactor(code-review): clean up step-01 substep 3 and add error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Substep 3 (Extract File List): - Removed repetitive wording - Reference {story_content} variable instead of generic "story file" - Add error handling: if Dev Agent Record/File List not found, set story_file_list = NO_FILE_LIST - Consistent with NO_GIT pattern used elsewhere 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 7a1b3f01..d402fb9d 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -56,9 +56,9 @@ Verify the filename ends with `.md` extension. Remove `.md` to get `story_key` ( ### 3. Extract File List from Story -Extract `story_file_list` from the Dev Agent Record → File List section of the story file. +Extract `story_file_list` from the Dev Agent Record → File List section of {story_content}. -Set `story_file_list` = list of files from Dev Agent Record → File List +**If Dev Agent Record or File List section not found:** Report to user and set `story_file_list` = NO_FILE_LIST. ### 4. Discover Git Changes From 71a1c325f74d405ddc7b2cb06b0dbfd3b62b9b15 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 05:54:32 -0800 Subject: [PATCH 26/43] refactor(code-review): add rename detection to git change discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step-01 substep-4: - Use git diff -M to detect renamed/moved files - Include deleted, renamed files in git_changed_files - Adversarial reviewer needs to see deletions (e.g., critical code removed) - Downstream steps will handle these appropriately (documented in implementation-notes) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index d402fb9d..63223380 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -70,11 +70,11 @@ Check if git repository exists. ```bash git status --porcelain -git diff --name-only -git diff --cached --name-only +git diff -M --name-only +git diff -M --cached --name-only ``` -Compile `git_changed_files` = union of modified, staged, and new files. +Compile `git_changed_files` = union of modified, staged, new, deleted, and renamed files. ### 5. Cross-Reference Story vs Git From e479b4164cdd9c1fb60d663eb9543f8a35c73464 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:02:17 -0800 Subject: [PATCH 27/43] refactor(code-review): add checkpoint for empty git changes and exclude ignored files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step-01 substeps 5: - If no git changes detected: halt and ask user "Continue anyway?" Allows AC audit on story File List even if no code changes in git - Exclude git-ignored files from discrepancy comparison Prevents false positives if story modified only ignored files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 63223380..efac14d5 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -78,12 +78,21 @@ Compile `git_changed_files` = union of modified, staged, new, deleted, and renam ### 5. Cross-Reference Story vs Git -Compare {story_file_list} with {git_changed_files}: +**If {git_changed_files} is empty:** + +Ask user: "No git changes detected. Continue anyway?" + +- If **no**: HALT the workflow +- If **yes**: Continue to comparison + +**Compare {story_file_list} with {git_changed_files}:** + +Exclude git-ignored files from the comparison (run `git check-ignore` if needed). Set `git_discrepancies` with categories: - **files_in_git_not_story**: Files changed in git but not in story File List -- **files_in_story_not_git**: Files in story File List but no git changes +- **files_in_story_not_git**: Files in story File List but no git changes (excluding git-ignored) - **uncommitted_undocumented**: Uncommitted changes not tracked in story ### 6. Load Project Context From 0ae6799cb6a7f4dc772c1c798c9c2fd636feae1e Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:04:34 -0800 Subject: [PATCH 28/43] refactor(code-review): remove project context loading from step-01 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step-01 focus is: load story + discover git changes. Nothing else. Project context loading belongs in step-04 (Context-Aware Review) where it provides audit rules, principles, and requirements for validating AC implementation against project standards. (See implementation-notes.md for detail) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index efac14d5..25a81a2b 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -95,10 +95,6 @@ Set `git_discrepancies` with categories: - **files_in_story_not_git**: Files in story File List but no git changes (excluding git-ignored) - **uncommitted_undocumented**: Uncommitted changes not tracked in story -### 6. Load Project Context - -- Load {project_context} if exists (**/project-context.md) for coding standards - --- ## NEXT STEP DIRECTIVE @@ -117,7 +113,6 @@ Set `git_discrepancies` with categories: - `story_file_list` compiled from Dev Agent Record - `git_changed_files` discovered via git commands - `git_discrepancies` calculated -- `project_context` loaded if exists - Explicit NEXT directive provided ## FAILURE MODES From 38ab12da85d42591a492333bc40f1b2f935d5741 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:17:37 -0800 Subject: [PATCH 29/43] refactor(code-review): remove cargo cult failure modes repetition from step-01 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FAILURE MODES section was just inverted SUCCESS METRICS. Not valuable. Replaced with single catch-all statement: failure to meet any success metric = failure. Let actual failure modes emerge from usage patterns, not speculation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 25a81a2b..bac25ea1 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -110,17 +110,12 @@ Set `git_discrepancies` with categories: - `story_path` identified and loaded - `story_key` extracted - `story_content` captured completely and unmodified -- `story_file_list` compiled from Dev Agent Record -- `git_changed_files` discovered via git commands +- `story_file_list` compiled from Dev Agent Record (or NO_FILE_LIST if not found) +- `git_changed_files` discovered via git commands (or NO_GIT if not a git repo) - `git_discrepancies` calculated - Explicit NEXT directive provided ## FAILURE MODES -- Proceeding without story file loaded -- Missing `story_key` extraction -- `story_content` incomplete or modified (breaks later steps) -- Skipping git change discovery -- Not calculating discrepancies -- No explicit NEXT directive at step completion +Failure to meet any SUCCESS METRIC constitutes workflow failure. From 4ba6e193037a87a9c4c5a4db32a862de0d684db7 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:29:52 -0800 Subject: [PATCH 30/43] refactor(code-review): rename SUCCESS METRICS to COMPLETION CHECKLIST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correct terminology: - "Metrics" implies quantitative measurement - These are actually pass/fail criteria for step completion - Section is self-validation checklist, not measurement data Reframe as checkpoint before proceeding to next step: - Add "Before proceeding to the next step, verify ALL of the following:" - Change "If any metric" to "If any item" - Explicit instruction: "Do NOT proceed to the next step" if checklist fails 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index bac25ea1..b5bcd2cb 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -105,7 +105,9 @@ Set `git_discrepancies` with categories: --- -## SUCCESS METRICS +## COMPLETION CHECKLIST + +Before proceeding to the next step, verify ALL of the following: - `story_path` identified and loaded - `story_key` extracted @@ -115,7 +117,5 @@ Set `git_discrepancies` with categories: - `git_discrepancies` calculated - Explicit NEXT directive provided -## FAILURE MODES - -Failure to meet any SUCCESS METRIC constitutes workflow failure. +**If any item is not met:** Report to the user immediately and HALT the workflow. Do NOT proceed to the next step. From b3643af6dc3718554e1177e0919c5152553bba08 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:30:31 -0800 Subject: [PATCH 31/43] refactor(code-review): remove redundancy and clarify halt instruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove redundant "Do NOT proceed to the next step" (halt already means this) - Change "item" to "criterion" (more precise terminology) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index b5bcd2cb..1544f917 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -117,5 +117,5 @@ Before proceeding to the next step, verify ALL of the following: - `git_discrepancies` calculated - Explicit NEXT directive provided -**If any item is not met:** Report to the user immediately and HALT the workflow. Do NOT proceed to the next step. +**If any criterion is not met:** Report to the user immediately and HALT the workflow. From 53045d35b1e350fcd8c614a4c93278f9a50740fe Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:31:19 -0800 Subject: [PATCH 32/43] refactor(code-review): move NEXT STEP DIRECTIVE after COMPLETION CHECKLIST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Logical flow: verify checklist → then declare next step Not: declare next step → then verify checklist 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../code-review/steps/step-01-load-story.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 1544f917..ca4b16dc 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -97,14 +97,6 @@ Set `git_discrepancies` with categories: --- -## NEXT STEP DIRECTIVE - -**CRITICAL:** When this step completes, explicitly state: - -"**NEXT:** Loading `step-02-build-attack-plan.md`" - ---- - ## COMPLETION CHECKLIST Before proceeding to the next step, verify ALL of the following: @@ -119,3 +111,11 @@ Before proceeding to the next step, verify ALL of the following: **If any criterion is not met:** Report to the user immediately and HALT the workflow. +--- + +## NEXT STEP DIRECTIVE + +**CRITICAL:** When this step completes, explicitly state: + +"**NEXT:** Loading `step-02-build-attack-plan.md`" + From 1636bd5a55099ea2d67866502c9e7f381c34bb9b Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:33:05 -0800 Subject: [PATCH 33/43] refactor(code-review): remove redundant 'immediately' from halt instruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'Immediately' is implied by HALT. No timing choice exists. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index ca4b16dc..828345bb 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -109,7 +109,7 @@ Before proceeding to the next step, verify ALL of the following: - `git_discrepancies` calculated - Explicit NEXT directive provided -**If any criterion is not met:** Report to the user immediately and HALT the workflow. +**If any criterion is not met:** Report to the user and HALT the workflow. --- From dbdaae1be78600d119f9713628267af3768e7fe0 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 06:34:33 -0800 Subject: [PATCH 34/43] refactor(code-review): remove NEXT directive from completion checklist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checklist validates work done DURING step execution. The NEXT directive is OUTPUT of completion, not a validation criterion. It happens AFTER the checklist passes, so it does not belong there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 828345bb..aa106933 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -107,7 +107,6 @@ Before proceeding to the next step, verify ALL of the following: - `story_file_list` compiled from Dev Agent Record (or NO_FILE_LIST if not found) - `git_changed_files` discovered via git commands (or NO_GIT if not a git repo) - `git_discrepancies` calculated -- Explicit NEXT directive provided **If any criterion is not met:** Report to the user and HALT the workflow. From 9e6e991b53ff95736dbf7788310f9a777cffb481 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:09:56 -0800 Subject: [PATCH 35/43] fix(code-review): correct flow control directive in substep 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed "Skip to substep 6" (which does not exist) to "Proceed to substep 5". Step only has 5 substeps. After setting NO_GIT flag, workflow continues to substep 5 (Cross-Reference Story vs Git), not to a non-existent substep 6. Fixes h2 finding from adversarial review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index aa106933..be474967 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -64,7 +64,7 @@ Extract `story_file_list` from the Dev Agent Record → File List section of {st Check if git repository exists. -**If NOT a git repo:** Set `git_changed_files` = NO_GIT, `git_discrepancies` = NO_GIT. Skip to substep 6. +**If NOT a git repo:** Set `git_changed_files` = NO_GIT, `git_discrepancies` = NO_GIT. Proceed to substep 5. **If git repo detected:** From ec30b580e75d91683b47cb0e8dc3881ec5a4dc9f Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:13:41 -0800 Subject: [PATCH 36/43] refactor(code-review): use Skip to for flow control directive in substep 4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip to substep 5 correctly communicates jumping past the rest of the git discovery logic in substep 4 when git repo is not found. Proceed would suggest normal sequential flow, but we are skipping the conditional branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index be474967..61f65dce 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -64,7 +64,7 @@ Extract `story_file_list` from the Dev Agent Record → File List section of {st Check if git repository exists. -**If NOT a git repo:** Set `git_changed_files` = NO_GIT, `git_discrepancies` = NO_GIT. Proceed to substep 5. +**If NOT a git repo:** Set `git_changed_files` = NO_GIT, `git_discrepancies` = NO_GIT. Skip to substep 5. **If git repo detected:** From 3fc411d9c998bbfd7fdde5eebc9d08b3c75d5aa5 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:19:31 -0800 Subject: [PATCH 37/43] docs(code-review): clarify sprint_status file definition and location --- .../4-implementation/code-review/steps/step-01-load-story.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 61f65dce..951471e2 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -35,7 +35,10 @@ If input resolves to an existing file: - If no → return to user prompt (ask "Which story would you like to review?" again) **Search sprint_status** (if input is not a direct file): -Search for stories with status `review` or `done`. Match by priority: + +The `sprint_status` file (`{implementation_artifacts}/sprint-status.yaml`) is generated by the sprint-planning workflow and contains the development_status map with all epics and stories, along with their current status (backlog, ready-for-dev, in-progress, review, done). + +Search this file for stories with status `review` or `done`. Match by priority: 1. Story number resembles input closely enough (e.g., "1-2" matches "1 2", "1.2", "one dash two", "one two"; "1-32" matches "one thirty two"). Do NOT match if numbers differ (e.g., "1-33" does not match "1-32") 2. Exact story name/key (e.g., "1-2-user-auth-api") 3. Story name/title resembles input closely enough From 551a2ccb53b2087f97c52677dba35b82d17618b3 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:21:44 -0800 Subject: [PATCH 38/43] docs(code-review): use variable reference for sprint-status path --- .../code-review/steps/step-01-load-story.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 951471e2..73aa4bca 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -28,17 +28,14 @@ Ask user: "Which story would you like to review?" **Try input as direct file path first:** If input resolves to an existing file: - - Verify it's in `sprint_status` with status `review` or `done` + - Verify it's in {implementation_artifacts}/sprint-status.yaml with status `review` or `done` - If verified → set `story_path` to that file path - - If NOT verified → Warn user the file is not in sprint_status (or wrong status). Ask: "Continue anyway?" + - If NOT verified → Warn user the file is not in {implementation_artifacts}/sprint-status.yaml (or wrong status). Ask: "Continue anyway?" - If yes → set `story_path` - If no → return to user prompt (ask "Which story would you like to review?" again) -**Search sprint_status** (if input is not a direct file): - -The `sprint_status` file (`{implementation_artifacts}/sprint-status.yaml`) is generated by the sprint-planning workflow and contains the development_status map with all epics and stories, along with their current status (backlog, ready-for-dev, in-progress, review, done). - -Search this file for stories with status `review` or `done`. Match by priority: +**Search {implementation_artifacts}/sprint-status.yaml** (if input is not a direct file): +Search for stories with status `review` or `done`. Match by priority: 1. Story number resembles input closely enough (e.g., "1-2" matches "1 2", "1.2", "one dash two", "one two"; "1-32" matches "one thirty two"). Do NOT match if numbers differ (e.g., "1-33" does not match "1-32") 2. Exact story name/key (e.g., "1-2-user-auth-api") 3. Story name/title resembles input closely enough From 2785d382d5f879ae56d0fd18d9ec6f58dc1ea15a Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:24:11 -0800 Subject: [PATCH 39/43] docs(code-review): use {sprint_status} variable instead of expanded path --- .../code-review/steps/step-01-load-story.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 73aa4bca..38252e41 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -28,13 +28,13 @@ Ask user: "Which story would you like to review?" **Try input as direct file path first:** If input resolves to an existing file: - - Verify it's in {implementation_artifacts}/sprint-status.yaml with status `review` or `done` + - Verify it's in {sprint_status} with status `review` or `done` - If verified → set `story_path` to that file path - - If NOT verified → Warn user the file is not in {implementation_artifacts}/sprint-status.yaml (or wrong status). Ask: "Continue anyway?" + - If NOT verified → Warn user the file is not in {sprint_status} (or wrong status). Ask: "Continue anyway?" - If yes → set `story_path` - If no → return to user prompt (ask "Which story would you like to review?" again) -**Search {implementation_artifacts}/sprint-status.yaml** (if input is not a direct file): +**Search {sprint_status}** (if input is not a direct file): Search for stories with status `review` or `done`. Match by priority: 1. Story number resembles input closely enough (e.g., "1-2" matches "1 2", "1.2", "one dash two", "one two"; "1-32" matches "one thirty two"). Do NOT match if numbers differ (e.g., "1-33" does not match "1-32") 2. Exact story name/key (e.g., "1-2-user-auth-api") From 58e0b6a63486556bd99a28df4fa4469210b5d0a1 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:26:50 -0800 Subject: [PATCH 40/43] docs(code-review): add generic error handling for git commands --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 38252e41..0cecec8e 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -74,6 +74,8 @@ git diff -M --name-only git diff -M --cached --name-only ``` +If any git command fails: Report the error to user and set `git_changed_files` = NO_GIT. + Compile `git_changed_files` = union of modified, staged, new, deleted, and renamed files. ### 5. Cross-Reference Story vs Git From 5a16c3a1029abae9dd1de3ec9f1a3192b8678503 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:27:12 -0800 Subject: [PATCH 41/43] fix(code-review): halt on git command failure instead of silently treating as NO_GIT --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 0cecec8e..0ad72346 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -74,7 +74,7 @@ git diff -M --name-only git diff -M --cached --name-only ``` -If any git command fails: Report the error to user and set `git_changed_files` = NO_GIT. +If any git command fails: Report the error to the user and HALT the workflow. Compile `git_changed_files` = union of modified, staged, new, deleted, and renamed files. From b73670700bd4b26d658024a93d07bd5ef9a288a0 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Mon, 5 Jan 2026 07:39:56 -0800 Subject: [PATCH 42/43] docs(code-review): expand story file validation to include empty and malformed files --- .../4-implementation/code-review/steps/step-01-load-story.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md index 0ad72346..16a0c4df 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-01-load-story.md @@ -49,7 +49,7 @@ Search for stories with status `review` or `done`. Match by priority: ### 2. Load Story File **Load file content:** -Read the complete contents of {story_path} and assign to `story_content` WITHOUT filtering, truncating or summarizing. If {story_path} cannot be read: report the error to the user and HALT the workflow. +Read the complete contents of {story_path} and assign to `story_content` WITHOUT filtering, truncating or summarizing. If {story_path} cannot be read, is empty, or obviously doesn't have the story: report the error to the user and HALT the workflow. **Extract story identifier:** Verify the filename ends with `.md` extension. Remove `.md` to get `story_key` (e.g., "1-2-user-authentication.md" → "1-2-user-authentication"). If filename doesn't end with `.md` or the result is empty: report the error to the user and HALT the workflow. From 59ed596392b03d29afd42977d769c914025ec570 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Tue, 6 Jan 2026 23:01:29 -0800 Subject: [PATCH 43/43] refactor(code-review): swap phase order - adversarial first, context-aware second Reorder dual-phase review so adversarial diff review runs before context-aware review. This ensures fresh-eyes code quality checks happen before story-biased validation. --- ...eview.md => step-02-adversarial-review.md} | 18 ++++---- ...k-plan.md => step-03-build-attack-plan.md} | 42 ++++++++++--------- .../steps/step-04-context-aware-review.md | 7 ++-- .../steps/step-05-consolidate-findings.md | 10 ++--- 4 files changed, 40 insertions(+), 37 deletions(-) rename src/modules/bmm/workflows/4-implementation/code-review/steps/{step-03-adversarial-review.md => step-02-adversarial-review.md} (90%) rename src/modules/bmm/workflows/4-implementation/code-review/steps/{step-02-build-attack-plan.md => step-03-build-attack-plan.md} (74%) diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-adversarial-review.md similarity index 90% rename from src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md rename to src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-adversarial-review.md index 45681fda..b6b24290 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-adversarial-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-adversarial-review.md @@ -1,9 +1,9 @@ --- -name: 'step-03-adversarial-review' -description: 'Context-independent adversarial diff review via subagent - no story knowledge' +name: 'step-02-adversarial-review' +description: 'Lean adversarial review - context-independent diff analysis, no story knowledge' --- -# Step 3: Adversarial Review (Information Asymmetric) +# Step 2: Adversarial Review (Information Asymmetric) **Goal:** Perform context-independent adversarial review of code changes. Reviewer sees ONLY the diff - no story, no ACs, no context about WHY changes were made. @@ -19,13 +19,13 @@ From previous steps: - `{story_path}`, `{story_key}` - `{file_list}` - Files listed in story's File List section -- `{context_aware_findings}` - Findings from Phase 1 +- `{git_changed_files}` - Files changed according to git +- `{baseline_commit}` - From story file Dev Agent Record --- ## STATE VARIABLE (capture now) -- `{baseline_commit}` - From story file Dev Agent Record - `{diff_output}` - Complete diff of changes - `{asymmetric_findings}` - Findings from adversarial review @@ -101,12 +101,12 @@ Add each finding to `{asymmetric_findings}` (no IDs yet - assigned after merge): } ``` -### 4. Phase 2 Summary +### 4. Phase 1 Summary Present adversarial findings: ``` -**Phase 2: Adversarial Review Complete** +**Phase 1: Adversarial Review Complete** **Reviewer Context:** Pure diff review (no story knowledge) **Findings:** {count} @@ -120,7 +120,7 @@ Present adversarial findings: - Noise: {count} - Undecided: {count} -Proceeding to findings consolidation... +Proceeding to attack plan construction... ``` --- @@ -129,7 +129,7 @@ Proceeding to findings consolidation... **CRITICAL:** When this step completes, explicitly state: -"**NEXT:** Loading `step-04-context-aware-review.md`" +"**NEXT:** Loading `step-03-build-attack-plan.md`" --- diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-build-attack-plan.md similarity index 74% rename from src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md rename to src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-build-attack-plan.md index 13907a12..a14a8010 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-02-build-attack-plan.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-03-build-attack-plan.md @@ -1,23 +1,24 @@ --- -name: 'step-02-build-attack-plan' -description: 'Extract ACs and tasks, create comprehensive review plan for both phases' +name: 'step-03-build-attack-plan' +description: 'Extract ACs and tasks, create comprehensive review plan for context-aware phase' --- -# Step 2: Build Review Attack Plan +# Step 3: Build Review Attack Plan -**Goal:** Extract all reviewable items from story and create attack plan for both review phases. +**Goal:** Extract all reviewable items from story and create attack plan for context-aware review phase. --- ## AVAILABLE STATE -From previous step: +From previous steps: - `{story_path}` - Path to the story file - `{story_key}` - Story identifier - `{story_file_list}` - Files claimed in story - `{git_changed_files}` - Files actually changed (git) - `{git_discrepancies}` - Differences between claims and reality +- `{asymmetric_findings}` - Findings from Phase 1 (adversarial review) --- @@ -26,7 +27,7 @@ From previous step: - `{acceptance_criteria}` - All ACs extracted from story - `{tasks_with_status}` - All tasks with their [x] or [ ] status - `{comprehensive_file_list}` - Union of story files + git files -- `{review_attack_plan}` - Structured plan for both phases +- `{review_attack_plan}` - Structured plan for context-aware phase --- @@ -82,7 +83,11 @@ Exclude from review: Structure the `{review_attack_plan}`: ``` -PHASE 1: Context-Aware Review (Step 3) +PHASE 1: Adversarial Review (Step 2) [COMPLETE - {asymmetric_findings} findings] +├── Fresh code review without story context +│ └── {asymmetric_findings} items to consolidate + +PHASE 2: Context-Aware Review (Step 4) ├── Git vs Story Discrepancies │ └── {git_discrepancies} items ├── AC Validation @@ -91,12 +96,6 @@ PHASE 1: Context-Aware Review (Step 3) │ └── {tasks_with_status} marked [x] to verify └── Code Quality Review └── {comprehensive_file_list} files to review - -PHASE 2: Adversarial Asymmetric Review (Step 4) -└── Context-independent diff review - └── Diff constructed from git changes - └── Story file EXCLUDED from reviewer context - └── Pure code quality assessment ``` ### 5. Preview Attack Plan @@ -107,12 +106,15 @@ Present to user (brief summary): **Review Attack Plan** **Story:** {story_key} -**ACs to verify:** {count} -**Tasks marked complete:** {count} -**Files to review:** {count} -**Git discrepancies detected:** {count} -Proceeding with dual-phase review... +**Phase 1 (Adversarial - Complete):** {asymmetric_findings count} findings from fresh review +**Phase 2 (Context-Aware - Starting):** + - ACs to verify: {count} + - Tasks marked complete: {count} + - Files to review: {count} + - Git discrepancies detected: {count} + +Proceeding with context-aware review... ``` --- @@ -121,7 +123,7 @@ Proceeding with dual-phase review... **CRITICAL:** When this step completes, explicitly state: -"**NEXT:** Loading `step-03-adversarial-review.md`" +"**NEXT:** Loading `step-04-context-aware-review.md`" --- @@ -131,7 +133,7 @@ Proceeding with dual-phase review... - All tasks extracted with completion status - Comprehensive file list built (story + git) - Exclusions applied correctly -- Attack plan structured for both phases +- Attack plan structured for context-aware phase - Summary presented to user - Explicit NEXT directive provided diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md index ca145736..af0702b6 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-04-context-aware-review.md @@ -20,6 +20,7 @@ From previous steps: - `{story_file_list}`, `{git_changed_files}`, `{git_discrepancies}` - `{acceptance_criteria}`, `{tasks_with_status}` - `{comprehensive_file_list}`, `{review_attack_plan}` +- `{asymmetric_findings}` - From Phase 1 (adversarial review) --- @@ -132,12 +133,12 @@ Re-examine for: --- -## PHASE 1 SUMMARY +## PHASE 2 SUMMARY Present context-aware findings: ``` -**Phase 1: Context-Aware Review Complete** +**Phase 2: Context-Aware Review Complete** **Findings:** {count} - Critical: {count} @@ -145,7 +146,7 @@ Present context-aware findings: - Medium: {count} - Low: {count} -Proceeding to Phase 2: Adversarial Review... +Proceeding to findings consolidation... ``` Store `{context_aware_findings}` for consolidation in step 5. diff --git a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md index 2c310e5f..6370f441 100644 --- a/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md +++ b/src/modules/bmm/workflows/4-implementation/code-review/steps/step-05-consolidate-findings.md @@ -5,7 +5,7 @@ description: 'Merge and deduplicate findings from both review phases' # Step 5: Consolidate Findings -**Goal:** Merge findings from context-aware review (Phase 1) and adversarial review (Phase 2), deduplicate, and present unified findings table. +**Goal:** Merge findings from adversarial review (Phase 1) and context-aware review (Phase 2), deduplicate, and present unified findings table. --- @@ -14,8 +14,8 @@ description: 'Merge and deduplicate findings from both review phases' From previous steps: - `{story_path}`, `{story_key}` -- `{context_aware_findings}` - Findings from Phase 1 (step 3) -- `{asymmetric_findings}` - Findings from Phase 2 (step 4) +- `{asymmetric_findings}` - Findings from Phase 1 (step 2 - adversarial review) +- `{context_aware_findings}` - Findings from Phase 2 (step 4 - context-aware review) --- @@ -123,8 +123,8 @@ Build `{consolidated_findings}`: --- **Phase Sources:** -- Context-Aware (Phase 1): {count} findings -- Adversarial (Phase 2): {count} findings +- Adversarial (Phase 1): {count} findings +- Context-Aware (Phase 2): {count} findings ``` ---