fix: disambiguate same-numbered stories across epics (build-cmd + orchestrator-helper)#47
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
WalkthroughThe CLI now accepts an optional ChangesEpic-scoped story key flow
Sequence Diagram(s)sequenceDiagram
participant build_cmd as "build-cmd in tmux.py"
participant normalize_story_key_for_epic as "normalize_story_key_for_epic"
participant render_step_prompt as "render_step_prompt"
participant auto_prompt as "skills/bmad-story-automator/data/prompts/auto.md"
build_cmd->>normalize_story_key_for_epic: derive epic-scoped story_key from --epic
build_cmd->>render_step_prompt: pass story_key into step prompt rendering
render_step_prompt->>auto_prompt: replace {{story_key}} with the resolved story_key
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (1)
skills/bmad-story-automator/src/story_automator/commands/tmux.py (1)
202-208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate epic-resolution logic.
This block (
num = re.split(r"[.\-]", story_id)[-1]→normalize_story_key_for_epic(...)) duplicates_resolve_with_epicinorchestrator.py(Lines 399-405). Consider extracting a shared helper instory_keys.pyso the two surfaces can't drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py` around lines 202 - 208, The epic-resolution logic in the tmux command duplicates the same story-key handling already implemented in Orchestrator._resolve_with_epic, so refactor it into a shared helper in story_keys.py and have both call sites use that helper. Keep the behavior for deriving the numeric suffix from story_id and passing it through normalize_story_key_for_epic, but centralize the logic so StoryAutomator and Orchestrator stay aligned and cannot drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@skills/bmad-story-automator/src/story_automator/commands/tmux.py`:
- Around line 202-208: The epic-resolution logic in the tmux command duplicates
the same story-key handling already implemented in
Orchestrator._resolve_with_epic, so refactor it into a shared helper in
story_keys.py and have both call sites use that helper. Keep the behavior for
deriving the numeric suffix from story_id and passing it through
normalize_story_key_for_epic, but centralize the logic so StoryAutomator and
Orchestrator stay aligned and cannot drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78bffe4c-a521-487c-bb6c-3f4c0e4f9d08
📒 Files selected for processing (8)
scripts/smoke-test.shskills/bmad-story-automator/data/prompts/auto.mdskills/bmad-story-automator/src/story_automator/commands/orchestrator.pyskills/bmad-story-automator/src/story_automator/commands/tmux.pyskills/bmad-story-automator/src/story_automator/core/prompt_rendering.pyskills/bmad-story-automator/steps-c/step-03a-execute-review.mdtests/test_normalize_story_key.pytests/test_state_policy_metadata.py
…hestrator-helper)
|
Addressed the duplicate epic-resolution nitpick: extracted |
cannt
force-pushed
the
fix/epic-qualified-story-resolution
branch
from
7fe0e7e to
7e32301
Compare
1 participant
Problem
When two epics each contain a story with the same bare number (e.g. epic
foostory 6.1 and epicbarstory 6.1),build-cmdandorchestrator-helper'ssprint-status get/normalize-keyresolve a story by its bare number / prefix. In a multi-epic sprint that can match the wrong epic's story:Fix
The codebase already disambiguates stories by epic via
normalize_story_key_for_epic()(core/story_keys.py) — it is used throughout the epic internals (orchestrator_epic_agents.py,core/epic_parser.py,core/sprint.py) and is unit-tested. But two command surfaces do not thread the epic through and fall back to bare-number / prefix resolution:build-cmd(worker spawn) andorchestrator-helper'snormalize-key/sprint-status get.This PR makes those two surfaces consistent with the rest of the codebase: it threads an optional
--epicinto them and routes through the samenormalize_story_key_for_epic(). A new{{story_key}}prompt variable lets the worker target the exact story file instead of aprefix-*glob. No new resolution logic is introduced.Backward compatibility
Without
--epic, behavior is unchanged —{{story_key}}falls back to{{story_prefix}}, and resolution falls back to the existingnormalize_story_key.Changes
core/prompt_rendering.py— optionalstory_keyparam +{{story_key}}replacementcommands/tmux.py— parse--epicinbuild-cmd, resolve the epic-qualified key, pass it oncommands/orchestrator.py— accept--epiconsprint-status getandnormalize-key; two small helpersdata/prompts/auto.md— target{{story_key}}.md; warn against cross-epic number collisionssteps-c/step-03a-execute-review.md— pass--epic {epic}intobuild-cmdscripts/smoke-test.sh— update the twobuild-cmdassertions to expect--epic {epic}Tests
Added:
_resolve_with_epicdisambiguates a bare number across two epics, and falls back to the plain resolver without an epic.build-cmd auto 1 --epic <e>renders the matching epic's exact story file and not the other epic's.npm run pack:dry-run,npm run test:cli, andnpm run test:smokeall pass.python3 -m unittest discover -s tests→ 413/414. The one failing test (test_build_cmd_uses_legacy_ai_command_consistently_for_claude, assertingclaude --printvs--dangerously-skip-permissions) also fails on a clean checkout ofmainand is unrelated to this change.No new dependencies.
Summary by CodeRabbit
New Features
Bug Fixes
Tests