chore: convert code-review workflow.yaml to workflow.md#1860
Merged
alexeyv merged 2 commits intobmad-code-org:mainfrom Mar 8, 2026
Merged
chore: convert code-review workflow.yaml to workflow.md#1860alexeyv merged 2 commits intobmad-code-org:mainfrom
alexeyv merged 2 commits intobmad-code-org:mainfrom
Conversation
Step 6 of 9 in yaml-to-md conversion plan. Merges workflow.yaml config and instructions.xml content into a single workflow.md following the established conversion pattern. Updates agent yaml and module-help.csv references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🤖 Augment PR SummarySummary: This PR migrates the Phase 4 Changes:
Technical Notes: Workflow discovery/parsing relies on YAML frontmatter in 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe PR migrates the code-review workflow from a YAML configuration with separate XML instructions to a unified Markdown-based workflow format, updating agent workflow references from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/bmm/workflows/4-implementation/code-review/workflow.md (5)
97-98: Consider removing profanity from workflow document."placeholder bullshit" (line 97) may be intentional for adversarial tone, but could be problematic for enterprise adoption or professional settings. Consider alternatives like "placeholder assertions" or "fake tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/workflow.md` around lines 97 - 98, Replace the profane phrase in the heading "4. **Test Quality**: Real tests vs placeholder bullshit" with a professional alternative; edit the heading text to something like "4. **Test Quality**: Real tests vs placeholder placeholder tests" or "4. **Test Quality**: Real tests vs placeholder assertions" (or "fake tests") so the document remains enterprise-friendly while keeping the original intent; update only the heading string while preserving the surrounding markdown structure.
56-58: Missing HALT conditions for critical failures.The EXECUTION section doesn't define HALT conditions for scenarios like: story file not found, story file unreadable, malformed story structure, or inability to determine git status. Other workflows in this directory define explicit halt conditions. Consider adding them for robustness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/workflow.md` around lines 56 - 58, Add explicit HALT conditions to the EXECUTION workflow block to cover critical failures: define HALT entries for "story file not found", "story file unreadable", "malformed story structure" and "unable to determine git status" inside the <workflow> EXECUTION section; each HALT should reference the failing check (e.g., file lookup/IO parse/validation/git status), include a descriptive error message, set a non-retryable terminal status, and (where possible) include diagnostic details (path, parse error, git error) so callers can act on the failure.
189-210: No fallback for invalid user input.The
<ask>block accepts "[1], [2], or specify which issue" but only three<check>blocks exist for choices 1, 2, and 3. If the user enters invalid input or specifies an issue number, there's no defined behavior. Add a fallback or clarify expected input format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/workflow.md` around lines 189 - 210, The ask block lacks a fallback for invalid input; update the workflow by adding a default `<check if="otherwise">` (or similar fallback condition) after the existing `<check if="user chooses 1">`, `<check if="user chooses 2">`, and `<check if="user chooses 3">` blocks to handle invalid responses or explicit issue numbers: in that block return a clear error/help message prompting valid options (e.g., "Please choose 1, 2, or 3 or specify an issue ID"), optionally loop back to the `<ask>` or provide guidance on accepted formats, and ensure it sets {{action_count}} and {{fixed_count}} appropriately for the fallback path so downstream logic is stable; reference the `<ask>` tag and the three `<check if="user chooses X">` blocks when making the change.
267-267: Inconsistent templating syntax: Handlebars vs XML conditionals.The entire workflow uses XML-style
<check if="...">for conditionals, but line 267 switches to Handlebars syntax{{#ifnew_status == "done"}}. This inconsistency could confuse the executing agent. Convert to the prevailing XML pattern for consistency.♻️ Proposed fix
- {{`#if` new_status == "done"}}Code review complete!{{else}}Address the action items and continue development.{{/if}} + <check if="{{new_status}} == 'done'">Code review complete!</check> + <check if="{{new_status}} != 'done'">Address the action items and continue development.</check>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/workflow.md` at line 267, Replace the stray Handlebars conditional "{{`#if` new_status == "done"}}Code review complete!{{else}}Address the action items and continue development.{{/if}}" with the project's XML-style conditional used elsewhere (e.g. the <check if="..."> pattern); specifically, use a <check if="new_status == 'done'"> block that outputs "Code review complete!" and an appropriate <otherwise> (or matching else-style sibling) that outputs "Address the action items and continue development." so the conditional matches the existing XML templating convention.
69-74: Git commands lack error handling for non-git environments.The workflow runs git commands assuming: (1) git is installed, (2) current directory is a git repository. While line 69 checks "if git repository exists," the commands on lines 70-73 could still fail if git CLI is unavailable. Consider adding a fallback or explicit error message when git is not available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/workflow.md` around lines 69 - 74, The workflow assumes git is available when executing the actions under the "check if git repository exists" block (the actions that run `git status --porcelain`, `git diff --name-only`, and `git diff --cached --name-only`), so add an explicit guard that verifies the git CLI is runnable (e.g., attempt `git --version` or check PATH) and handle failures by emitting a clear error/fallback: if git is not found, return a descriptive error message and either (a) fall back to a filesystem-based changed-files detection (stat/mtime or VCS-agnostic diff) or (b) abort the step gracefully; update the "check if git repository exists" logic to catch command errors and ensure the subsequent compile-list-of-changed-files action only runs when the git check succeeded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bmm/workflows/4-implementation/code-review/workflow.md`:
- Around line 60-65: Step 1 references the input variable {{story_path}} but
it's not declared in the workflow inputs; add {{story_path}} to the workflow's
Inputs table or configuration section and document it as an expected parameter
(e.g., string path to story file), update any initialization/usage notes where
"Use provided {{story_path}} or ask user..." appears, and ensure related symbols
like {{story_key}} extraction logic (the filename-to-key mapping) are noted to
rely on {{story_path}} being present.
- Around line 154-180: Unify severity terminology across this workflow: decide
whether "CRITICAL" equals "HIGH" or is a distinct top-level severity, then
update the Categorize action, any conditional logic (e.g., the step that sets
categories and the "Fix" actions), and the Output template so labels match
everywhere; specifically, change the <action>Categorize findings: HIGH (must
fix), MEDIUM (should fix), LOW (nice to fix)</action> or the output headings
(e.g., "## 🔴 CRITICAL ISSUES") so they use the same term, update references
like "Fix all HIGH and MEDIUM issues" to include CRITICAL if it is distinct (or
replace CRITICAL with HIGH everywhere if synonymous), and ensure the variables
{{high_count}}/{{critical_count}} and any decision logic (e.g., the step at line
~126 that logs "CRITICAL finding") are renamed/normalized (functions/steps:
Categorize findings, the output block headings, and the "Fix" action text) so
all places consistently reference the chosen severity names.
- Line 20: Markdown lint MD037 is triggered by unescaped underscores in the
folder names; update the text in workflow.md to escape the underscores in the
folder references by replacing `_bmad/` and `_bmad-output/` with `\_bmad/` and
`\_bmad-output/` (also search for any other occurrences of `_bmad` or
`_bmad-output` in the same file and escape them) so the underscores are treated
as literal characters rather than emphasis markers.
---
Nitpick comments:
In `@src/bmm/workflows/4-implementation/code-review/workflow.md`:
- Around line 97-98: Replace the profane phrase in the heading "4. **Test
Quality**: Real tests vs placeholder bullshit" with a professional alternative;
edit the heading text to something like "4. **Test Quality**: Real tests vs
placeholder placeholder tests" or "4. **Test Quality**: Real tests vs
placeholder assertions" (or "fake tests") so the document remains
enterprise-friendly while keeping the original intent; update only the heading
string while preserving the surrounding markdown structure.
- Around line 56-58: Add explicit HALT conditions to the EXECUTION workflow
block to cover critical failures: define HALT entries for "story file not
found", "story file unreadable", "malformed story structure" and "unable to
determine git status" inside the <workflow> EXECUTION section; each HALT should
reference the failing check (e.g., file lookup/IO parse/validation/git status),
include a descriptive error message, set a non-retryable terminal status, and
(where possible) include diagnostic details (path, parse error, git error) so
callers can act on the failure.
- Around line 189-210: The ask block lacks a fallback for invalid input; update
the workflow by adding a default `<check if="otherwise">` (or similar fallback
condition) after the existing `<check if="user chooses 1">`, `<check if="user
chooses 2">`, and `<check if="user chooses 3">` blocks to handle invalid
responses or explicit issue numbers: in that block return a clear error/help
message prompting valid options (e.g., "Please choose 1, 2, or 3 or specify an
issue ID"), optionally loop back to the `<ask>` or provide guidance on accepted
formats, and ensure it sets {{action_count}} and {{fixed_count}} appropriately
for the fallback path so downstream logic is stable; reference the `<ask>` tag
and the three `<check if="user chooses X">` blocks when making the change.
- Line 267: Replace the stray Handlebars conditional "{{`#if` new_status ==
"done"}}Code review complete!{{else}}Address the action items and continue
development.{{/if}}" with the project's XML-style conditional used elsewhere
(e.g. the <check if="..."> pattern); specifically, use a <check if="new_status
== 'done'"> block that outputs "Code review complete!" and an appropriate
<otherwise> (or matching else-style sibling) that outputs "Address the action
items and continue development." so the conditional matches the existing XML
templating convention.
- Around line 69-74: The workflow assumes git is available when executing the
actions under the "check if git repository exists" block (the actions that run
`git status --porcelain`, `git diff --name-only`, and `git diff --cached
--name-only`), so add an explicit guard that verifies the git CLI is runnable
(e.g., attempt `git --version` or check PATH) and handle failures by emitting a
clear error/fallback: if git is not found, return a descriptive error message
and either (a) fall back to a filesystem-based changed-files detection
(stat/mtime or VCS-agnostic diff) or (b) abort the step gracefully; update the
"check if git repository exists" logic to catch command errors and ensure the
subsequent compile-list-of-changed-files action only runs when the git check
succeeded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e32333c5-4acc-44aa-b575-64d1d5d4fd86
⛔ Files ignored due to path filters (1)
src/bmm/module-help.csvis excluded by!**/*.csv
📒 Files selected for processing (5)
src/bmm/agents/dev.agent.yamlsrc/bmm/agents/quick-flow-solo-dev.agent.yamlsrc/bmm/workflows/4-implementation/code-review/instructions.xmlsrc/bmm/workflows/4-implementation/code-review/workflow.mdsrc/bmm/workflows/4-implementation/code-review/workflow.yaml
💤 Files with no reviewable changes (2)
- src/bmm/workflows/4-implementation/code-review/workflow.yaml
- src/bmm/workflows/4-implementation/code-review/instructions.xml
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
workflow.yamlconfig andinstructions.xmlcontent into a singleworkflow.mdfollowing the established conversion pattern (retrospective, sprint-status)dev.agent.yaml,quick-flow-solo-dev.agent.yaml) andmodule-help.csvto point to the new.mdfileTest plan
workflow.mdcontains all config variables, input file patterns, and behavioral constraints from the original yaml+xmlworkflow.md,checklist.md, andbmad-skill-manifest.yamlremain in the directorycode-review/workflow.yamlin the codebase🤖 Generated with Claude Code