Compare commits

...

14 Commits

Author SHA1 Message Date
Alex Verkhovsky 060d5562a4 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
2026-01-05 04:24:29 -08:00
Alex Verkhovsky 2bd6e9df1b 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
2026-01-05 04:14:14 -08:00
Alex Verkhovsky 6886e3c8cd refactor(code-review): clarify step-01 description and NO_GIT handling 2026-01-05 02:54:00 -08:00
Alex Verkhovsky 1f5700ea14 refactor(code-review): remove unused thisStepFile/nextStepFile from frontmatter 2026-01-05 02:37:00 -08:00
Alex Verkhovsky 9700da9dc6 refactor(code-review): remove input_file_patterns from workflow.md to prevent context leak 2026-01-05 01:14:37 -08:00
Alex Verkhovsky 0f18c4bcba refactor(code-review): replace discover_inputs protocol with explicit file loading 2026-01-05 01:12:35 -08:00
Alex Verkhovsky ae9b83388c 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
2026-01-05 01:09:38 -08:00
Alex Verkhovsky 64c32d8c8c refactor(code-review): add web_bundle: false, use "Read and follow" wording
- 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 <noreply@anthropic.com>
2026-01-05 00:39:17 -08:00
Alex Verkhovsky eae4ad46a1 refactor(code-review): remove unused validation path and checklist 2026-01-04 21:38:04 -08:00
Alex Verkhovsky a8758b0393 refactor(code-review): remove CRITICAL DIRECTIVES, add communication_language 2026-01-04 21:32:10 -08:00
Alex Verkhovsky ac081a27e8 docs(code-review): clarify step file loading in workflow architecture 2026-01-04 21:15:04 -08:00
Alex Verkhovsky 7c914ae8b2 refactor(code-review): inline single-use adversarial task path 2026-01-04 21:05:48 -08:00
Alex Verkhovsky dadca29b09 refactor(code-review): use installed_path variable in step files 2026-01-04 21:00:18 -08:00
Alex Verkhovsky 25f93a3b64 refactor(code-review): simplify workflow.md 2026-01-04 20:59:58 -08:00
8 changed files with 68 additions and 119 deletions

View File

@ -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}}_

View File

@ -1,27 +1,21 @@
---
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'
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)
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`
---
@ -29,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. 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
4. Story description resembles input closely enough
- 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
@ -53,13 +60,15 @@ 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:
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
@ -67,26 +76,21 @@ 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
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
- **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
---
@ -100,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
- Context documents loaded
- `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

View File

@ -1,10 +1,6 @@
---
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
@ -125,7 +121,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`"
---

View File

@ -1,13 +1,9 @@
---
name: 'step-04-adversarial-review'
name: 'step-03-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)
# 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.
@ -133,7 +129,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`"
---

View File

@ -1,13 +1,9 @@
---
name: 'step-03-context-aware-review'
name: 'step-04-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
# Step 4: Context-Aware Review
**Goal:** Perform story-aware validation - verify AC implementation, audit task completion, review code quality with full story context.
@ -37,6 +33,16 @@ Initialize `{context_aware_findings}` as empty list.
## EXECUTION SEQUENCE
### 0. Load Planning Context (JIT)
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.
### 1. Git vs Story Discrepancies
Review `{git_discrepancies}` and create findings:
@ -150,7 +156,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`"
---

View File

@ -1,10 +1,6 @@
---
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

View File

@ -1,9 +1,6 @@
---
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

View File

@ -1,23 +1,16 @@
---
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.'
web_bundle: false
---
# Code Review Workflow
**Goal:** Execute a comprehensive two-phase code review that validates story claims AND performs context-independent adversarial analysis.
## WORKFLOW ARCHITECTURE: STEP FILES
**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"
- 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}`
- Dual-phase review: context-aware (step 3) + adversarial asymmetric (step 4)
---
@ -31,32 +24,16 @@ 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`
- `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
<critical>YOU ARE AN ADVERSARIAL CODE REVIEWER - Find what's wrong or missing!</critical>
<critical>Your purpose: Validate story file claims against actual implementation</critical>
<critical>Challenge everything: Are tasks marked [x] actually done? Are ACs really implemented?</critical>
<critical>Find 3-10 specific issues in every review minimum - no lazy "looks good" reviews</critical>
<critical>Read EVERY file in the File List - verify implementation against story requirements</critical>
<critical>Tasks marked complete but not done = CRITICAL finding</critical>
<critical>Acceptance Criteria not implemented = HIGH severity finding</critical>
<critical>Exclude `_bmad/`, `_bmad-output/`, `.cursor/`, `.windsurf/`, `.claude/` from review</critical>
---
## 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.