From 95853b3bdd9ed97b45c859acae8809aa2986b316 Mon Sep 17 00:00:00 2001 From: pbean Date: Tue, 14 Jul 2026 21:14:34 -0700 Subject: [PATCH] fix(sprintstatus): recognize split-story keys with a letter suffix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BMAD splits an oversized story into 2-6a/2-6b, but STORY_RE required a digits-only story number, so split stories fell into unknown_keys and were silently dropped: invisible to run/--story/the TUI tree and skipped by advance()'s epic-lift — with the loop walking on to later, possibly dependent, stories. The suffix is a first-class Story/StorySelector field rather than an uncaptured widening (which would have let --story 2-6a dispatch 2-6b and rendered both halves identically in the TUI): 2-6a / 2.6a / --epic 2 --story 6a select exactly that half, while a plain 2-6 matches the whole split family in file order. run/--dry-run now also print a stderr warning when sprint-status keys remain unparseable instead of only journaling them. Fixes #144 --- CHANGELOG.md | 8 +++ src/bmad_loop/cli.py | 23 ++++++-- src/bmad_loop/sprintstatus.py | 28 +++++++--- src/bmad_loop/tui/widgets.py | 2 +- tests/test_cli.py | 44 +++++++++++++++ tests/test_sprintstatus.py | 88 ++++++++++++++++++++++++++++++ tests/test_sprintstatus_advance.py | 18 ++++++ tests/test_tui_app.py | 11 ++++ 8 files changed, 207 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2333de2..1ef56fe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,14 @@ PATH)`, the TUI notifies `multiplexer backend unavailable — launch/attach disa ### Fixed +- **Split-story keys (`2-6a-…`) are no longer silently skipped (#144).** The sprint-status + parser rejected story numbers carrying BMAD's split-story letter suffix, dropping exactly the + stories that were split to be loop-tractable — invisible to `run`/`--story`/the TUI tree, and + skipped by the epic-lift. The suffix is now a first-class `Story`/selector field: `--story 2-6a` + (or `2.6a`, or `--epic 2 --story 6a`) selects exactly that half, while a plain `2-6` selects the + whole `2-6a`/`2-6b` family in file order. `run` and `--dry-run` also print a stderr warning when + sprint-status keys remain unparseable, instead of only journaling them. + - **Review leg repairs a finalize-tail death.** A review session that died between writing its terminal `## Auto Run Result` (`Status: done`) and flipping the spec frontmatter off the transient `in-review` marker left the orchestrator re-reviewing already-finished work — a burned review diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index f729dec1..82f95132 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -538,6 +538,18 @@ def _validate_stories_queue( problems.extend(stories_probs) +def _warn_unknown_keys(ss: sprintstatus.SprintStatus) -> None: + """Surface sprint-status keys the parser could not classify. Silently + dropping one reads to the operator as "that story is done, or not mine to + do" (issue #144) — so `run`/`--dry-run` say it out loud; the journal's + ``sprint-status-unknown-keys`` event stays the durable record.""" + if ss.unknown_keys: + print( + f"warning: ignoring unparseable sprint-status keys: {', '.join(ss.unknown_keys)}", + file=sys.stderr, + ) + + def cmd_run(args: argparse.Namespace) -> int: if (rc := _reject_bad_run_id(args.run_id)) is not None: return rc @@ -566,9 +578,9 @@ def cmd_run(args: argparse.Namespace) -> int: return 1 else: try: - sprintstatus.select_actionable( - sprintstatus.load(paths.sprint_status), args.epic, args.story - ) + ss = sprintstatus.load(paths.sprint_status) + _warn_unknown_keys(ss) + sprintstatus.select_actionable(ss, args.epic, args.story) except sprintstatus.SprintStatusError as e: print(e, file=sys.stderr) return 1 @@ -660,6 +672,7 @@ def render(role: str, prompt: str) -> str: return _render_invocation(pol, paths.project, role, prompt) ss = sprintstatus.load(paths.sprint_status) + _warn_unknown_keys(ss) try: queue = sprintstatus.select_actionable(ss, args.epic, args.story) except sprintstatus.SprintStatusError as e: @@ -1837,8 +1850,8 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: run_p.add_argument("--epic", type=int, help="only stories from this epic (sprint mode)") run_p.add_argument( "--story", - help="story: E-S / E.S, a slug fragment, or full key (sprint mode); " - "a story id (stories mode)", + help="story: E-S / E.S (split suffix ok, e.g. 2-6a), a slug fragment, " + "or full key (sprint mode); a story id (stories mode)", ) run_p.add_argument("--max-stories", type=int, help="stop after N stories") run_p.add_argument("--dry-run", action="store_true", help="print the plan, spawn nothing") diff --git a/src/bmad_loop/sprintstatus.py b/src/bmad_loop/sprintstatus.py index 40a87c0d..f3693226 100644 --- a/src/bmad_loop/sprintstatus.py +++ b/src/bmad_loop/sprintstatus.py @@ -18,9 +18,11 @@ EPIC_RE = re.compile(r"^epic-(\d+)$") RETRO_RE = re.compile(r"^epic-(\d+)-retrospective$") RETRO_ITEM_RE = re.compile(r"^epic-(\d+)-retro-item-(\d+)-(.+)$") -STORY_RE = re.compile(r"^(\d+)-(\d+)-(.+)$") -SHORT_REF_RE = re.compile(r"^(\d+)[-.](\d+)$") # short story ref: 3-1 or 3.1 -BARE_NUM_RE = re.compile(r"^(\d+)$") # a lone story number, needs --epic +# The story number may carry a single lowercase split suffix (2-6a / 2-6b — +# the shape BMAD produces when an oversized story is split, see issue #144). +STORY_RE = re.compile(r"^(\d+)-(\d+)([a-z]?)-(.+)$") +SHORT_REF_RE = re.compile(r"^(\d+)[-.](\d+)([a-z]?)$") # short story ref: 3-1, 3.1, 3-1a +BARE_NUM_RE = re.compile(r"^(\d+)([a-z]?)$") # a lone story number, needs --epic STORY_STATUSES = {"backlog", "ready-for-dev", "in-progress", "review", "done"} # Lifecycle order, earliest -> latest. `advance` never moves a story backward @@ -41,6 +43,7 @@ class Story: num: int slug: str status: str + suffix: str = "" # split-story letter ("a" in 2-6a), "" for a whole story @dataclass(frozen=True) @@ -111,8 +114,9 @@ def load(path: Path) -> SprintStatus: key=key, epic=int(m.group(1)), num=int(m.group(2)), - slug=m.group(3), + slug=m.group(4), status=status, + suffix=m.group(3), ) ) else: @@ -237,7 +241,9 @@ class StorySelector: * full key ``3-1-user-auth`` — exact match * short ref ``3-1`` / ``3.1`` — epic 3, story 1 (any slug) - * bare number ``1`` with ``--epic 3`` — epic 3, story 1 + * suffixed short ref ``2-6a`` / ``2.6a`` — exactly the ``a`` half of a + split story; the plain ``2-6`` matches the whole ``2-6a``/``2-6b`` family + * bare number ``1`` (or ``6a``) with ``--epic 3`` — epic 3, story 1 (or 6a) * slug fragment ``user-auth`` / ``auth`` — substring of the slug (must be unique) * epic only (``--epic 3``, blank story) — every story in the epic """ @@ -246,6 +252,7 @@ class StorySelector: num: int | None = None key: str | None = None # exact full key slug: str | None = None # slug substring + suffix: str | None = None # split-story letter; None matches any suffix @property def is_targeted(self) -> bool: @@ -260,6 +267,8 @@ def matches(self, story: Story) -> bool: return False if self.num is not None and story.num != self.num: return False + if self.suffix is not None and story.suffix != self.suffix: + return False if self.slug is not None and self.slug not in story.slug: return False return True @@ -280,20 +289,21 @@ def _check_epic(parsed_epic: int) -> None: f"--epic {epic} conflicts with story '{text}' (epic {parsed_epic})" ) + # empty suffix group -> None: a plain `2-6` matches the whole split family if m := STORY_RE.match(text): # full key 3-1-slug e, n = int(m.group(1)), int(m.group(2)) _check_epic(e) - return StorySelector(epic=e, num=n, key=text) - if m := SHORT_REF_RE.match(text): # 3-1 / 3.1 + return StorySelector(epic=e, num=n, key=text, suffix=m.group(3) or None) + if m := SHORT_REF_RE.match(text): # 3-1 / 3.1 / 3-1a e, n = int(m.group(1)), int(m.group(2)) _check_epic(e) - return StorySelector(epic=e, num=n) + return StorySelector(epic=e, num=n, suffix=m.group(3) or None) if m := BARE_NUM_RE.match(text): # bare story number, needs --epic if epic is None: raise SprintStatusError( f"ambiguous story '{text}': use --epic E --story {text}, or E-{text}" ) - return StorySelector(epic=epic, num=int(m.group(1))) + return StorySelector(epic=epic, num=int(m.group(1)), suffix=m.group(2) or None) return StorySelector(epic=epic, slug=text) # slug fragment diff --git a/src/bmad_loop/tui/widgets.py b/src/bmad_loop/tui/widgets.py index 26828654..5e58870a 100644 --- a/src/bmad_loop/tui/widgets.py +++ b/src/bmad_loop/tui/widgets.py @@ -286,7 +286,7 @@ def _short(value: Any, limit: int = 60) -> str: def sprint_story_label(story: Story) -> Text: glyph = SPRINT_GLYPHS.get(story.status, "?") style = SPRINT_STYLES.get(story.status, "dim") - return Text(f"{glyph} {story.num}-{story.slug}", style=style) + return Text(f"{glyph} {story.num}{story.suffix}-{story.slug}", style=style) def sprint_retro_label(status: str) -> Text: diff --git a/tests/test_cli.py b/tests/test_cli.py index d637b1e3..ee163e7f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -107,6 +107,50 @@ def test_dry_run_selects_story_by_short_ref(project, capsys, epic, story): assert "3-2-foo" not in out and "4-1-bar" not in out +@pytest.mark.parametrize( + "story,expected", + [ + ("2-6a", ["2-6a-build-structure"]), + ("2.6a", ["2-6a-build-structure"]), + ("2-6a-build-structure", ["2-6a-build-structure"]), + ("2-6", ["2-6a-build-structure", "2-6b-extend-structure"]), # whole family + ], +) +def test_dry_run_selects_split_story(project, capsys, story, expected): + # split-story keys (issue #144): visible, selectable, and suffix-exact + write_sprint( + project, + { + "2-6a-build-structure": "backlog", + "2-6b-extend-structure": "backlog", + "2-7-later": "backlog", + }, + ) + _write_policy(project.project) + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + args = argparse.Namespace(epic=None, story=story, max_stories=None) + + assert cli._dry_run(project, pol, args) == 0 + out = capsys.readouterr().out + assert [k for k in ("2-6a-build-structure", "2-6b-extend-structure") if k in out] == expected + assert "2-7-later" not in out + + +def test_dry_run_warns_unknown_keys(project, capsys): + write_sprint( + project, + {"1-1-a": "ready-for-dev", "totally-weird": "huh", "2-6.1-nested-split": "backlog"}, + ) + _write_policy(project.project) + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + args = argparse.Namespace(epic=None, story=None, max_stories=None) + + assert cli._dry_run(project, pol, args) == 0 + err = capsys.readouterr().err + assert "warning: ignoring unparseable sprint-status keys: " in err + assert "totally-weird" in err and "2-6.1-nested-split" in err + + def test_dry_run_reports_targeted_not_actionable(project, capsys): write_sprint(project, {"3-1-user-auth": "ready-for-dev", "3-2-foo": "done"}) _write_policy(project.project) diff --git a/tests/test_sprintstatus.py b/tests/test_sprintstatus.py index 4de19572..d318bf71 100644 --- a/tests/test_sprintstatus.py +++ b/tests/test_sprintstatus.py @@ -32,6 +32,36 @@ def test_load_classifies_keys(project): assert ss.unknown_keys == ("weird-key",) +def test_load_split_story_keys(project): + # BMAD splits an oversized story into 2-6a / 2-6b (issue #144); both halves + # must parse as stories — not fall into unknown_keys — and keep file order. + write_sprint( + project, + { + "2-5-intact": "done", + "2-6a-build-structure": "backlog", + "2-6b-extend-structure": "backlog", + "2-7-later": "backlog", + "2-6ab-not-a-split": "backlog", # multi-letter: not the convention + "2-6A-not-lower": "backlog", # uppercase: not the convention + }, + ) + ss = sprintstatus.load(project.sprint_status) + assert [s.key for s in ss.stories] == [ + "2-5-intact", + "2-6a-build-structure", + "2-6b-extend-structure", + "2-7-later", + ] + a, b = ss.stories[1], ss.stories[2] + assert (a.epic, a.num, a.suffix, a.slug) == (2, 6, "a", "build-structure") + assert (b.epic, b.num, b.suffix, b.slug) == (2, 6, "b", "extend-structure") + assert ss.stories[0].suffix == "" # whole stories carry no suffix + assert ss.unknown_keys == ("2-6ab-not-a-split", "2-6A-not-lower") + # the split halves are picked in file order, before later stories + assert sprintstatus.next_actionable(ss).key == "2-6a-build-structure" + + def test_load_classifies_retro_items(project): write_sprint( project, @@ -121,6 +151,21 @@ def test_parse_selector_forms(): assert sel.epic == 3 and not sel.is_targeted +def test_parse_selector_split_suffix(): + # every numeric form carries the split suffix through to the selector + sel = sprintstatus.parse_selector(None, "2-6a-build-structure") + assert (sel.epic, sel.num, sel.suffix, sel.key) == (2, 6, "a", "2-6a-build-structure") + for ref in ("2-6a", "2.6a"): + sel = sprintstatus.parse_selector(None, ref) + assert (sel.epic, sel.num, sel.suffix, sel.key, sel.slug) == (2, 6, "a", None, None) + sel = sprintstatus.parse_selector(2, "6a") + assert (sel.epic, sel.num, sel.suffix) == (2, 6, "a") + # suffix-less forms leave suffix None — the whole-family wildcard + for epic, ref in [(None, "2-6"), (None, "2.6"), (2, "6"), (None, "2-6-whole-slug")]: + sel = sprintstatus.parse_selector(epic, ref) + assert sel.suffix is None, ref + + def test_parse_selector_bare_number_needs_epic(): with pytest.raises(sprintstatus.SprintStatusError, match="ambiguous story '1'"): sprintstatus.parse_selector(None, "1") @@ -149,6 +194,49 @@ def test_select_actionable_short_ref_and_epic_story(project): ] +def test_select_actionable_split_suffix(project): + write_sprint( + project, + { + "2-6a-build-structure": "backlog", + "2-6b-extend-structure": "backlog", + "2-7-later": "backlog", + }, + ) + ss = sprintstatus.load(project.sprint_status) + # a suffixed ref selects exactly its half — never the sibling + for epic, story in [(None, "2-6a"), (None, "2.6a"), (2, "6a")]: + got = sprintstatus.select_actionable(ss, epic, story) + assert [s.key for s in got] == ["2-6a-build-structure"] + assert [s.key for s in sprintstatus.select_actionable(ss, None, "2.6b")] == [ + "2-6b-extend-structure" + ] + # the plain short ref selects the whole split family, in file order + assert [s.key for s in sprintstatus.select_actionable(ss, None, "2-6")] == [ + "2-6a-build-structure", + "2-6b-extend-structure", + ] + # a suffix that doesn't exist matches nothing + with pytest.raises(sprintstatus.SprintStatusError, match="no story matches '2-6c'"): + sprintstatus.select_actionable(ss, None, "2-6c") + + +def test_select_actionable_split_suffix_not_actionable(project): + write_sprint( + project, + {"2-6a-build-structure": "done", "2-6b-extend-structure": "backlog"}, + ) + ss = sprintstatus.load(project.sprint_status) + with pytest.raises( + sprintstatus.SprintStatusError, + match=r"story 2-6a matched 2-6a-build-structure but its status is 'done'", + ): + sprintstatus.select_actionable(ss, None, "2-6a") + # the family ref still finds the remaining actionable half + got = sprintstatus.select_actionable(ss, None, "2-6") + assert [s.key for s in got] == ["2-6b-extend-structure"] + + def test_select_actionable_targeted_not_actionable(project): write_sprint(project, {"3-1-user-auth": "ready-for-dev", "3-2-foo": "done"}) ss = sprintstatus.load(project.sprint_status) diff --git a/tests/test_sprintstatus_advance.py b/tests/test_sprintstatus_advance.py index 31d072e2..a42830d9 100644 --- a/tests/test_sprintstatus_advance.py +++ b/tests/test_sprintstatus_advance.py @@ -37,6 +37,24 @@ def test_advance_to_in_progress_lifts_backlog_epic(tmp_path): assert sprintstatus.load(p).epics[3] == "in-progress" # epic lifted +def test_advance_split_story_lifts_backlog_epic(tmp_path): + # a split-story key (issue #144) must advance and lift its epic like any other + text = ( + "last_updated: 01-06-2026 10:00\n" + "development_status:\n" + " epic-2: backlog\n" + " 2-6a-build-structure: backlog\n" + " 2-6b-extend-structure: backlog\n" + ) + p = tmp_path / "sprint-status.yaml" + p.write_text(text, encoding="utf-8") + out = sprintstatus.advance(p, "2-6a-build-structure", "in-progress") + assert out == "in-progress" + assert sprintstatus.story_status(p, "2-6a-build-structure") == "in-progress" + assert sprintstatus.load(p).epics[2] == "in-progress" # epic lifted + assert sprintstatus.story_status(p, "2-6b-extend-structure") == "backlog" # sibling untouched + + def test_advance_preserves_comments_and_structure(tmp_path): p = _write(tmp_path) sprintstatus.advance(p, "3-2-digest-delivery", "in-progress") diff --git a/tests/test_tui_app.py b/tests/test_tui_app.py index 54128c4d..a138bf8d 100644 --- a/tests/test_tui_app.py +++ b/tests/test_tui_app.py @@ -67,6 +67,7 @@ journal_line, pause_label, pause_tag, + sprint_story_label, story_checkpoint_cell, story_state_cell, ) @@ -1483,6 +1484,16 @@ def test_pause_tag_and_label_render(): assert label == "escalation" and "red" in style +def test_sprint_story_label_split_suffix(): + # split halves (issue #144) must render distinctly: 6a-… / 6b-…, not both 6-… + from bmad_loop.sprintstatus import Story + + whole = Story(key="2-5-intact", epic=2, num=5, slug="intact", status="done") + half = Story(key="2-6a-build", epic=2, num=6, slug="build", status="backlog", suffix="a") + assert sprint_story_label(whole).plain == "✓ 5-intact" + assert sprint_story_label(half).plain == "· 6a-build" + + def test_story_cells_render(): assert story_state_cell("done").plain == "✓ done" assert story_state_cell("sentinel:unresolved").plain.startswith("⚠")