chore(sprint-planning): convert workflow.yaml to unified workflow.md#1856
Conversation
…ified workflow.md Step 4 of 9 in the yaml-to-md conversion plan. Merges config and instructions into a single self-contained workflow.md, dropping engine scaffolding (critical tags, invoke-protocol).
🤖 Augment PR SummarySummary: Converts the Sprint Planning workflow from a split Changes:
Technical Notes: Workflow still FULL_LOADs epics from planning artifacts and generates 🤖 Was this summary useful? React with 👍 or 👎 |
📝 WalkthroughWalkthroughThe sprint-planning workflow is being restructured by consolidating its configuration from a separate YAML file into the markdown documentation. The workflow.yaml is removed entirely, and the workflow.md is expanded with explicit initialization, configuration sections, path definitions, and a clearer Scrum Master role orientation, reorganizing the execution flow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/bmm/workflows/4-implementation/sprint-planning/workflow.md (8)
117-120:⚠️ Potential issue | 🟠 MajorThis lookup only works for a flat stories directory.
Checking
{story_location_absolute}/{story-key}.mdassumes every story file sits directly under{implementation_artifacts}. The dev-story workflow searches by story-key pattern within{implementation_artifacts}, so nested story folders are already a supported shape elsewhere. Sprint planning will under-detect ready stories in those repos.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 117 - 120, The sprint-planning story file lookup currently only checks a single path pattern `{story_location_absolute}/{story-key}.md`, which misses nested story files; update the detection logic to mirror the dev-story workflow by performing a recursive/glob search for files matching `**/{story-key}.md` under `{implementation_artifacts}` (or otherwise use the same search routine used by dev-story), so `story-key` matches are discovered in nested folders and statuses are correctly upgraded to at least `ready-for-dev`.
122-126:⚠️ Potential issue | 🟠 MajorStatus preservation cannot work without reading the current
{status_file}first.The workflow promises to preserve more advanced existing statuses, but there is no step that loads and parses the current sprint-status file before recomputing entries. As written, reruns rebuild from epics and filesystem only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 122 - 126, The preservation rule refers to an existing {status_file} but the workflow never reads it; add a step to load and parse the current {status_file} into an in-memory map (e.g., existingStatuses) before recomputing sprint entries, then merge new statuses with existingStatuses so that any existing "more advanced" state is kept and downgrades are prevented (ensure merge logic enforces "never downgrade" semantics when combining computedStatus -> existingStatuses); update the recompute step (where entries are built from epics/filesystem) to consult this map and write the merged result back to {status_file}.
187-200:⚠️ Potential issue | 🟠 MajorValidate before replacing the existing sprint-status file.
Line 187 writes
{status_file}first, and only Lines 193-200 validate it. If the YAML is malformed or the status map is inconsistent, the workflow has already clobbered the previous good file. Generate in memory or a temp file, validate, then replace atomically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 187 - 200, The workflow currently writes to {status_file} before performing the validation in step n="5", risking loss of the previous good file; instead, write the complete sprint status YAML to a temporary buffer or temp file, perform the validation checks listed in step n="5" (presence of all epics and stories, retrospectives, no extraneous items, legal status values, and valid YAML syntax, and ordering: epic → its stories → its retrospective → next epic), and only after all validations pass atomically replace {status_file} (e.g., write adjacent temp file and fs.rename/atomic move) so the original file is never clobbered on validation failure.
202-217:⚠️ Potential issue | 🟡 MinorUse one placeholder name for the "epics in progress" count.
Line 206 defines
{{in_progress_count}}, but Line 216 renders{{epics_in_progress_count}}. One of those values will be unset, so the completion summary is wrong or empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 202 - 217, The template uses two different placeholders for the same metric: {{in_progress_count}} (defined) and {{epics_in_progress_count}} (rendered); pick one name and make both occurrences consistent—either change the header/count definition to {{epics_in_progress_count}} or update the summary to use {{in_progress_count}} so the "Epics in Progress" value is populated (locate the placeholders in the sprint-planning workflow.md and make them identical).
58-77:⚠️ Potential issue | 🟠 MajorThe sharded discovery branch is documented, but not actually executed.
Lines 61-65 describe reading
epics/index.mdand then every listed shard, but Line 76 only performs a flat scan for{epics_pattern}under{epics_location}. A project that only has the sharded layout will never be fully loaded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 58 - 77, The workflow currently only performs a flat scan using `{epics_pattern}` under `{epics_location}` and never follows the sharded-index branch; update the implementation for the "Epic Discovery Process" and the `<step n="1" goal="Parse epic files and extract all work items">` actions so that if an `epics/index.md` (or any detected index file) is present you parse that index (read `index.md`) and then explicitly iterate and load each shard listed (e.g., `epic-1.md`, `epic-2.md`), merging their content before falling back to the flat `{epics_pattern}` scan; ensure the code checks for both whole-document and sharded layouts and respects the stated priority ("If both whole and sharded versions exist, use the whole document") using the existing `{epics_location}` and `{epics_pattern}` variables.
79-92:⚠️ Potential issue | 🟠 MajorAdd collision detection for generated story keys.
After slugging titles into kebab-case, two distinct headers can collapse to the same key. The same thing happens if both a whole epic document and its shards are loaded. YAML will keep only the last value, so one story disappears silently unless you validate uniqueness before writing
development_status.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 79 - 92, When generating kebab-case story keys (from epic/story headers) before writing the development_status mapping, add collision detection: maintain a global set/map of generated keys (include source identifier like filename and header text), and on each new key check for existence; if a duplicate is found, either fail with a clear error/log mentioning the conflicting key and both sources or generate a deterministic disambiguator (e.g., append -2, -3) and record the mapping so YAML won't silently overwrite an earlier story; ensure this check runs in the epic/story extraction routine that converts `### Story 1.1: ...` to `1-1-...` and before writing the `development_status` output.
192-200:⚠️ Potential issue | 🟠 MajorThe validator is stricter than the official sprint-status reader.
sprint-status/workflow.mdstill accepts legacy story statusdraftedand epic statuscontexted, but this workflow's legality check excludes them. Rerunning sprint planning on an older project can now fail even though the downstream reader still supports those values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 192 - 200, The sprint-planning validator in the "Validate and report" step is stricter than the official sprint-status reader because it disallows legacy statuses 'drafted' (story) and 'contexted' (epic); update the legality check so it aligns with sprint-status/workflow.md — either by adding 'drafted' and 'contexted' to the allowed status set used by the check (referenced in the Validate and report step and any function performing "All status values are legal") or by loading the canonical allowed-status list from the sprint-status reader/state-machine definitions so the validator accepts the same legacy values.
114-131:⚠️ Potential issue | 🟠 MajorFile existence is not enough to detect the current story status.
This logic can infer
ready-for-dev, but it cannot recoverin-progress,review, ordonefrom story files. If{status_file}is missing or stale, stories already in flight get flattened back toready-for-deveven though the story documents themselves carry richer state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 114 - 131, The current step n="3" logic only checks existence of `{story_location_absolute}/{story-key}.md` and upgrades to `ready-for-dev`, which incorrectly flattens stories already `in-progress`, `review`, or `done`; update the detection to parse the story document and other sources for richer state before deciding: inspect the story file for frontmatter/status fields or in-body markers (e.g., "status:", badges, or explicit `in-progress`/`review`/`done` tokens), check associated `{status_file}` and preserve any more advanced status, and optionally consult VCS/PR metadata (branch names or open PRs) to infer `in-progress`/`review`; ensure the code that computes finalStatus never downgrades an existing more-advanced status and apply this in the routine currently using `story_location_absolute`, `{story-key}.md`, and `{status_file}`.
🧹 Nitpick comments (1)
src/bmm/workflows/4-implementation/sprint-planning/workflow.md (1)
18-25:document_output_languageis loaded, but the workflow never applies it.The chat response is tied to
{communication_language}, but the generated YAML comments and notes are hardcoded in English. If this conversion is supposed to preserve both config channels, the document-generation path is still ignoring one of them.Also applies to: 209-225
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md` around lines 18 - 25, The workflow loads document_output_language but never uses it when emitting generated YAML comments/notes (they remain hardcoded English); update the document-generation path so functions that render output (e.g., the code that produces YAML comments/notes—look for routines named generate_documentation, render_yaml_comments, build_notes, or similar) accept and apply document_output_language in addition to communication_language, then use that locale to format/translate all YAML comments and notes (either by passing document_output_language into the templating/translation layer or invoking the translator with that variable) so generated documents are emitted in the configured document_output_language while agent messages continue using communication_language.
🤖 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/sprint-planning/workflow.md`:
- Around line 36-44: The epics glob is too narrow: update the configuration so
epics_pattern (and optionally epics_location) matches filenames like
bmm-epics.md and user-stories.md as described in the docs; for example replace
the current epics_pattern = `epic*.md` with a broader pattern (e.g., `*epic*.md`
or `*epics*.md`), or change epics_pattern to accept a list of patterns including
`*epic*.md` and `user-stories.md`; ensure related references to
`planning_artifacts` and `status_file` remain unchanged and that the new pattern
is consistent with the "Input Files" examples on lines referencing epics in the
workflow.
- Around line 1-4: Search the repository for the literal string "workflow.yaml"
and replace it with "workflow.md" where referenced (notably in sm.agent.yaml and
module-help.csv), ensuring the agent configuration key that loads the workflow
and the help registry entry point now point to "workflow.md" instead of
"workflow.yaml"; run a quick grep to confirm no remaining "workflow.yaml"
occurrences and update any tests or docs that reference the old filename.
---
Outside diff comments:
In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md`:
- Around line 117-120: The sprint-planning story file lookup currently only
checks a single path pattern `{story_location_absolute}/{story-key}.md`, which
misses nested story files; update the detection logic to mirror the dev-story
workflow by performing a recursive/glob search for files matching
`**/{story-key}.md` under `{implementation_artifacts}` (or otherwise use the
same search routine used by dev-story), so `story-key` matches are discovered in
nested folders and statuses are correctly upgraded to at least `ready-for-dev`.
- Around line 122-126: The preservation rule refers to an existing {status_file}
but the workflow never reads it; add a step to load and parse the current
{status_file} into an in-memory map (e.g., existingStatuses) before recomputing
sprint entries, then merge new statuses with existingStatuses so that any
existing "more advanced" state is kept and downgrades are prevented (ensure
merge logic enforces "never downgrade" semantics when combining computedStatus
-> existingStatuses); update the recompute step (where entries are built from
epics/filesystem) to consult this map and write the merged result back to
{status_file}.
- Around line 187-200: The workflow currently writes to {status_file} before
performing the validation in step n="5", risking loss of the previous good file;
instead, write the complete sprint status YAML to a temporary buffer or temp
file, perform the validation checks listed in step n="5" (presence of all epics
and stories, retrospectives, no extraneous items, legal status values, and valid
YAML syntax, and ordering: epic → its stories → its retrospective → next epic),
and only after all validations pass atomically replace {status_file} (e.g.,
write adjacent temp file and fs.rename/atomic move) so the original file is
never clobbered on validation failure.
- Around line 202-217: The template uses two different placeholders for the same
metric: {{in_progress_count}} (defined) and {{epics_in_progress_count}}
(rendered); pick one name and make both occurrences consistent—either change the
header/count definition to {{epics_in_progress_count}} or update the summary to
use {{in_progress_count}} so the "Epics in Progress" value is populated (locate
the placeholders in the sprint-planning workflow.md and make them identical).
- Around line 58-77: The workflow currently only performs a flat scan using
`{epics_pattern}` under `{epics_location}` and never follows the sharded-index
branch; update the implementation for the "Epic Discovery Process" and the
`<step n="1" goal="Parse epic files and extract all work items">` actions so
that if an `epics/index.md` (or any detected index file) is present you parse
that index (read `index.md`) and then explicitly iterate and load each shard
listed (e.g., `epic-1.md`, `epic-2.md`), merging their content before falling
back to the flat `{epics_pattern}` scan; ensure the code checks for both
whole-document and sharded layouts and respects the stated priority ("If both
whole and sharded versions exist, use the whole document") using the existing
`{epics_location}` and `{epics_pattern}` variables.
- Around line 79-92: When generating kebab-case story keys (from epic/story
headers) before writing the development_status mapping, add collision detection:
maintain a global set/map of generated keys (include source identifier like
filename and header text), and on each new key check for existence; if a
duplicate is found, either fail with a clear error/log mentioning the
conflicting key and both sources or generate a deterministic disambiguator
(e.g., append -2, -3) and record the mapping so YAML won't silently overwrite an
earlier story; ensure this check runs in the epic/story extraction routine that
converts `### Story 1.1: ...` to `1-1-...` and before writing the
`development_status` output.
- Around line 192-200: The sprint-planning validator in the "Validate and
report" step is stricter than the official sprint-status reader because it
disallows legacy statuses 'drafted' (story) and 'contexted' (epic); update the
legality check so it aligns with sprint-status/workflow.md — either by adding
'drafted' and 'contexted' to the allowed status set used by the check
(referenced in the Validate and report step and any function performing "All
status values are legal") or by loading the canonical allowed-status list from
the sprint-status reader/state-machine definitions so the validator accepts the
same legacy values.
- Around line 114-131: The current step n="3" logic only checks existence of
`{story_location_absolute}/{story-key}.md` and upgrades to `ready-for-dev`,
which incorrectly flattens stories already `in-progress`, `review`, or `done`;
update the detection to parse the story document and other sources for richer
state before deciding: inspect the story file for frontmatter/status fields or
in-body markers (e.g., "status:", badges, or explicit
`in-progress`/`review`/`done` tokens), check associated `{status_file}` and
preserve any more advanced status, and optionally consult VCS/PR metadata
(branch names or open PRs) to infer `in-progress`/`review`; ensure the code that
computes finalStatus never downgrades an existing more-advanced status and apply
this in the routine currently using `story_location_absolute`, `{story-key}.md`,
and `{status_file}`.
---
Nitpick comments:
In `@src/bmm/workflows/4-implementation/sprint-planning/workflow.md`:
- Around line 18-25: The workflow loads document_output_language but never uses
it when emitting generated YAML comments/notes (they remain hardcoded English);
update the document-generation path so functions that render output (e.g., the
code that produces YAML comments/notes—look for routines named
generate_documentation, render_yaml_comments, build_notes, or similar) accept
and apply document_output_language in addition to communication_language, then
use that locale to format/translate all YAML comments and notes (either by
passing document_output_language into the templating/translation layer or
invoking the translator with that variable) so generated documents are emitted
in the configured document_output_language while agent messages continue using
communication_language.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b178910a-4469-49a1-a782-43179004c761
📒 Files selected for processing (2)
src/bmm/workflows/4-implementation/sprint-planning/workflow.mdsrc/bmm/workflows/4-implementation/sprint-planning/workflow.yaml
💤 Files with no reviewable changes (1)
- src/bmm/workflows/4-implementation/sprint-planning/workflow.yaml
Triage of Review FindingsAll 4 inline review threads have been replied to and resolved. Here is the triage summary for the "outside diff range" findings from CodeRabbit's review body:
|
- Update stale workflow.yaml references to workflow.md in sm.agent.yaml and module-help.csv - Widen epics_pattern from epic*.md to *epic*.md to match documented input files - Fix placeholder mismatch: epics_in_progress_count → in_progress_count Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
workflow.yamlconfig andinstructions.mdinto a single self-containedworkflow.mdfor the sprint-planning workflow<critical>tags, step 0.5invoke-protocol) — behavioral instructions fully preservedTest plan