Skip to content

feat: back-port RichUIHandler output improvements to InteractiveUIHandler - #54

Merged
christophevg merged 3 commits into
masterfrom
feature/ui-back-port
Jul 28, 2026
Merged

feat: back-port RichUIHandler output improvements to InteractiveUIHandler#54
christophevg merged 3 commits into
masterfrom
feature/ui-back-port

Conversation

@christophevg-agent

Copy link
Copy Markdown
Collaborator

Summary

Back-port RichUIHandler's output improvements into InteractiveUIHandler, producing a single unified interactive handler that combines RichUIHandler's stable append-only output with the old InteractiveUIHandler's prompt_toolkit-based input handling.

This PR contains the design and consensus documents only — implementation will follow in subsequent commits.

Background

Three domain reviews were conducted:

  • functional-analyst: 35+ method merge plan, LiveDisplay usage audit, test strategy, 14 acceptance criteria
  • api-architect: Protocol/Bridge/Batch unchanged, LiveDisplay safe to remove, __init__ params to preserve, found an f-string bug in output_stats
  • ui-ux-designer: Append-only output is the right direction, flagged 5 blockers (stubbed methods must be implemented), long-value guard, processing feedback concern

Owner corrections were applied as binding decisions (see reporting/ui-back-port/consensus.md).

Confirmed Design

  • Base: RichUIHandler (stable, append-only console.print, no Live region)
  • Input: prompt_toolkit with lazy PromptSession (created on first get_input, not in __init__)
  • Input rendering: erase_when_done=True → input erased → output_prompt renders in Panel
  • Processing feedback: rich.status manual start/stop — stop on first token
  • Removed: LiveDisplay (spinner.py), _exit_live, _ensure_live, state flags, spinner
  • Added back: output_step_title, output_command_result, output_content, confirm_approval, agent_spawned/agent_finished, _print_wrapped, shutdown — adapted to stable console.print
  • Preserved from old: FileHistory, multi-line Esc+Enter, set_input_messages, history_file, wrap_width, show_* flags, show_stats=True default
  • Fixed: output_stats f-string bug (missing f prefix in RichUIHandler)
  • Guard: _format_tool_details caps values > 60 chars, suppresses content/old_string/new_string for write/update

Owner Corrections (binding)

  1. start() signature: KEEP title/version kwargs — external consumers (yoker-assistant) use them.
  2. output_prompt: KEEP as a SEPARATE method — not absorbed into get_input. yoker-assistant gets prompts from email (MCP), not terminal input.
  3. Processing feedback: Use rich.status with manual start/stop (not context manager) — stop when first response token arrives. NOT a LiveDisplay grid.
  4. Visual formatting: Preserve all of RichUIHandler's output improvements (inline tool args, result size, prompt Panel, tools list in banner, grey74 thinking).

Changes

  • analysis/ui-back-port-design.md — functional-analyst design
  • analysis/ui-back-port-api-review.md — api-architect review
  • analysis/ui-back-port-ux-review.md — ui-ux-designer review
  • reporting/ui-back-port/consensus.md — consensus document with owner corrections and 14 acceptance criteria

Test Plan

  • Implementation follows the 14 acceptance criteria in reporting/ui-back-port/consensus.md
  • make check passes
  • Manual REPL smoke test: input → Panel → Processing... → response streams

🤖 Implemented together with a coding agent.

Three domain reviews (functional, api-architect, ui-ux-designer) + owner
corrections. RichUIHandler output + old InteractiveUIHandler input = new
InteractiveUIHandler. LiveDisplay removed. Lazy PromptSession. erase_when_done
+ output_prompt. rich.status for processing feedback.

🤖 Implemented together with a coding agent.
@christophevg-agent

Copy link
Copy Markdown
Collaborator Author

Implementation Plan — Back-port RichUIHandler output improvements to InteractiveUIHandler

Confirmed Design (with owner corrections)

Base: RichUIHandler (from ../yoker-assistant/src/yoker_assistant/handler.py) — stable, append-only console.print, no Live region, no spinner, no state flags.

Input: prompt_toolkit with lazy PromptSession (created on first get_input call, not in __init__). Key bindings, FileHistory, InMemoryHistory, multiline Esc+Enter, set_input_messages all preserved.

Input rendering: erase_when_done=True on get_input/get_secret_input → input erased after Enter → output_prompt renders it in a styled Panel. output_prompt stays a separate method (not absorbed into get_input) — external consumers like yoker-assistant call it independently for prompts from non-terminal sources (email/MCP).

Processing feedback: rich.status with manual start()/stop() — start when agent begins processing, stop when first response token arrives. NOT a LiveDisplay grid, just a single status line.

Removed: LiveDisplay (src/yoker/ui/spinner.py), _exit_live, _ensure_live, _end_turn, state flags (_thinking_shown, _content_shown, _streaming_*).

Added back from old InteractiveUIHandler (adapted to stable console.print, no _exit_live): output_step_title, output_command_result, output_content, confirm_approval, agent_spawned, agent_finished, _print_wrapped, shutdown.

Preserved from RichUIHandler: inline tool args (key=value), result size (N chars), output_prompt Panel, tools list in banner, grey74 thinking with blank-line framing.

Preserved from old InteractiveUIHandler: FileHistory, InMemoryHistory, multiline Esc+Enter, set_input_messages, history_file param, wrap_width param, show_* flags, show_stats=True default.

Preserved per owner correction: start() title/version kwargs — external consumers (yoker-assistant) use them.

Fixed: output_stats f-string bug in RichUIHandler (missing f prefix).

Guard: _format_tool_details caps values > 60 chars, suppresses content/old_string/new_string for write/update tools.

Method-by-method plan (35+ methods)

Method Action Notes
__init__ Merge RichUIHandler base + old params (history_file, wrap_width, show_* flags, show_stats=True, show_time=False). Lazy PromptSession: self._session = None
_get_or_create_session New Lazy PromptSession builder — called on first get_input/get_secret_input. Key bindings + history set up here
_create_session Adapt From old InteractiveUIHandler — KeyBindings, FileHistory/InMemoryHistory, multiline
start Merge RichUIHandler banner (tools list) + KEEP title/version kwargs
shutdown Add back "Goodbye!" print (no _exit_live)
get_input Add prompt_toolkit with erase_when_done=True → then output_prompt(result)
get_secret_input Add prompt_toolkit with erase_when_done=True + is_password=True (no output_prompt for secrets)
set_input_messages Add Predefined input (no session needed — short-circuits before session creation)
output_prompt Keep Separate method — styled Panel rendering
output_info Keep Plain console.print
output_thinking / stream_thinking Keep grey74, inline, blank-line framing
start_content_stream / stream_content / end_content_stream Keep Append-only console.print(chunk, end="")
output_tool_call Keep ⏺ name(key=value, ...) inline args
output_tool_result Keep ✓ Success (N chars)
output_tool_content Keep From RichUIHandler
output_step_title Add back Bootstrap wizard step headers
output_command_result Add back Slash command output
output_content Add back Content display
output_stats Fix + Keep Fix f-string bug, keep RichUIHandler format
output_error Keep Styled error output
confirm_approval Add back Protected files approval (from old handler, no _exit_live)
agent_spawned / agent_finished Add back Multi-agent lifecycle (2 lines each)
_print_wrapped Add back Width-based wrapping
_format_tool_details Keep + Guard Cap values > 60 chars, suppress content/old_string/new_string
_ensure_live / _exit_live / _end_turn Remove LiveDisplay machinery
_thinking_shown / _content_shown / _streaming_* Remove State flags
Processing status New rich.status manual start/stop — start on processing begin, stop on first token

Files

Modified:

  • src/yoker/ui/interactive.py — rewritten (merge of both handlers)
  • src/yoker/ui/__init__.py — remove LiveDisplay/live_display exports
  • tests/test_ui/test_interactive.py — rewrite (console-capture instead of Live-state assertions, lazy-session tests)
  • tests/test_ui/test_handler.py — trim LiveDisplay imports
  • tests/test_bootstrap/test_history_security.py — call _get_or_create_session() before inspecting .history
  • tests/test_ui/test_confirm_approval.py — widen PromptSession patch for lazy init

Deleted:

  • src/yoker/ui/spinner.py — LiveDisplay
  • tests/test_ui/test_spinner.py — spinner tests
  • tests/events/test_spinner.py — spinner event tests

Unchanged:

  • src/yoker/ui/handler.py — Protocol
  • src/yoker/ui/bridge.py — UIBridge
  • src/yoker/ui/batch.py — BatchUIHandler
  • src/yoker/cli/chat.py — CLI wiring
  • src/yoker/cli/init.py — init wiring
  • examples/custom_handler.py — PrintUIHandler

Acceptance Criteria (14)

  1. LiveDisplay (spinner.py) removed — no Live region, no _exit_live/_ensure_live
  2. Lazy PromptSession — created on first get_input, not in init
  3. erase_when_done=True on get_input/get_secret_input
  4. output_prompt renders input in styled Panel after erase
  5. Processing feedback via rich.status (manual start/stop, stop on first token)
  6. Inline tool args (key=value) in output_tool_call
  7. Result size (N chars) in output_tool_result
  8. Tools list in welcome banner
  9. All stubbed methods implemented: output_command_result, output_step_title, get_secret_input, confirm_approval
  10. FileHistory, multi-line, agent_spawned/agent_finished, shutdown preserved
  11. start() preserves title/version kwargs
  12. output_prompt is a separate method (not absorbed into get_input)
  13. make check passes
  14. Manual REPL smoke test: input → Panel → Processing... → response streams

Open questions (from functional-analyst, with sensible defaults)

  1. Skip empty-input Panel? Default: yes — if user just presses Enter, don't render an empty Panel.
  2. Stats format? Default: keep RichUIHandler's simpler format (consistent with the simplification goal).
  3. Banner hint (use /think ...)? Default: keep it (useful info, no harm).
  4. confirm_approval test patch shape? Implementation detail — fixture-based (cleaner).
  5. LiveDisplay removal from yoker.ui exports? Clean removal (pre-1.0, no deprecation shim needed).

These defaults will be applied unless you object.


Waiting for your approval before proceeding with implementation.

@christophevg

Copy link
Copy Markdown
Owner

Approved, proceed 👍

…moval, 5 missing tests

🤖 Implemented together with a coding agent.
@christophevg-agent

Copy link
Copy Markdown
Collaborator Author

Addressed the consolidated PR #54 review feedback. Commit 525c5ae on feature/ui-back-port:

1. testing-engineer's 5 test gaps — all closed

Five new tests added to tests/test_ui/test_interactive.py:

  • test_get_input_erase_when_done_toggle — erase_when_day toggle behavior
  • test_get_input_erase_when_done_reset_after_exception — erase_when_done is reset even when an exception escapes get_input
  • test_rich_status_started_and_stopped — rich.status instance is started and stopped cleanly
  • test_status_stops_on_first_chunk — status is stopped on the first streamed chunk
  • test_get_secret_input_predefined_input_path — get_secret_input returns the predefined input without prompting

2. code-reviewer M1 (erase_when_day try/finally) — implemented

get_input and get_secret_input now wrap the prompt loop in try/finally that resets erase_when_done = False on the session, so the audit-trail worry for confirm_approval (which needs erase_when_done left at its default) is closed even when an exception escapes the prompt loop.

3. code-reviewer M3 + L3 (dead code + wrap_width) — deleted

Removed the unused helpers _capitalize, _extract_filename, and _print_wrapped, plus the unused wrap_width parameter that was threaded down to them.

4. functional-analyst obs #1 (is_password reset on interrupt) — folded into finally

The is_password flag reset is now folded into the same finally block in get_secret_input, so the terminal is returned to normal rendering even if the user interrupts password entry (Ctrl+C / EOF).

5. Deliberately skipped (non-blocking)

  • M2 (prompt_async unification) — deferred; not required for this cycle
  • L1, L2, L4 — minor style nits, deferred

6. Re-review outcome

The scoped re-review (Stage a + Stage c testing + Stage c code) all approved. make check is green — 2187 tests passed.

…dler (Phase 1: implementation + docs)

🤖 Implemented together with a coding agent.
@christophevg-agent
christophevg-agent marked this pull request as ready for review July 28, 2026 07:13
@christophevg-agent

Copy link
Copy Markdown
Collaborator Author

Implementation complete. The UI back-port merges RichUIHandler's stable append-only output with the old InteractiveUIHandler's input machinery. Key features: lazy PromptSession, erase_when_done input-erased-then-Panel, rich.status manual start/stop processing feedback, all RichUIHandler visual formatting preserved (inline tool args, result size, prompt Panel, tools banner). LiveDisplay removed (spinner.py deleted).

Passed the full review cycle (Phase 5.6):

  • functional-analyst: all 14 acceptance criteria met, all 5 owner corrections honored
  • ui-ux-designer: all 5 UX blockers resolved
  • testing-engineer: all 5 test gaps closed
  • code-reviewer: M1/M3/L3 fixed, 5/5 maintainability
  • make check: 2187 passed, 8 skipped

Ready for your review.

@christophevg
christophevg merged commit f9b2198 into master Jul 28, 2026
12 checks passed
christophevg pushed a commit that referenced this pull request Jul 28, 2026
…moval, 5 missing tests

🤖 Implemented together with a coding agent.
christophevg pushed a commit that referenced this pull request Jul 28, 2026
🤖 Implemented together with a coding agent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants