refactor(tui): prompt deep-module PR 2 — awaited lifecycle + state reducer - #219
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe prompt session now uses immutable reducer state and a session-scoped async lifecycle. Modal visibility, mode changes, delegates, document suspension, task cancellation, and resource cleanup are centralized and covered by regression tests. ChangesInteractive prompt lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Add prompting/lifecycle.py (PromptLifecycle: owned tasks, reverse-order async closers, idempotent awaited aclose) and async context management on CustomPromptSession; Shell.run now shuts the session down via async with. Replace the private prompt_toolkit exception-filter hack with the public set_exception_handler=False option.
Add prompting/state.py with a pure transition(state, event) reducer over PromptState/PromptTransition (mode, phase, running delegate, modal stack, suspended document, shortcut help). CustomPromptSession routes turn, delegate, modal, mode, shortcut, and buffer mutations through one _dispatch that applies effects and invalidates once; the distributed flags become reducer-owned mirrors. PromptMode and RunningPromptDelegate move to the new module and are re-exported for compatibility.
6d842fb to
b8d8ec1
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
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 `@src/pythinker_code/ui/shell/prompting/lifecycle.py`:
- Around line 59-69: Update the CancelledError handler in aclose so cancellation
escaping a closer is logged and re-raised instead of continuing to the next
resource. Preserve the existing warning handling for ordinary exceptions and
allow individual closers to manage their own internal cancellations.
In `@src/pythinker_code/ui/shell/prompting/state.py`:
- Around line 225-273: Extract the duplicated suspend/restore transition logic
from the ModalAttached and ModalDetached branches into a shared helper, such as
_suspend_restore_effects, accepting the old/new active modal states, suspended
document, and current document and returning the updated suspended document plus
effects. Replace both inline computations with calls to the helper, preserving
each branch’s existing stack construction and state updates.
In `@tests/ui_and_conv/test_prompt_state.py`:
- Around line 372-398: Refactor
test_session_dispatch_keeps_reducer_modal_snapshot_authoritative to construct
CustomPromptSession through its public setup path, using attach_modal() and
related session APIs instead of object.__new__ and direct assignments to private
attributes. Preserve the test’s assertions about the modal snapshot, suspended
document, visibility_queries, and invalidations, and configure the fixture so
attach_modal() produces the same modal_stack.
🪄 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: dd5495d1-f4a0-40d4-9aee-c91eff557128
📒 Files selected for processing (10)
CHANGELOG.mdsrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/prompting/lifecycle.pysrc/pythinker_code/ui/shell/prompting/state.pytests/ui_and_conv/test_prompt_lifecycle.pytests/ui_and_conv/test_prompt_state.pytests/ui_and_conv/test_prompt_tips.pytests/ui_and_conv/test_shell_run_placeholders.pytests/ui_and_conv/test_statusline.py
💤 Files with no reviewable changes (1)
- tests/ui_and_conv/test_prompt_tips.py
Address CodeRabbit review on #219: - PromptLifecycle.aclose no longer swallows CancelledError of aclose() itself: a wait_for-timed-out shutdown now propagates cancellation instead of continuing to close remaining resources past the deadline. Individual closers still own their internal cancellation. - Extract the duplicated modal suspend/restore transition shared by ModalAttached/ModalDetached into _suspend_restore_effects (behavior preserving).
PR 2 of the TUI prompt deep-module refactor plan, covering Tasks 4–5. Stacked on #218 (base is the PR 1 branch); review/merge after #218.
What
bebee17d): newprompting/lifecycle.py(PromptLifecycle: owned tasks, reverse-order async closers, idempotent awaitedaclose).CustomPromptSessiongains async context management (__aenter__/__aexit__/aclose);Shell.runshuts the session down viaasync with. Replaces the private prompt_toolkit exception-filter hack with the publicset_exception_handler=Falseoption.6d842fb2): newprompting/state.pywith a puretransition(state, event) -> PromptTransitionoverPromptState(mode, phase, running delegate, modal stack, suspended document, shortcut help).CustomPromptSessionroutes turn/delegate/modal/mode/shortcut/buffer mutations through one_dispatchthat applies effects and invalidates once; the distributed flags become reducer-owned mirrors.PromptModeandRunningPromptDelegatemove to the new module and are re-exported for compatibility.Verification
make check-pythinker-codeandmake test-pythinker-codegreen (8265 + 65 passed).test_visualize_running_prompt,test_prompt_tips,test_prompt_public_contract) pass unmodified.test_prompt_lifecycle.py(cancellation/reaping) andtest_prompt_state.py(full transition table incl. idempotent stale events).Changelog
Unreleased bullet added covering height budgets + awaited lifecycle.
Summary by CodeRabbit