From 59ed596392b03d29afd42977d769c914025ec570 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Tue, 6 Jan 2026 23:01:29 -0800 Subject: [PATCH] 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 ``` ---