feat(model): add awaiting-operator phase, sprint token, and surfaces (inert) - #347
Conversation
Names, at every layer, the state a story reaches when its agent-doable work is finished and committed but its acceptance criteria include external actions only a human can perform. Nothing writes any of it yet: the park path is part 2 of the program, so no run can currently reach the phase. - Phase.AWAITING_OPERATOR, terminal, reachable only from COMMITTING so a park can never skip the gates and the commit a DONE story clears - sprint token ordered immediately before `done` — confirming a parked story is then a forward advance through the sole writer, and nothing can regress `done` back into it — and deliberately not actionable - StoryTask.operator_actions, round-tripped through state.json - run-summary count, `status` text + --json, TUI glyphs and header count - drops the dead STORY_STATUSES set (no readers since STATUS_ORDER landed) refs #335 (1/4)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughChangesAwaiting-operator lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CommitFlow
participant StoryTask
participant Engine
participant CLI
participant TUI
CommitFlow->>StoryTask: transition from COMMITTING
StoryTask-->>Engine: expose AWAITING_OPERATOR phase
Engine->>CLI: render parked-task count and phase
Engine->>TUI: render awaiting count
TUI-->>TUI: show ⏸ glyph with yellow styling
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@CHANGELOG.md`:
- Around line 12-25: In the CHANGELOG entry describing the sprint-status token,
replace the article “a” before “awaiting-operator” with “an”; leave the
surrounding wording unchanged.
In `@tests/test_engine.py`:
- Around line 1196-1201: Update the StoryTask fixtures in both affected tests to
use valid AWAITING_OPERATOR states: populate representative outstanding
operator_actions for each parked task, and include a commit SHA wherever the
fixture represents committed work. Keep DONE fixtures unchanged and ensure the
tests no longer rely on the default empty action list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 387401d0-baf7-4cb9-9e8e-494dcba6522d
📒 Files selected for processing (15)
CHANGELOG.mdsrc/bmad_loop/cli.pysrc/bmad_loop/engine.pysrc/bmad_loop/model.pysrc/bmad_loop/sprintstatus.pysrc/bmad_loop/statemachine.pysrc/bmad_loop/tui/widgets.pytests/test_cli.pytests/test_engine.pytests/test_model.pytests/test_sprintstatus.pytests/test_sprintstatus_advance.pytests/test_statemachine.pytests/test_tui_app.pytests/test_verify.py
Greptile SummaryThis PR introduces the
Confidence Score: 5/5Safe to merge — vocabulary-only change, no writer lands, the engine cannot reach the new phase after this PR. Every layer is wired consistently: the phase enum, terminal-phases frozenset, transition table dead-end, sprint-status order, engine count, CLI column width, and TUI glyphs all agree. The cross-module invariant test ensures neither TERMINAL_PHASES nor the transition table can drift silently in future PRs. Backwards-compatibility is handled: state.json round-trips default empty operator_actions for pre-upgrade states, and the --json schema version stays at 1. No logic bugs or unsafe defaults found. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/bmad_loop/model.py | Adds Phase.AWAITING_OPERATOR enum member, expands TERMINAL_PHASES frozenset, and adds operator_actions list field to StoryTask with correct default_factory and safe from_dict deserialization. |
| src/bmad_loop/statemachine.py | Adds COMMITTING → AWAITING_OPERATOR edge and AWAITING_OPERATOR dead-end entry; the new cross-module test pins TERMINAL_PHASES to the table's actual dead-ends. |
| src/bmad_loop/sprintstatus.py | Inserts awaiting-operator into STATUS_ORDER immediately before done; removes the now-dead STORY_STATUSES set (confirmed unused); awaiting-operator is correctly absent from ACTIONABLE_STATUSES. |
| src/bmad_loop/engine.py | Adds RunSummary.awaiting_operator count (with correct default=0 rationale), extends the summary() constructor, and appends the render clause only when non-zero. |
| src/bmad_loop/cli.py | Widens the phase column from :16s to :17s; awaiting-operator is 17 chars and without the fix its row overflows, shifting all subsequent columns on that row. |
| src/bmad_loop/tui/widgets.py | Adds ⏸/yellow glyph+style entries to SPRINT_GLYPHS/SPRINT_STYLES/STORY_GLYPHS/STORY_STYLES, and adds the conditional awaiting count to RunHeader; a new coverage test pins STATUS_ORDER ⊆ SPRINT_GLYPHS. |
| tests/test_statemachine.py | Adds terminal-phases invariant test, park path sequence, and exhaustive reachability-only-from-COMMITTING parametrized test; all three are ablation-validated. |
| tests/test_verify.py | Corrects an existing parametrized case (replaces awaiting-operator with a genuinely unknown token) and adds a new test asserting the now-ordered token is classified as a sign-off regression. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PENDING --> DEV_RUNNING
PENDING --> TRIAGE_RUNNING
DEV_RUNNING --> DEV_VERIFY
DEV_VERIFY --> DEV_RUNNING
DEV_VERIFY --> REVIEW_RUNNING
DEV_VERIFY --> COMMITTING
DEV_VERIFY --> DEFERRED
DEV_VERIFY --> ESCALATED
REVIEW_RUNNING --> REVIEW_VERIFY
REVIEW_VERIFY --> REVIEW_RUNNING
REVIEW_VERIFY --> DEV_RUNNING
REVIEW_VERIFY --> COMMITTING
REVIEW_VERIFY --> DEFERRED
REVIEW_VERIFY --> ESCALATED
COMMITTING --> DONE
COMMITTING --> ESCALATED
COMMITTING -->|new: operator_actions present| AWAITING_OPERATOR
TRIAGE_RUNNING --> TRIAGE_VERIFY
TRIAGE_VERIFY --> TRIAGE_RUNNING
TRIAGE_VERIFY --> DONE
TRIAGE_VERIFY --> ESCALATED
DONE([DONE]):::terminal
DEFERRED([DEFERRED]):::terminal
ESCALATED([ESCALATED]):::terminal
AWAITING_OPERATOR([AWAITING_OPERATOR]):::newTerminal
classDef terminal fill:#ccc,stroke:#999
classDef newTerminal fill:#ffcc00,stroke:#cc9900,color:#000
Reviews (2): Last reviewed commit: "docs(model): scope the operator_actions ..." | Re-trigger Greptile
Review feedback on #347. The field comment stated "a park is defined by owing at least one action" as a live invariant, but this PR ships no writer and no validator, so nothing can hold it — which is exactly why a reviewer read the empty-list fixtures as invalid state. Attribute the rule to the park path that will enforce it and say plainly that the field is unvalidated on both load and write. Also: fix the article before `awaiting-operator` in the CHANGELOG, and pin that the v1 status document deliberately does not project the actions themselves (the fixture set them but asserted nothing).
|
Review response pushed in 67f98a3 (no force-push — the review pass is still diffable against 18f9643). Both inline threads answered and resolved: the CHANGELOG article was valid and is fixed; the fixture ask is declined with reasons, but the mismatch it detected was real and is fixed at its root. Answering the two failed pre-merge checks, since neither is actionable here: Linked Issues. Working as intended. This is a deliberate 1-of-4 slice of #335 — the body opens with Docstring Coverage (50% vs 80%). No such gate exists in this repo, and the threshold is the bot's, not ours. The house style in |
1 participant
refs #335 (1/4)
Part 1 of the four-PR
awaiting-operatorprogram: vocabulary only. It names the state at every layer and wires the display surfaces, but ships no writer — nothing in the engine can reach the phase after this PR. The park path is part 2,bmad-loop confirmis part 3, review-demotion composition is part 4.Does not close #335.
What lands
model.pyPhase.AWAITING_OPERATOR+TERMINAL_PHASES;StoryTask.operator_actions: list[str]withto_dict/from_dictstatemachine.pyCOMMITTING → AWAITING_OPERATOR(plus the phase's own empty outgoing set)sprintstatus.pyawaiting-operatorinSTATUS_ORDER, immediately beforedone; not inACTIONABLE_STATUSES; drops the deadSTORY_STATUSESsetengine.pyRunSummary.awaiting_operator+ thesummary()count + a render clausecli.pystatusphase column widened 16 → 17 (awaiting-operatoris the longest phase token)tui/widgets.pySPRINT_*/STORY_*glyph+style entries (⏸, yellow) and the run-header countDesign notes
Terminal, not paused. A story owing external human actions must not block the stories behind it, so it is terminal like
DONErather than run-halting like the spec'sBlock If:→blocked→ CRITICAL → pause channel. Terminal says nothing about success:DONEandAWAITING_OPERATORcarry a commit,DEFERREDandESCALATEDdo not.Reachable only from
COMMITTING. Parking rides the normal commit path — the work commits, then the final phase is chosen by whether the task carriesoperator_actions. MakingCOMMITTINGthe sole inbound edge is what stops a future park from skipping the gates and the commit aDONEstory clears.Token position is the whole contract. Sitting immediately below
donemeans part 3'sconfirmis an ordinary forwardadvance()— no exception to the sole-writer invariant — whiledone → awaiting-operatoris refused by the existing never-regress guard.documents.pyneeded no edit, despite being on the plan's touch list: the--jsonprojection is already phase-agnostic (str(task.phase); there are no aggregate per-phase counts in the document). Schema stays at 1. A test pins that claim rather than leaving it as an assertion in a PR body.Two consequences worth reviewing
state.jsonis forward-only. A run that records the new phase is rejected by an older binary. Standard for phase additions; noted in the CHANGELOG.One real behavior change, and it is deliberate. The sprint token is now ordered rather than unknown, so
verify._is_signoff_regression(Escalate deliberate sprint-status regressions at review-verify instead of burning review cycles #334) classifies a review writingawaiting-operatoronto a board the orchestrator had already advanced todoneas a deliberate sign-off regression → escalate, where it previously fell through to a retry. Until the park path exists nothing legitimately writes that token, so a review that writes it has revoked a sign-off — and this is exactly part 4's defaultoperator.on_review_demotion = escalate, arriving early. This flipped an existing parametrized case intest_verify.pythat usedawaiting-operatoras its example of an out-of-STATUS_ORDERtoken; that case now uses a genuinely unknown token, and the new semantics gets its own test.Tests
Statemachine (park path, reachable-only-from-
COMMITTING, and a newTERMINAL_PHASES ==dead-ends guard — the two spellings of terminality live in different modules with nothing previously linking them); model round-trip incl. pre-upgradestate.jsondefaults and no shared mutable default; sprint token ordering, confirm-forward and never-regress-into; parked story never picked / never targetable; summary counts and render;statustext +--json; TUI glyph coverage and header count.Ablations run (repo rule) — each confirmed to make the named test fail, then reverted:
ACTIONABLE_STATUSES(inverse form — the gate is an absence)STATUS_ORDERadvancenever-regresses-done-into-parkedDEV_VERIFYCOMMITTINGTERMINAL_PHASES:16sstatuscolumn alignmentAblation 6 caught a real defect in my own test first: the original assertion used
_story_row, which splits on whitespace and is therefore blind to column shift, so it passed under ablation. Rewritten to compare two rows'dev×offsets, which does bite.uv run pytest -q -n auto→ 3467 passed, 24 skipped. Fulltrunk checkclean;pyright@1.1.4110 errors.Summary by CodeRabbit
New Features
awaiting-operatorworkflow phase for tasks that need human action, with persisted operator instructions.awaiting-operatorcounts and the ⏸ indicator.awaiting-operatoris treated as a terminal “parked” state.Bug Fixes
doneback toawaiting-operator, and ensured parked tasks aren’t selectable for new work.Documentation
awaiting-operatorregression behavior.