refactor(tui): prompt deep-module PR 1 — frame snapshot + height budgets#218
Conversation
Add prompting/renderer.py with PromptSceneBudget and a priority-ordered row allocator (modal > input > footer > pinned > body > status > shortcuts). Rendering is unchanged for scenes that fit; on overflow the scene is clipped tail-first per priority, pinned allocation is zero-safe, and the interactive preview no longer derives a competing body budget.
Resize the live PTY through heights 12/8/6/4 during a running turn and assert the redrawn frame stays within the new geometry, never fossilizes the input card, and the turn still completes after the size is restored.
📝 WalkthroughWalkthroughThe prompt renderer now captures delegate output once per frame, allocates terminal rows across prompt surfaces, clips preambles and toolbars consistently, and adds coverage for resizing, scene precedence, sampling, and public compatibility. ChangesPrompt rendering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CustomPromptSession
participant PromptFrameCollector
participant allocate_prompt_scene_rows
participant Terminal
CustomPromptSession->>PromptFrameCollector: Capture delegate fragments and UI state
PromptFrameCollector->>allocate_prompt_scene_rows: Request bounded surface allocations
allocate_prompt_scene_rows-->>CustomPromptSession: Return PromptSceneAllocation
CustomPromptSession->>Terminal: Render clipped prompt and toolbar
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@AGENTS.md`:
- Around line 44-48: Update the “Never propose deferring an applicable issue or
fix” guidance in AGENTS.md to align with the surgical-change rule: require
immediate fixes only for release-blocking correctness, security, or
data-integrity issues, and require explicit tracking for other findings instead
of forcing unrelated redesigns or silently ignoring them.
In `@src/pythinker_code/ui/shell/prompt.py`:
- Around line 3149-3160: Update _render_shell_prompt_message to refresh
_prompt_footer_row_budget before or alongside the _fit_preamble_with_pinned_tail
flow, using the current terminal dimensions and shell-mode footer requirements.
Ensure the refreshed budget is applied before _fit_toolbar_to_terminal runs,
including after mode switches and terminal resizes.
In `@tests/e2e/test_shell_pty_prompt_layout_e2e.py`:
- Around line 272-274: Remove the trivially true len(rows) == height assertion
in the resized-frame test around _render_sized. Replace it with an assertion
that validates observable post-resize behavior, such as preservation of the
running-prompt content or absence of a fossil border on the final line, while
retaining the existing _has_fossil_border_above_content check.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a531ca57-7c2b-4377-ba87-f7ab7e3371c1
⛔ Files ignored due to path filters (3)
docs/superpowers/plans/2026-07-19-tui-content-rendering-standardization.mdis excluded by!docs/**docs/superpowers/specs/2026-07-19-tui-content-rendering-standardization-design.mdis excluded by!docs/**tasks/lessons.mdis excluded by!tasks/**
📒 Files selected for processing (12)
AGENTS.mdCHANGELOG.mdsrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/prompting/__init__.pysrc/pythinker_code/ui/shell/prompting/frame.pysrc/pythinker_code/ui/shell/prompting/renderer.pysrc/pythinker_code/ui/shell/visualize/_interactive.pytests/e2e/test_shell_pty_prompt_layout_e2e.pytests/ui_and_conv/test_prompt_height_budget.pytests/ui_and_conv/test_prompt_public_contract.pytests/ui_and_conv/test_prompt_tips.pytests/ui_and_conv/test_visualize_running_prompt.py
…rtion Shell-mode rendering never set _prompt_footer_row_budget, so _fit_toolbar_to_terminal clipped the bottom toolbar against a stale agent-mode value (or the initial 0, hiding the footer entirely in a pure shell session). Refresh it every shell render. Replace a trivially-true pyte row-count assertion in the resize e2e with a width-overflow check.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/ui/shell/prompt.py (1)
2845-2846: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh the update notice for every captured frame.
_append_update_noticetrusts_prompt_frame_update_noticewhenever_current_prompt_frameis set, but the cache is populated only by agent rendering and is never cleared with the frame. Shell mode can therefore suppress a current notice on its first frame or replay stale agent-mode text after a mode switch.Refresh the notice in shell rendering (or capture it in
_capture_prompt_frame) and clear it in_clear_prompt_frame.Suggested fix
def _clear_prompt_frame(_app: Application[str]) -> None: self._current_prompt_frame = None + self._prompt_frame_update_notice = None def _render_shell_prompt_message(self) -> FormattedText: frame = self._prompt_frame_for_render() + provider = self._update_notice_provider + self._prompt_frame_update_notice = provider() if provider is not None else NoneAlso applies to: 3127-3134, 4355-4364
🤖 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 `@src/pythinker_code/ui/shell/prompt.py` around lines 2845 - 2846, Update prompt-frame lifecycle handling so _append_update_notice always uses a notice captured for the current frame: refresh _prompt_frame_update_notice during shell rendering or within _capture_prompt_frame, and reset it alongside _current_prompt_frame in _clear_prompt_frame. Apply the same behavior to the corresponding frame-rendering paths.
🤖 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.
Outside diff comments:
In `@src/pythinker_code/ui/shell/prompt.py`:
- Around line 2845-2846: Update prompt-frame lifecycle handling so
_append_update_notice always uses a notice captured for the current frame:
refresh _prompt_frame_update_notice during shell rendering or within
_capture_prompt_frame, and reset it alongside _current_prompt_frame in
_clear_prompt_frame. Apply the same behavior to the corresponding
frame-rendering paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fca03a6d-c2fc-4558-a14f-4f6ea314c86c
📒 Files selected for processing (2)
src/pythinker_code/ui/shell/prompt.pytests/e2e/test_shell_pty_prompt_layout_e2e.py
The per-frame _prompt_frame_update_notice snapshot was refreshed only by the agent render path and never cleared with the frame, so shell frames suppressed a live update notice (initial None) or replayed a stale agent-mode notice after a mode switch. Refresh the snapshot in the shell render like the agent path and reset it in _clear_prompt_frame.
|
@coderabbitai Addressed the outside-diff finding (
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant symbols in prompt.py =='
rg -n -C 5 \
'def _render_shell_prompt_message\b|def _clear_prompt_frame\b|_prompt_frame_update_notice|def _append_update_notice\b' \
src/pythinker_code/ui/shell/prompt.py
echo
echo '== Outline and test location =='
ast-grep outline tests/ui_and_conv/test_prompt_height_budget.py --items all --type function --match 'test_shell_render_refreshes_update_notice_snapshot|test_update_notice_is_sampled_once_for_prompt_and_footer' || true
rg -n -C 8 \
'def test_shell_render_refreshes_update_notice_snapshot\b|def test_update_notice_is_sampled_once_for_prompt_and_footer\b' \
tests/ui_and_conv/test_prompt_height_budget.py
echo
echo '== Commit summary =='
git show --stat --oneline --no-renames 70786925Length of output: 7476 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Shell update-notice regression test =='
sed -n '148,190p' tests/ui_and_conv/test_prompt_height_budget.py
echo
echo '== Update-notice cache-read branch =='
sed -n '4359,4395p' src/pythinker_code/ui/shell/prompt.py
echo
echo '== Shell renderer refresh context =='
sed -n '3130,3165p' src/pythinker_code/ui/shell/prompt.pyLength of output: 6227
The rendering finding is resolved. |
There was a problem hiding this comment.
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 `@tests/ui_and_conv/test_prompt_height_budget.py`:
- Around line 148-180: The test
test_shell_render_refreshes_update_notice_snapshot should also exercise the
registered after_render cleanup callback. Complete a rendered frame through the
existing session/render lifecycle, then assert that
session._prompt_frame_update_notice is None after the frame finishes, covering
cleanup independently of the provider refresh and None-clearing assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a57efce-f2c8-40f6-972c-97946b3cecf8
📒 Files selected for processing (3)
CHANGELOG.mdsrc/pythinker_code/ui/shell/prompt.pytests/ui_and_conv/test_prompt_height_budget.py
Extract the after_render frame-clear closure into a named _clear_prompt_frame_snapshot method so the lifecycle cleanup is directly testable, and assert a completed frame leaves _prompt_frame_update_notice as None.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ui_and_conv/test_prompt_height_budget.py (1)
109-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the real footer render in the height assertion.
This test fabricates a 3/4-row footer and mocks
get_app_or_none, so it can pass even if_render_bottom_toolbaror the actual prompt-toolkit layout adds rows. Assert the observable rendered layout using the real toolbar/render lifecycle (or the existing PTY path).As per path instructions, tests under
tests/**/*.pyshould avoid mocking internal implementation details in place of observable behavior.🤖 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/ui_and_conv/test_prompt_height_budget.py` around lines 109 - 126, Update the height-budget test around _fit_toolbar_to_terminal to exercise the real footer rendering lifecycle, including _render_bottom_toolbar and prompt-toolkit layout behavior, instead of fabricating footer rows and mocking shell_prompt.get_app_or_none. Assert the observable rendered layout through the existing PTY or equivalent real render path, while preserving the message-plus-footer height constraint.Source: Path instructions
🤖 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.
Outside diff comments:
In `@tests/ui_and_conv/test_prompt_height_budget.py`:
- Around line 109-126: Update the height-budget test around
_fit_toolbar_to_terminal to exercise the real footer rendering lifecycle,
including _render_bottom_toolbar and prompt-toolkit layout behavior, instead of
fabricating footer rows and mocking shell_prompt.get_app_or_none. Assert the
observable rendered layout through the existing PTY or equivalent real render
path, while preserving the message-plus-footer height constraint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbf19eba-b690-450a-9a5c-5c932be45a0a
📒 Files selected for processing (2)
src/pythinker_code/ui/shell/prompt.pytests/ui_and_conv/test_prompt_height_budget.py
1 participant
PR 1 of the TUI prompt deep-module refactor plan (docs/superpowers/plans/2026-07-19-tui-prompt-deep-module-refactor.md), covering Tasks 1–3.
What
4a38abf2): freezes the shell prompt's public import surface and constructor semantics intests/ui_and_conv/test_prompt_public_contract.py(22 exported symbols, keyword construction, 30 FPS render cap), plus characterization of delegate render sampling.fe2d4f42): newui/shell/prompting/package withPromptFrame/PromptFrameCollector; every render samples each running-prompt/modal delegate method exactly once per frame viabefore_render/after_render, eliminating redundant delegate calls. Live input decisions still use live delegate lookup.5bd40e23): newprompting/renderer.pywithPromptSceneBudget(replaces the fixed 7-row subtraction) and a priority-ordered row allocator (modal → input → footer → pinned → separator → body → status → shortcuts). Scenes that fit render byte-identically; clipping engages only on overflow, pinned allocation is zero-safe, and the interactive preview no longer derives a competing body budget.2fd528f2): PTY test resizing a live turn through heights 12/8/6/4, asserting the redrawn frame fits and the turn survives.Verification
make check-pythinker-codeandmake test-pythinker-codegreen at the PR boundary (8238 passed in the pre-push gate).test_prompt_tips,test_visualize_running_prompt,test_modal_lifecycle) pass unmodified.Summary by CodeRabbit