feat(tui): port full-featured Ratatui terminal UI onto staging

[serrrfirat]

Summary

Supersedes #1870.

Ports the Ratatui TUI onto current staging, preserving the feature work from the drifted branch while integrating against the current runtime and channel contracts. This branch also adds a TUI-native /model picker flow so model switching no longer relies on parsing a raw slash-command response.

Notable pieces included here:

• Ratatui TUI channel and startup wiring on top of current staging
• Mouse support for tabs, approvals, thread picker, modal-aware scrolling, and overlay dismissal
• Visible-text drag selection and copy support in the TUI
• Plain Up / Down input history navigation at chat-bar edges, while preserving multiline editing behavior
• Layout loading from tui/layout.json plus TUI_THEME / TUI_SIDEBAR wiring
• Stable tool call_id propagation so duplicate same-name tool calls do not collide in the TUI
• Thread sidebar fixes so real thread-list events populate the thread UI instead of mirroring jobs
• Non-streaming response handling fix, terminal restore guard, and TUI log/stderr behavior cleanup
• Slash-command /model now opens a dedicated model picker in the TUI
• The picker is seeded from startup model discovery when available
• If startup discovery is unavailable, the first /model response hydrates the picker and later selections submit /model <name> directly
• If live model discovery is still unavailable, /model falls back cleanly to the raw command instead of trapping the user in the slash overlay

Test Plan

• cargo fmt --all
• CARGO_TARGET_DIR=/tmp/ironclaw-tui-staging-target cargo test -p ironclaw_tui app::tests:: --offline
• CARGO_TARGET_DIR=/tmp/ironclaw-tui-staging-target cargo check -p ironclaw_tui --all-targets --offline
• CARGO_TARGET_DIR=/tmp/ironclaw-tui-staging-target cargo check --features tui --tests --offline
• cargo test -p ironclaw_tui slash_model
• cargo test -p ironclaw_tui model_response_hydrates_picker_after_first_fetch
• cargo check --features tui --bin ironclaw

Notes

This branch is the staging-based replacement for the closed drifted PR. The TUI interaction work added after the original branch drifted is included here as part of the port.

Full provider onboarding still stays in ironclaw onboard; the TUI change here is intentionally scoped to switching between already-configured models.

[gemini-code-assist]

Code Review

This pull request introduces a modular, Ratatui-based Terminal User Interface (TUI) for IronClaw, implemented as a self-contained crate. Key features include a widget system, configurable layouts, markdown rendering with syntax highlighting, and an event loop that integrates terminal input with agent status updates. Feedback focuses on critical performance optimizations, specifically regarding inefficient screen snapshotting and redundant conversation rendering. Additionally, improvements are suggested for more robust whitespace handling in text wrapping, more flexible context window inference, and standardizing timestamp parsing to avoid incorrect thread activity displays.

[gemini-code-assist]

The capture_screen_snapshot function is extremely inefficient as it is called at the end of every frame (~30 times per second). It iterates over every cell on the screen and performs a String allocation for each symbol (symbol().to_string()). On a typical terminal, this results in thousands of allocations per frame, which will significantly impact performance and CPU usage.

Consider cloning the ratatui::buffer::Buffer directly (as suggested in the ScreenSnapshot struct change) and only performing the symbol-to-string conversion when the user actually performs a copy operation.

fn capture_screen_snapshot(frame: &mut ratatui::Frame<'_>, state: &mut AppState) {
    state.screen_snapshot = ScreenSnapshot {
        area: frame.area(),
        buffer: frame.buffer_mut().clone(),
    };
}

[serrrfirat]

Addressed in ef1efaf3. capture_screen_snapshot now stores a cloned ratatui::buffer::Buffer directly instead of allocating one String per cell each frame, and symbol-to-string conversion is deferred until copy extraction.

[gemini-code-assist]

Using Vec<Vec> to store a screen snapshot is very memory-intensive and leads to excessive allocations during capture. Since this crate already depends on ratatui, it is much more efficient to store a ratatui::buffer::Buffer directly, which can be cloned in a single allocation.

Suggested change

	#[derive(Debug, Clone, Default)]
	pub struct ScreenSnapshot {
	pub area: Rect,
	pub rows: Vec<Vec<String>>,
	}
	#[derive(Debug, Clone, Default)]
	pub struct ScreenSnapshot {
	pub area: Rect,
	pub buffer: Buffer,
	}

[serrrfirat]

Addressed in ef1efaf3. ScreenSnapshot now holds { area, buffer: Buffer } instead of Vec<Vec<String>>, with a lightweight default backed by Buffer::empty(area).

[gemini-code-assist]

This function should be updated to work with the optimized ScreenSnapshot using a ratatui::buffer::Buffer.

fn snapshot_symbol(snapshot: &ScreenSnapshot, column: u16, row: u16) -> Option<&str> {
    if column >= snapshot.area.x && column < snapshot.area.x + snapshot.area.width && row >= snapshot.area.y && row < snapshot.area.y + snapshot.area.height {
        Some(snapshot.buffer[(column, row)].symbol())
    } else {
        None
    }
}

[serrrfirat]

Addressed in ef1efaf3. snapshot_symbol now bounds-checks against the snapshot rect and reads directly from snapshot.buffer[(column, row)].symbol().

[gemini-code-assist]

The render function re-processes the entire conversation history on every frame, including markdown parsing and text wrapping for every message. This will cause significant UI lag as the conversation grows.

Consider caching the rendered Lines for each message in AppState. You only need to re-render a message if the available width changes or if the message content is updated (e.g., during streaming).

[serrrfirat]

Addressed in ef1efaf3. I added a per-message render cache in the conversation widget keyed by message content/position plus usable width, so markdown parsing and wrapping are only recomputed when a message changes or the available width changes.

[gemini-code-assist]

Using split_whitespace() causes the loss of multiple consecutive spaces and other formatting whitespace within the text. This can lead to incorrect display of user input or system messages where spacing is intentional.

References

1. When reconstructing a string from whitespace-separated fields, prefer joining the collected fields over re-trimming the original string to handle extra spaces robustly.

[serrrfirat]

Addressed in ef1efaf3. wrap_text no longer uses split_whitespace(); it now preserves consecutive spaces (and expands tabs consistently) while wrapping.

[gemini-code-assist]

The infer_context_window function relies on hardcoded model name heuristics, some of which appear to be placeholders (e.g., gpt-5.4, gpt-4.1). This is fragile and will likely be incorrect for many models.

It would be better to retrieve the context window size from the model provider's metadata if available, or allow it to be explicitly configured in the TUI settings.

[serrrfirat]

Addressed in ef1efaf3. The TUI now prefers llm.model_metadata() at startup to seed the context window shown in the UI, and only falls back to a conservative heuristic when the provider does not expose metadata.

[gemini-code-assist]

The parse_thread_activity_timestamp function uses a very specific format (%Y-%m-%d %H:%M) which will fail to parse standard ISO 8601 timestamps. If parsing fails, it defaults to chrono::Utc::now(), which will cause the sidebar to display incorrect uptime (0s) for all resumed threads. Per repository rules, do not fall back to the current time if a timestamp cannot be parsed, as this makes the resource appear perpetually new. Instead, log a warning and handle the missing data.

References

1. When calculating the age of a resource for cleanup, if the creation timestamp is unknown or cannot be parsed, treat it as un-reapable by logging a warning and skipping it. Do not fall back to the current time, as this will make the resource perpetually appear new and prevent it from ever being cleaned up.

[serrrfirat]

Addressed in ef1efaf3. On this staging-based branch the problematic fallback lived in the engine thread timestamp mapping path, so I fixed it there: we now parse RFC3339 plus the legacy %Y-%m-%d %H:%M format, and invalid timestamps stay None instead of falling back to Utc::now(). The sidebar shows unknown uptime rather than misleading 0s.

[ilblackdragon]

Automated review — Needs changes (performance + correctness)

TL;DR: Full-featured Ratatui TUI port is well-structured with proper terminal restoration and correct logging levels, but has 5 performance/correctness issues that should be addressed before merge. Snapshot allocations and per-frame re-render are the big ones.

TUI quality baseline ✓

• Logging: only `error!` used — no `info!`/`warn!` in render loops (respects CLAUDE.md rule)
• Terminal restoration: `TerminalRestoreGuard` in Drop ensures cleanup on panic
• Error handling: production uses `Result`; tests use `.expect()`
• Event handling: comprehensive (key, mouse, resize, bracketed paste)
• Signal handling: Ctrl+C exit via event loop

Findings (ordered by load-bearing impact)

#	Severity	File:Line	Issue	Suggestion
1	High	`app.rs:2298` (`capture_screen_snapshot`)	Allocates thousands of strings per frame (~30fps) via `symbol().to_string()` on every cell	Clone `ratatui::buffer::Buffer` directly; defer string conversion until the user actually triggers a copy action
2	Medium	`widgets/mod.rs:271`	`Vec<Vec>` screen snapshot is memory-inefficient	Store `Buffer` in `ScreenSnapshot` struct instead
3	Medium	`widgets/conversation.rs:146`	Entire conversation re-parsed and wrapped every frame (markdown + wrap for all messages)	Cache rendered `Lines` per message; invalidate only on width change or message update
4	Medium	`render.rs:23`	`split_whitespace()` loses consecutive spaces → breaks intentional spacing in user input	Preserve whitespace; collect-and-join instead of re-trimming with `split_whitespace`
5	Medium	`src/channels/tui.rs:128` (`infer_context_window`)	Hardcodes placeholder model names (gpt-5.4, gpt-4.1)	Fetch from provider metadata (add to `LlmProvider` trait if needed) or expose explicit TUI config
6	Medium	`src/channels/tui.rs` (`parse_thread_activity_timestamp`)	On parse failure, falls back to `now()` → all threads show "0s uptime" indefinitely	Log a warning and keep `None`; do not fall back to current time

Open questions

1. Are the snapshot optimizations (#1, #2) blocking merge, or can they land post-merge as a perf follow-up? They are in the render hot path, so strongly recommend fixing before merge.
2. Does the timestamp fallback (#6) manifest in user testing, or is it a hypothetical edge case?
3. Context window inference: should this come from a discovery API (provider trait method) instead of hardcoded heuristics?

[serrrfirat]

Pushed ef1efaf3 to firat/feat-tui-on-staging to address the review feedback.

Covered in this update:

• switched TUI screen snapshots to cloned ratatui::buffer::Buffer
• updated selection/copy to read symbols lazily from that buffer
• added per-message conversation render caching
• preserved whitespace in plain-text wrapping
• seeded TUI context window from provider model_metadata() when available
• removed the now() fallback for unparseable engine thread timestamps

Verification run on this branch:

• cargo fmt --all
• cargo test -p ironclaw_tui --offline
• cargo test --features tui channels::tui::tests --offline

[ilblackdragon]

Interrupt during processing: input layer fixed, agent dispatch still blocked

Nice to see the TUI coming back on staging. Wanted to flag a gap I hit while tracing the Esc-interrupt path — the new TUI fixes the input-layer problem the REPL has, but the agent-dispatch bottleneck underneath is still there, so Esc mid-turn won't actually stop the running thread.

The trace

Input layer — this PR fixes it. In crates/ironclaw_tui/src/app.rs:456, InputAction::Interrupt sends TuiUserMessage::text_only("/interrupt") via msg_tx. Ratatui's event loop runs independently of the agent, so unlike the REPL (where rl.readline() is parked behind stdin_locked during processing — see src/channels/repl.rs:565), the TUI can actually produce the keystroke mid-turn. Good.

Agent dispatch layer — still blocked. The outer agent loop in src/agent/agent_loop.rs:941 awaits handle_message inline:

match self.handle_message(&message).await { ... }

For a UserInput, this routes through src/bridge/router.rs:handle_with_engine_inner → await_thread_outcome (router.rs:1981), which blocks on the thread's event stream for up to 5 minutes. Until that returns, message_stream.next() is not polled — so the /interrupt message the TUI just sent sits in the channel queue and only dequeues after the turn it was supposed to stop has ended.

The engine-v2 interrupt machinery itself is fine: handle_interrupt at src/bridge/router.rs:1435 correctly calls ThreadManager::stop_thread() on running threads. It's just unreachable from user input while a turn is in flight.

Suggested fix (TUI-local, no agent refactor)

Add an out-of-band interrupt path that bypasses the message channel:

1. Define a narrow InterruptHandle trait in ironclaw_tui — keeps the crate decoupled from the engine:

   #[async_trait]
   pub trait InterruptHandle: Send + Sync {
       async fn interrupt_active(&self, user_id: &str, channel: &str);
   }

2. Pass an Arc<dyn InterruptHandle> into TuiAppConfig / start_tui.
3. In handle_event where InputAction::Interrupt is matched (app.rs:456), call handle.interrupt_active(...).await in addition to sending the /interrupt message (keep the message as a fallback / for the post-turn "Nothing to interrupt." case).
4. In src/channels/tui.rs, implement InterruptHandle with a concrete type that calls into src/bridge/router.rs::handle_interrupt logic — i.e. look up the active engine conversation for (channel, user_id) and call ThreadManager::stop_thread() on each running thread. This mirrors what handle_interrupt already does, but runs directly from the TUI event loop instead of waiting its turn in the channel queue.

No changes needed to agent_loop.rs or the v2 engine — the machinery is all there, just needs a direct call path.

Why this matters beyond Esc

The same bottleneck affects any future "while processing" feature — cancel buttons, timeouts, status pokes, etc. Landing the out-of-band handle in this PR sets up the pattern. Happy to do the follow-up if it's out of scope here — just wanted to flag it while the TUI architecture is fresh.

Context on the trace: I was evaluating whether an older REPL branch of mine (feat/repl-interrupt-during-processing) was superseded by this PR + #1412 + #1557. The REPL-specific pieces are superseded, but the out-of-band-flag idea from that branch is still load-bearing for the agent-dispatch layer.

[ilblackdragon]

Verdict: Approve

• ~9.5k LOC port into a new crates/ironclaw_tui crate plus a 704-line src/channels/tui.rs adapter. Well-isolated, has its own CLAUDE.md.
• All tracing::warn! calls are in foreground request handlers / startup paths — none in background tasks, so the TUI corruption rule is respected. Worth a quick scan post-merge to confirm no info! fires from background tasks once the TUI is attached.
• All .expect() instances are confined to #[cfg(test)] modules or the explicit examples/dev.rs binary. LAST_COPIED_TEXT mutex correctly handles poison via unwrap_or_else(|e| e.into_inner()).
• src/main.rs use_repl = !tui_mode || cfg!(not(feature = "tui")) correctly falls back when feature missing.
• Stable call_id map prevents the duplicate-tool-call collision regression.
• Strong unit coverage: slash_model, picker hydration, thread switching, snapshot rendering, layout/theme round-trips, approval re-emit. Router changes covered via router approval re-emit test.
• app.rs (3,342 lines) and render.rs (1,135 lines) are large — consider splitting in a follow-up.