fix(tui): scrollable modal dialogs so action buttons stay reachable (#275)

[pbean]

Closes #275.

Problem

The TUI "Decision Panel" (DecisionModal) — and several sibling run-control modals — rendered their body content and their action buttons as direct children of a non-scrolling Vertical(id="dialog") capped at max-height: 90%. When the LLM-written decision/escalation text or option list was long, the dialog hit its cap and the plain Vertical clipped the overflow with no scrollbar and no scroll key, so the lowest choose/skip (or Resolve/Re-arm/close) button became unreachable. This is a pure Textual layout bug — OS-independent (macOS in #275 was just where it surfaced); it reproduces on any terminal short enough that content exceeds 90% of the height.

Fix — the repo's own docked-buttons pattern, in two tiers

The codebase already endorses the correct pattern (modals.py:108, used by StartRunModal/DeferredEntryModal/SpecReviewModal/ValidateFindingsModal/TextOutputModal): a VerticalScroll body region + a docked Horizontal(classes="buttons") row that never scrolls off.

• Tier 1 — action modals with frequently-long content (DecisionModal, EscalationModal). Switch #dialog to a definite height: 90% and wrap all growable content in VerticalScroll(id="body") { height: 1fr }; the button row stays docked outside. (A 1fr child needs a definite parent height — hence the fixed #dialog height.)
• Tier 2 — bounded modals (ConfirmModal → also ConfirmResumeModal, StartSweepModal, StoryCheckpointModal). Keep #dialog { height: auto; max-height: 90% } so small dialogs still hug their content, and wrap the variable body in VerticalScroll(id="body") { height: auto; max-height: N% } (grows to content, scrolls past the cap — the same mechanism the old #context/#blocking boxes used). No visual change in the common short case; the docked buttons can no longer be pushed off-bottom by an unexpectedly long body (e.g. an engine-written paused_reason).

All widget ids are preserved verbatim (tests click by id), and all user/LLM strings stay wrapped in rich.Text. The five already-correct modals and BaseDialog's shared CSS are untouched.

Tests

Adds four regression tests to tests/test_tui_app.py that push each modal directly with long content and assert (a) the #body scrolls (max_scroll_y > 0) and (b) the docked action buttons are fully on-screen (screen.region.contains_region(...)). The sweep/checkpoint test runs at height 14 — just above the modal frame floor (a thick-bordered dialog with a title and a 3-row button row needs ~13 rows before any body content), so the body-scroll fix is what keeps the buttons reachable.

Verification

• uv run pytest -q tests/test_tui_app.py — 135 passed (new + existing modal tests, including the #opt-1/#act-resolve/#act-rearm click paths).
• uv run pytest -q — full suite green (the 2 test_module_skills_sync failures are pre-existing local-install drift; those tests skip on a clean checkout/CI and are unrelated to this change).
• uvx pyright@1.1.411 clean; trunk fmt + trunk check clean.
• Manual: on a short terminal, press d with a long pending decision (or open an escalation) — the body scrolls (wheel/arrows/Tab) and every button stays reachable.

Summary by CodeRabbit

• Bug Fixes
  • Improved modal dialogs on small terminal screens so long content scrolls while action buttons remain visible and reachable.
  • Kept important safety warnings visible while scrolling, including Resume and Re-arm confirmations.
  • Resume confirmations now re-check engine status before proceeding, preventing unsafe resume attempts.
• Documentation
  • Added changelog entries describing the modal scrolling and resume-safety improvements.

[coderabbitai]

Warning

Review limit reached

@pbean, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: adb0d834-18d3-46d9-8921-6e49e4929ebc

📥 Commits

Reviewing files that changed from the base of the PR and between cba7a9f and 851c636.

📒 Files selected for processing (2)

• src/bmad_loop/tui/screens/modals.py
• tests/test_tui_app.py

Walkthrough

Multiple TUI modals now scroll long content while keeping action buttons and safety warnings visible. Resume confirmation routes through shared logic that re-checks engine liveness before launching.

Changes

Scrollable modal dialogs

Layer / File(s)	Summary
Scrollable modal layout src/bmad_loop/tui/screens/modals.py	Start sweep, confirmation, decision, checkpoint, and escalation dialogs now use scrollable body regions while keeping controls and relevant warnings docked.
Modal reachability regression coverage tests/test_tui_app.py, CHANGELOG.md	Regression tests cover constrained-screen scrolling, reachable actions, docked escalation warnings, and the corresponding changelog entry.

Resume confirmation guard

Layer / File(s)	Summary
Confirm-time resume validation src/bmad_loop/tui/app.py	Confirmed resume actions now use the shared _do_resume path for liveness checks and error handling.
Resume liveness regression coverage tests/test_tui_app.py	A regression test verifies that a possibly-live engine blocks resume and emits a notification.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: dracic

Poem

A bunny hops through panels wide,
While buttons stay close by its side.
Long words gently scroll and sway,
Warnings remain in view all day.
Resume checks before it goes—
Safe little paws, no risky toes!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: making TUI modal dialogs scrollable so action buttons remain reachable.
Linked Issues check	✅ Passed	The changes address issue #275 by making the decision panel content scrollable and keeping the lowest choose button reachable.
Out of Scope Changes check	✅ Passed	The additional modal and resume-flow updates are aligned with the stated UI accessibility fix and related follow-up coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/modal-scroll-275

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pbean]

Comprehensive review — changes requested

The scrolling implementation works, and the PR checks are green, but two enabled actions can be reached while their safety warnings remain hidden below long content. I recommend addressing these before merge.

Findings

1. High — Resume can bypass the double-drive warning

   ConfirmModal.compose() places the warning at the bottom of #body while keeping Resume docked and enabled (src/bmad_loop/tui/screens/modals.py:278-284). With a long pause reason at 64×12, the warning is around row 86 while Resume is visible at rows 9–11. The callback in src/bmad_loop/tui/app.py:364-376 launches without rechecking engine liveness, so a user can resume without ever seeing that it may double-drive the run.

2. High — Re-arm can bypass the restore-discard warning

   EscalationModal.compose() places the restore-patch warning after arbitrary scrollable content (src/bmad_loop/tui/screens/modals.py:596-639) while Re-arm remains docked and enabled (:640-650). With a long description at 90×16, the warning is around row 91 while Re-arm is visible at rows 10–12. The callback proceeds with _do_rearm (src/bmad_loop/tui/app.py:645-652), and the discarded restore is reported only after mutation (:719-738).

3. Medium — Tests prove overflow, not actual scroll access

   The new tests check max_scroll_y, docked-button visibility, and DOM presence (tests/test_tui_app.py:1266-1348). Those predicates can still pass with a non-scrollable container. The decision test never scrolls to or activates #opt-8, and the checkpoint fixture does not use long content. The current implementation does scroll and activate the eighth option correctly; this is a regression-protection gap rather than a current functional failure.

4. Medium, follow-up — The “any terminal size” claim exceeds the height boundary

   At 64×12, Start Sweep and Story Checkpoint buttons still extend beyond the screen; the layout needs roughly 13 rows of chrome. The test deliberately uses height 14, while CHANGELOG.md:70-73 says “any terminal size.” This floor largely predates the PR and needs either a documented minimum or compact-layout work.

5. Medium, follow-up — Fixed widths still clip actions on narrow terminals

   The 64-, 86-, and 90-column dialog widths remain non-responsive. On narrower terminals, parts of the Decision, Escalation, and Checkpoint action rows fall outside the screen. This is pre-existing and outside the reported vertical-overflow defect, but it contradicts the new universal wording.

Remediation plan

1. Keep critical warnings outside the scrolling body, immediately above the docked action row.
2. Add long-content tests asserting those warnings are fully on-screen whenever Resume or Re-arm is enabled.
3. Exercise real scrolling: scroll or keyboard-focus to the final item, assert it is visible, and activate it. Use genuinely long checkpoint content.
4. Either soften “any terminal size” to a documented supported minimum or follow up with responsive widths and compact small-height chrome.
5. Re-run tests/test_tui_app.py, the full suite, Pyright, and Trunk checks.

Verification performed

• Local uv run pytest -q tests/test_tui_app.py: 135 passed
• All current PR test, lint, type-check, and version-sync jobs pass
• Functional probe confirmed that scrolling/focus reaches and activates the eighth decision option

Review triage: 3 patch items, 2 pre-existing/follow-up items.

[pbean]

Review addressed — pushed cba7a9f

Validated all five findings against the code; all were accurate. Fixes for the three patch items + the CHANGELOG wording are in; the two pre-existing/follow-up items are tracked in #281.

#	Finding	Resolution
1	High — Resume double-drive warning can scroll out of view	The ⚠ warning is now docked outside VerticalScroll(#body), directly above the button row (ConfirmModal.compose), so it's visible whenever the confirm button is. Plus: action_resume_run's confirm callback now routes through _do_resume, which re-checks _engine_possibly_live at click time (matching the e/viewer and re-arm paths) — a newly-live engine is refused even if the user confirmed. Removes the duplicated inline launch.resume_detached.
2	High — Re-arm restore-discard warning can scroll out of view	The restore-discard hint is now docked outside #body (EscalationModal.compose, id="hint"), so it stays on-screen while the enabled #act-rearm is reachable. (Re-arm already re-checked liveness via _resolve_blocked_by_liveness; the residual gap was purely warning visibility.)
3	Medium — Tests prove overflow, not scroll-access	test_decision_modal_scrolls_when_content_long now scrolls #opt-8 into view, asserts it's on-screen, clicks it, and asserts the eighth option is returned. New test_confirm_warning_stays_on_screen / test_escalation_rearm_warning_stays_on_screen assert the warnings are on-screen while the destructive button is enabled. New test_resume_confirm_rechecks_liveness asserts resume is refused (no resume_detached) when the engine may be live. The checkpoint case now uses genuinely long content and asserts the body scrolls.
4	Medium (follow-up) — "any terminal size" over-claims	CHANGELOG reworded to "down to the dialog's minimum frame height." The compact small-height-layout work is tracked in #281.
5	Medium (follow-up) — Fixed widths clip on narrow terminals	Pre-existing; out of scope for the vertical-overflow fix. Tracked in #281 (responsive dialog widths).

Verification: uv run pytest -q tests/test_tui_app.py green (incl. the new/strengthened tests); full suite 2907 passed (the 2 test_module_skills_sync failures are pre-existing local-install drift — they skip on a clean checkout/CI); uvx pyright@1.1.411 clean; trunk fmt + trunk check clean.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/bmad_loop/tui/screens/modals.py`:
- Around line 213-232: Update the BaseDialog `#dialog` styling to use a definite
height, such as 90%, instead of height: auto, so the percentage max-height on
StartSweepModal and related modal bodies has a stable containing height.
Preserve the existing modal layout and scrolling behavior.

🪄 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: e5d5601a-1a95-4000-8ccc-681b0096cf2a

📥 Commits

Reviewing files that changed from the base of the PR and between 2a614a8 and cba7a9f.

📒 Files selected for processing (4)

• CHANGELOG.md
• src/bmad_loop/tui/app.py
• src/bmad_loop/tui/screens/modals.py
• tests/test_tui_app.py