feat(stories): TUI HITL surface + docs + E2E matrix (Phase 4) - #82
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✨ 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 |
🤖 Augment PR SummarySummary: Adds the stories-mode human-in-the-loop (HITL) surface to the TUI, expands docs, and introduces a full sandbox E2E test matrix for stories mode. Changes:
Technical Notes: Stories mode runs from typed 🤖 Was this summary useful? React with 👍 or 👎 |
| set -e | ||
| rd="$BMAD_LOOP_RUN_DIR"; tid="$BMAD_LOOP_TASK_ID" | ||
| story="$BMAD_LOOP_STORY_KEY"; folder="$BMAD_LOOP_SPEC_FOLDER" | ||
| ts=$(date +%s%N) |
There was a problem hiding this comment.
tests/test_stories_e2e.py:49 — the fake CLI uses date +%s%N, which is GNU-date specific; on macOS/BSD date doesn’t support %N, and this can produce a non-numeric timestamp and invalid JSON/filenames, breaking the E2E tests. Consider switching to a portable timestamp source so the skip condition doesn’t inadvertently allow failing runs on macOS when tmux is present.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| task = state.tasks.get(state.paused_story_key) if state.paused_story_key else None | ||
| if task is None or not task.spec_file: | ||
| return None, "" | ||
| path = Path(task.spec_file) |
There was a problem hiding this comment.
src/bmad_loop/tui/app.py:687 — StoryTask.spec_file can be persisted as a worktree-relative path (see StoryTask._serialized_spec_file()), but _paused_spec() treats it as CWD-relative, so the pause viewers may fail to load the spec under worktree isolation. Consider resolving relative spec_file values against the task’s worktree_path (or another stable base) before reading.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| from rich.text import Text | ||
|
|
||
| from .. import bmadconfig, deferredwork, sprintstatus | ||
| from .. import bmadconfig, deferredwork, sprintstatus, stories |
There was a problem hiding this comment.
src/bmad_loop/tui/data.py
In stories_overview() (around the new return stories.story_rows(folder) call), story_rows() can raise OSError if a story spec disappears mid-scan (e.g., sentinel auto-delete during re-arm), which would currently bubble out and could break the dashboard poll/render. Consider catching OSError alongside StoriesError so the stories board degrades to “unavailable” instead of crashing.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
4d4841f to
457ae90
Compare
28fa41c to
07bf711
Compare
52df1bd to
98e2546
Compare
The presentation + E2E-matrix portion of stories mode, stacked on the Phase 2/3 engine/CLI/resolve layer in #77. Split from the original single Phase-4 commit so the shared status-projection helpers + mode-aware CLI status/dry-run land in #77 (the code Phase 2/3 depends on) and this branch carries only the TUI surface, the prose docs, and the E2E test matrix. TUI (every action calls the same CLI code paths — no duplicated logic): - Stories board replaces the sprint tree for a stories-mode run (state glyph, id, independent spec/done checkpoint cell, title); per-run pause-kind badge + a global "N need attention" count on the runs table. - `p` opens the stage-appropriate viewer: plan-checkpoint spec review (Approve & resume / Request replan), story-checkpoint summary card (Continue / Stop), escalation with story context + blocking condition + sentinel indicator (Resolve / Re-arm & resume), and a gate spec viewer the spec-approval/epic pauses reuse. - Start-run modal gains a source select (prefilled from [stories]) + spec-folder input with a live schedule preview; buttons docked so they stay reachable. Docs: README dual-flow section + command/keybinding updates, FEATURES.md, tui-guide HITL pages, CHANGELOG [Unreleased]. E2E (tests/test_stories_e2e.py — real run/resume/resolve binaries through real tmux with a scripted fake claude): honest coverage map — - (1) two-story happy path, (2) spec_checkpoint two-leg plan-halt + resume, (4) blocked → resolve → re-dispatch run as full sandbox CLI E2E here. - (3) done_checkpoint and (5) worktree isolation are covered deterministically at the engine level (test_stories_engine.py), honestly documented as such. - (6) sprint-mode regression is added as a real sandbox E2E in a follow-up commit on this branch (Wave-1c item 12); prior to it, only the engine-level preflight additivity test existed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ssert Wave-1b item 10, the parts that target Phase-4-introduced text/code, so they ride the stacked Phase-4 branch alongside the TUI surface + docs they touch. - TUI _do_replan checks liveness BEFORE mutating the spec and honors the reset_spec_status bool (MINOR-D); _sentinel_kind guards a race-window OSError (NOTE-9). - docs: per-epic gates are inert in stories mode — the flat stories.yaml list has no epics (README policy note + FEATURES gate section). - tests: tightened the weak or-assertion in test_stories_e2e (NOTE-8) — assert BOTH paused state and the specific plan-checkpoint stage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tate Wave-1c item 13 (audit: story-checkpoint card was PARTIAL — the verify line was a hardcoded "✓ verification passed (story committed)" string, not read from any real result). The task model persists no per-command verify output, so the card now states only what is actually recorded: a done_checkpoint fires only after the story's verify + review gates passed and it committed (the pass is backed by the commit's existence), plus the follow-up review-cycle count the task carries. Falls back to "no commit recorded for this story" when — defensively — no commit is present, rather than asserting an outcome it cannot back. - app.py: _checkpoint_gate_line(review_cycle) derives the line; the card takes it in place of the constant. Pluralized cycle count. - modals.py: StoryCheckpointModal docstring corrected (was "verify-command outcome" — aspirational; no such data is stored). - tests: pure pluralization test + a modal test that the rendered card reflects the real review_cycle and never contains "verification passed". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wave-1c item 12 (audit MAJOR-2: scenario 6 was absent — only a preflight-function unit test on stub markers claimed it; the Phase-4 commit's "all six scenarios covered" overclaimed). Adds test_e2e_sprint_mode_regression: a full sandbox SPRINT-mode run through the real run binary + real tmux + the scripted fake claude, with the SAME new folder+id-capable bmad-dev-auto skill stub installed as the stories scaffold. It proves a plain sprint run still drives dev → verify → commit and lets the orchestrator advance sprint-status to done — unaffected by the stories wiring (the adapter's BMAD_LOOP_SPEC_FOLDER read-back branch, the per-session env exports). The fake grows a no-spec-folder branch that writes the mtime-scanned result artifact under implementation-artifacts (the orchestrator, not the skill, is the single sprint-status writer). Module docstring corrected to the honest coverage map: (1)/(2)/(4)/(6) are full sandbox CLI E2E here; (3) done_checkpoint and (5) worktree isolation stay engine-level in test_stories_engine.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
07bf711 to
7c1e56b
Compare
Phase 4 of the spec-to-loop / stories-mode work: the TUI human-in-the-loop surface, the prose docs, and the full sandbox E2E matrix. Stacked on #77 — base is
feat/spec-to-loop-stories-phase2, notmain. Review/merge #65 → #77 → this, in order.Why this is split from #77
The original single Phase-4 commit bundled the presentation layer with shared infrastructure the Phase-2/3 code already depends on. That infrastructure —
stories.resolve_spec_folder+ thestory_rows/StoryRow/state_labelstatus projection, and the mode-awarebmad-loop status+run --dry-runrefactor ontostory_rows— was moved down onto #77 (the code that calls it:validate, the resolve context, the engine folder render). This branch keeps only what is genuinely the read/observe surface plus the E2E tests, so each PR reviews as one honest scope.TUI (every action calls the same CLI code paths — no duplicated logic)
popens the stage-appropriate viewer: plan-checkpoint spec review (Approve & resume / Request replan), story-checkpoint summary card (Continue / Stop), escalation with story context + blocking condition + sentinel indicator (Resolve / Re-arm & resume), and a gate spec viewer the spec-approval/epic pauses reuse.[stories]) + spec-folder input with a live schedule preview._do_replanchecks liveness before mutating the spec and honors thereset_spec_statusbool (MINOR-D);_sentinel_kindguards a race-windowOSError(NOTE-9).Docs
README dual-flow section + command/keybinding updates,
docs/FEATURES.md,docs/tui-guide.mdHITL pages, CHANGELOG[Unreleased], and the "per-epic gates are inert in stories mode" note (README + FEATURES).E2E matrix — honest coverage map
tests/test_stories_e2e.pydrives the realrun/resume/resolvebinaries through real tmux with a scripted fakeclaude(no LLM, deterministic):spec_checkpointtwo-leg plan-halt + resume, (4) blocked → resolve → re-dispatch, (6) sprint-mode regression — the new folder+id-capable dev skill installed, yet a plain sprint run drives dev → verify → commit → sprint-status advance untouched by the stories wiring.test_stories_engine.py): (3)done_checkpoint, (5) worktree isolation.This corrects the original Phase-4 commit's "all six scenarios covered" overclaim: scenario 6 previously existed only as a preflight-function unit test on stub markers (audit MAJOR-2); it is now a real sandbox run.
Session-4 fixes carried here
Tests
TUI key-press / modal tests (stories board, the
pviewers, start-run preview),test_tui_data/test_tui_launch, the E2E matrix above, and the new story-checkpoint-card unit + modal tests. Full suite green;trunk check(no filter) clean. (The 2 localtest_module_skills_sync[bmad-loop-setup]failures are pre-existing gitignored-workspace drift and skip on CI.)🤖 Generated with Claude Code