Commit Graph

64 Commits

Author SHA1 Message Date
Alex Verkhovsky 59ed596392 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.
2026-01-06 23:29:41 -08:00
Alex Verkhovsky b73670700b docs(code-review): expand story file validation to include empty and malformed files 2026-01-05 07:39:56 -08:00
Alex Verkhovsky 5a16c3a102 fix(code-review): halt on git command failure instead of silently treating as NO_GIT 2026-01-05 07:27:12 -08:00
Alex Verkhovsky 58e0b6a634 docs(code-review): add generic error handling for git commands 2026-01-05 07:26:50 -08:00
Alex Verkhovsky 2785d382d5 docs(code-review): use {sprint_status} variable instead of expanded path 2026-01-05 07:24:11 -08:00
Alex Verkhovsky 551a2ccb53 docs(code-review): use variable reference for sprint-status path 2026-01-05 07:21:44 -08:00
Alex Verkhovsky 3fc411d9c9 docs(code-review): clarify sprint_status file definition and location 2026-01-05 07:19:31 -08:00
Alex Verkhovsky ec30b580e7 refactor(code-review): use Skip to for flow control directive in substep 4
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 <noreply@anthropic.com>
2026-01-05 07:13:41 -08:00
Alex Verkhovsky 9e6e991b53 fix(code-review): correct flow control directive in substep 4
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 <noreply@anthropic.com>
2026-01-05 07:09:56 -08:00
Alex Verkhovsky dbdaae1be7 refactor(code-review): remove NEXT directive from completion checklist
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 <noreply@anthropic.com>
2026-01-05 06:34:33 -08:00
Alex Verkhovsky 1636bd5a55 refactor(code-review): remove redundant 'immediately' from halt instruction
'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 <noreply@anthropic.com>
2026-01-05 06:33:05 -08:00
Alex Verkhovsky 53045d35b1 refactor(code-review): move NEXT STEP DIRECTIVE after COMPLETION CHECKLIST
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 <noreply@anthropic.com>
2026-01-05 06:31:19 -08:00
Alex Verkhovsky b3643af6dc refactor(code-review): remove redundancy and clarify halt instruction
- 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 <noreply@anthropic.com>
2026-01-05 06:30:31 -08:00
Alex Verkhovsky 4ba6e19303 refactor(code-review): rename SUCCESS METRICS to COMPLETION CHECKLIST
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 <noreply@anthropic.com>
2026-01-05 06:29:52 -08:00
Alex Verkhovsky 38ab12da85 refactor(code-review): remove cargo cult failure modes repetition from step-01
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 <noreply@anthropic.com>
2026-01-05 06:17:37 -08:00
Alex Verkhovsky 0ae6799cb6 refactor(code-review): remove project context loading from step-01
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 <noreply@anthropic.com>
2026-01-05 06:04:34 -08:00
Alex Verkhovsky e479b4164c refactor(code-review): add checkpoint for empty git changes and exclude ignored files
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 <noreply@anthropic.com>
2026-01-05 06:02:17 -08:00
Alex Verkhovsky 71a1c325f7 refactor(code-review): add rename detection to git change discovery
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 <noreply@anthropic.com>
2026-01-05 05:54:32 -08:00
Alex Verkhovsky 59c58b2e2c refactor(code-review): clean up step-01 substep 3 and add error handling
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 <noreply@anthropic.com>
2026-01-05 05:47:46 -08:00
Alex Verkhovsky 18ac3c931a refactor(code-review): audit step-01 substeps and success/failure criteria
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 <noreply@anthropic.com>
2026-01-05 05:42:04 -08:00
Alex Verkhovsky 8fc7db7b97 refactor(code-review): remove implementation notes from step-01
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 <noreply@anthropic.com>
2026-01-05 05:33:27 -08:00
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
Alex Verkhovsky 5fcdae02b5 refactor(code-review): defer finding IDs until consolidation 2026-01-04 05:33:21 -08:00
Alex Verkhovsky b8eeb78cff refactor(adversarial-review): simplify severity/validity classification 2026-01-04 04:13:46 -08:00
Alex Verkhovsky b628eec9fd refactor(code-review): simplify adversarial review task invocation 2026-01-04 04:07:23 -08:00
Alex Verkhovsky f5d949b922 feat(dev-story): capture baseline commit for code-review diff 2026-01-04 03:04:56 -08:00
Alex Verkhovsky 6d1d7d0e72 fix(adversarial-review): add tech-spec exclusion and read-only notes 2026-01-04 02:12:02 -08:00
Alex Verkhovsky 8b6a053d2e fix(code-review): simplify diff exclusion to implementation_artifacts only 2026-01-04 01:41:20 -08:00
Alex Verkhovsky 460c27e29a refactor(code-review): convert to sharded format with dual-phase review
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 <noreply@anthropic.com>
2026-01-04 01:07:58 -08:00
Nick Pirocanac a297235862
fix: *code-review is picking up non-application files (#1232)
Co-authored-by: Nick Pirocanac <nick.pirocanac@rtktickets.com>
2026-01-01 13:22:51 +08:00
Brian Madison 949cf64d3b workflow status, init and phase 4 items aligned with planning_artifacts and implementation_artifacts for discovery and output 2025-12-27 10:59:44 +08:00
Brian Madison 12dd97fe9b start implementing new bm paths into phases 1-4 2025-12-23 11:40:38 +08:00
Brian Madison 25c79e3fe5 folder rename from .bmad to _bmad 2025-12-13 16:22:34 +08:00
Alex Verkhovsky e2d9d35ce9
fix(bmm): improve code review completion message (#1095)
Change "Story is ready for next work!" to "Code review complete!"

The original phrasing was misleading - when a code review finishes
with status "done", it means the review itself is complete and the
story is marked done in tracking. However, the user may choose to
do additional reviews or the story may genuinely be finished.
"Code review complete" more accurately describes what actually
happened without implying next steps.
2025-12-12 06:42:52 +08:00
Brian Madison 446a0359ab fix bmb workflow paths 2025-12-10 20:50:24 +09:00
Alex Verkhovsky c95b65f462
fix(bmm): correct code-review workflow status logic and checklist (#1015) (#1028)
- Fix checklist to only accept 'review' status (not 'ready-for-review')
- Include MEDIUM issues in done/in-progress status determination
- Initialize and track fixed_count/action_count variables for summary
- Add sprint-status.yaml sync when story status changes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
2025-12-05 21:27:11 -06:00
Nguyen Quang Huy 72ef9e9722
fix: use backticks for quoted phrase in code-review description (#1025)
Replace 'looks good' with `looks good` to avoid nested single quote
issues when IDEs generate command files from workflow YAML.

Co-authored-by: Brian <bmadcode@gmail.com>
2025-12-05 21:26:04 -06:00