fix(bmad-dev-story): unify review findings handoff between code-review and dev-story#2447
fix(bmad-dev-story): unify review findings handoff between code-review and dev-story#2447oneby-wang wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR transitions the code-review and dev-story workflow from handling "AI-Review"/"Review Follow-ups (AI)" tasks to an explicit "Review Findings" approach marked with ChangesReview Findings Workflow Transition
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
why move it back to ready instead of in progress @oneby-wang ? |
|
Hi @bmadcode, thanks for reviewing. I think this is a good topic to discuss. I moved the story back to ready-for-dev because I believe we're currently missing a dedicated state for stories that require changes after review. In this case, the review findings require fairly significant updates, so I wanted to make it clear that additional development work is needed before the story is ready for another review cycle. Keeping the story in in-progress introduces some ambiguity, as there is no clear distinction between:
We could keep such stories in in-progress, but that would likely require additional logic (for example, "in-progress + unresolved review findings") to identify them correctly. Even then, it would still be difficult to distinguish between stories actively being worked on and stories that failed review but have not yet been picked up for rework. Also, even without introducing a new state, moving the story back to ready-for-dev while retaining the review findings provides a fairly clear signal that the review did not pass and that further development work is required. In my view, that is still more explicit than leaving the story in in-progress. If we conclude that a dedicated workflow state is needed for this scenario, I think it would be better to address that separately in a dedicated PR. |
What
Unifies the code review rework loop around the
Review Findingssection sobmad-dev-storyconsumes the same findings thatbmad-code-reviewwrites. Code review now sends unresolved patch action items back toready-for-devinstead ofin-progress.Why
Senior Developer Review (AI)andReview Follow-ups (AI)overlapped withReview Findings, creating a fragile CR -> DS handoff. This makes the flow simpler and keeps decision-needed findings in the existing code-review decision path.How
bmad-code-reviewStep 4 to return stories with unresolved review work toready-for-devand clarify the next-step prompt.bmad-dev-storyto detectReview Findings, halt on unresolved[Review][Decision], and prioritize unchecked[Review][Patch]findings.Review Findingscheckboxes.Testing
Ran
npm run validate:skillsandnpm run lint:md. Also verified oldSenior Developer Review,Review Follow-ups, and[AI-Review]markers no longer remain inbmad-dev-story.