feat(model): add awaiting-operator phase, sprint token, and surfaces (inert) - #347
Open
pbean wants to merge 1 commit into
Open
feat(model): add awaiting-operator phase, sprint token, and surfaces (inert)#347pbean wants to merge 1 commit into
pbean wants to merge 1 commit into
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)
WalkthroughChangesAwaiting-operator lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 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
Comment on lines
+12
to
+25
| - **`awaiting-operator` vocabulary, no writer yet (#335, part 1 of 4).** 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: `Phase.AWAITING_OPERATOR` (terminal, | ||
| reachable only from `COMMITTING`), a `awaiting-operator` sprint-status token ordered immediately | ||
| before `done` and deliberately not actionable, `operator_actions` on each task, and the matching | ||
| run-summary count, `status` output and TUI glyphs. Nothing writes any of it yet — the park path | ||
| is part 2 — so no run can currently reach the phase. | ||
|
|
||
| Two consequences worth knowing. A `state.json` carrying the new phase is forward-only: an older | ||
| binary rejects it, as with every phase addition. And because the sprint token is now _ordered_ | ||
| rather than unknown, a review session writing `awaiting-operator` onto a board the orchestrator | ||
| had already advanced to `done` is classified as a deliberate sign-off regression and escalates | ||
| (#334), where it previously fell through to a retry. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the article before awaiting-operator.
Line 15 reads “a awaiting-operator sprint-status token”; use “an” instead.
Proposed wording
- a `awaiting-operator` sprint-status token
+ an `awaiting-operator` sprint-status token📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **`awaiting-operator` vocabulary, no writer yet (#335, part 1 of 4).** 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: `Phase.AWAITING_OPERATOR` (terminal, | |
| reachable only from `COMMITTING`), a `awaiting-operator` sprint-status token ordered immediately | |
| before `done` and deliberately not actionable, `operator_actions` on each task, and the matching | |
| run-summary count, `status` output and TUI glyphs. Nothing writes any of it yet — the park path | |
| is part 2 — so no run can currently reach the phase. | |
| Two consequences worth knowing. A `state.json` carrying the new phase is forward-only: an older | |
| binary rejects it, as with every phase addition. And because the sprint token is now _ordered_ | |
| rather than unknown, a review session writing `awaiting-operator` onto a board the orchestrator | |
| had already advanced to `done` is classified as a deliberate sign-off regression and escalates | |
| (#334), where it previously fell through to a retry. | |
| - **`awaiting-operator` vocabulary, no writer yet (`#335`, part 1 of 4).** 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: `Phase.AWAITING_OPERATOR` (terminal, | |
| reachable only from `COMMITTING`), a `awaiting-operator` sprint-status token ordered immediately | |
| before `done` and deliberately not actionable, `operator_actions` on each task, and the matching | |
| run-summary count, `status` output and TUI glyphs. Nothing writes any of it yet — the park path | |
| is part 2 — so no run can currently reach the phase. | |
| Two consequences worth knowing. A `state.json` carrying the new phase is forward-only: an older | |
| binary rejects it, as with every phase addition. And because the sprint token is now _ordered_ | |
| rather than unknown, a review session writing `awaiting-operator` onto a board the orchestrator | |
| had already advanced to `done` is classified as a deliberate sign-off regression and escalates | |
| (`#334`), where it previously fell through to a retry. |
🤖 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 `@CHANGELOG.md` around lines 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.
Comment on lines
+1196
to
+1201
| for key, phase in ( | ||
| ("1-1-a", Phase.DONE), | ||
| ("1-2-b", Phase.AWAITING_OPERATOR), | ||
| ("1-3-c", Phase.AWAITING_OPERATOR), | ||
| ): | ||
| engine.state.tasks[key] = StoryTask(story_key=key, epic=1, phase=phase) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use valid AWAITING_OPERATOR fixtures.
Both tests rely on the default empty operator_actions list, even though the model contract defines a parked task by its outstanding human actions. Populate representative actions (and a commit SHA where the fixture models committed work) so these tests cannot pass with an invalid parked state.
Example adjustment
- ("1-2-b", Phase.AWAITING_OPERATOR),
- ("1-3-c", Phase.AWAITING_OPERATOR),
+ ("1-2-b", Phase.AWAITING_OPERATOR, ["publish the required record"]),
+ ("1-3-c", Phase.AWAITING_OPERATOR, ["complete the external setup"]),Also applies to: 1217-1219
🤖 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 `@tests/test_engine.py` around lines 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.
Greptile SummaryThis PR introduces the
Confidence Score: 5/5Vocabulary-only change; no writer exists yet so no run can reach the new phase in this release. Every changed layer is consistent and the cross-module invariants (TERMINAL_PHASES vs. transition table dead-ends, STATUS_ORDER position, column width) are each pinned by a dedicated test. The one real behavior change is intentional, well-documented, and has its own targeted test. The operator_actions field uses default_factory, deserializes defensively, and round-trips correctly from pre-upgrade state. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/bmad_loop/model.py | Adds Phase.AWAITING_OPERATOR to the Phase StrEnum, updates TERMINAL_PHASES to include it, and adds operator_actions list to StoryTask with correct default_factory, serialization, and backward-compatible deserialization. |
| src/bmad_loop/statemachine.py | Adds Phase.AWAITING_OPERATOR as an outgoing option from COMMITTING and registers it in TRANSITIONS with an empty outgoing set (terminal). The sole-inbound-edge design is enforced by the new parametrized test. |
| src/bmad_loop/sprintstatus.py | Inserts awaiting-operator immediately before done in STATUS_ORDER, deliberately excludes it from ACTIONABLE_STATUSES, and removes the now-dead STORY_STATUSES set. |
| src/bmad_loop/engine.py | Adds awaiting_operator: int = 0 to RunSummary (correctly defaulted) and renders it conditionally in render() between escalated and tokens. |
| src/bmad_loop/cli.py | Widens the phase column format from :16s to :17s to accommodate the new longest token awaiting-operator (17 chars). |
| src/bmad_loop/tui/widgets.py | Adds awaiting-operator glyph and style to both SPRINT and STORY maps; adds a conditional awaiting N count to RunHeader shown only when non-zero. |
| tests/test_statemachine.py | Adds dead-end/TERMINAL_PHASES cross-module consistency guard, full park-path sequence, and parametrized single-inbound-edge invariant. |
| tests/test_verify.py | Updates the unrecognized-token retry case to use a genuinely-unknown token and adds a test verifying awaiting-operator now triggers the sign-off regression path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PENDING --> DEV_RUNNING
DEV_RUNNING --> DEV_VERIFY
DEV_VERIFY --> COMMITTING
DEV_VERIFY --> DEFERRED
DEV_VERIFY --> ESCALATED
REVIEW_RUNNING --> REVIEW_VERIFY
REVIEW_VERIFY --> COMMITTING
REVIEW_VERIFY --> DEFERRED
REVIEW_VERIFY --> ESCALATED
COMMITTING --> DONE
COMMITTING --> ESCALATED
COMMITTING -->|new edge| AWAITING_OPERATOR
DONE:::terminal
DEFERRED:::terminal
ESCALATED:::terminal
AWAITING_OPERATOR:::newTerminal
classDef terminal fill:#6b7280,color:#fff,stroke:none
classDef newTerminal fill:#ca8a04,color:#fff,stroke:none
Reviews (1): Last reviewed commit: "feat(model): add awaiting-operator phase..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
awaiting-operatorworkflow phase for tasks requiring manual action.