From 4336dc7b3ca8ee909a7c5a3805c4d18978feaea3 Mon Sep 17 00:00:00 2001 From: Drew Galbraith Date: Wed, 25 Feb 2026 23:30:52 -0800 Subject: [PATCH] Display diff --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/core/orchestrator.rs | 100 ++++++++++++++++++++++++-- src/core/types.rs | 31 +++++++- src/tui/events.rs | 148 +++++++++++++++++++++++++++------------ src/tui/input.rs | 58 +++++++++++++-- src/tui/mod.rs | 22 +++++- src/tui/render.rs | 95 ++++++++++++------------- src/tui/tool_display.rs | 133 +++++++++++++++++++++++++++++++++++ 9 files changed, 481 insertions(+), 114 deletions(-) create mode 100644 src/tui/tool_display.rs diff --git a/Cargo.lock b/Cargo.lock index 65bb80c..a8d0402 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2061,6 +2061,12 @@ dependencies = [ "libc", ] +[[package]] +name = "similar" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" + [[package]] name = "siphasher" version = "1.0.2" @@ -2080,6 +2086,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "similar", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 97f34f8..dfd3141 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } reqwest = { version = "0.13", features = ["stream", "json"] } futures = "0.3" async-trait = "0.1" +similar = "2" landlock = "0.4" [dev-dependencies] diff --git a/src/core/orchestrator.rs b/src/core/orchestrator.rs index 319b657..7cfacf5 100644 --- a/src/core/orchestrator.rs +++ b/src/core/orchestrator.rs @@ -4,8 +4,8 @@ use tracing::{debug, warn}; use crate::core::history::ConversationHistory; use crate::core::types::{ - ContentBlock, ConversationMessage, Role, StampedEvent, StreamEvent, ToolDefinition, UIEvent, - UserAction, + ContentBlock, ConversationMessage, Role, StampedEvent, StreamEvent, ToolDefinition, + ToolDisplay, UIEvent, UserAction, }; use crate::provider::ModelProvider; use crate::sandbox::Sandbox; @@ -303,6 +303,86 @@ impl Orchestrator

{ .await; } + /// Build a [`ToolDisplay`] from the tool name and its JSON input. + /// + /// Matches on known tool names to extract structured fields; falls back to + /// `Generic` with a JSON summary for anything else. + fn build_tool_display(&self, tool_name: &str, input: &serde_json::Value) -> ToolDisplay { + match tool_name { + "write_file" => { + let path = input["path"].as_str().unwrap_or("").to_string(); + let new_content = input["content"].as_str().unwrap_or("").to_string(); + // Try to read existing content for diffing. + let old_content = self.sandbox.read_file(&path).ok(); + ToolDisplay::WriteFile { + path, + old_content, + new_content, + } + } + "shell_exec" => { + let command = input["command"].as_str().unwrap_or("").to_string(); + ToolDisplay::ShellExec { command } + } + "list_directory" => { + let path = input["path"].as_str().unwrap_or(".").to_string(); + ToolDisplay::ListDirectory { path } + } + "read_file" => { + let path = input["path"].as_str().unwrap_or("").to_string(); + ToolDisplay::ReadFile { path } + } + _ => ToolDisplay::Generic { + summary: serde_json::to_string(input).unwrap_or_default(), + }, + } + } + + /// Build a [`ToolDisplay`] for a tool result, incorporating the output content. + fn build_result_display( + &self, + tool_name: &str, + input: &serde_json::Value, + output: &str, + ) -> ToolDisplay { + match tool_name { + "write_file" => { + let path = input["path"].as_str().unwrap_or("").to_string(); + let new_content = input["content"].as_str().unwrap_or("").to_string(); + // For results, old_content isn't available post-write. We already + // showed the diff at approval/executing time; the result just + // confirms the byte count via the display formatter. + ToolDisplay::WriteFile { + path, + old_content: None, + new_content, + } + } + "shell_exec" => { + let command = input["command"].as_str().unwrap_or("").to_string(); + ToolDisplay::ShellExec { + command: format!("{command}\n{output}"), + } + } + "list_directory" => { + let path = input["path"].as_str().unwrap_or(".").to_string(); + ToolDisplay::ListDirectory { + path: format!("{path}\n{output}"), + } + } + "read_file" => { + let path = input["path"].as_str().unwrap_or("").to_string(); + let line_count = output.lines().count(); + ToolDisplay::ReadFile { + path: format!("{path} ({line_count} lines)"), + } + } + _ => ToolDisplay::Generic { + summary: truncate(output, 200), + }, + } + } + /// Execute a single tool, handling approval if needed. /// /// For auto-approve tools, executes immediately. For tools requiring @@ -325,14 +405,15 @@ impl Orchestrator

{ } }; - let input_summary = serde_json::to_string(input).unwrap_or_default(); + let display = self.build_tool_display(tool_name, input); // Check approval. let approved = match risk { RiskLevel::AutoApprove => { self.send(UIEvent::ToolExecuting { + tool_use_id: tool_use_id.to_string(), tool_name: tool_name.to_string(), - input_summary: input_summary.clone(), + display, }) .await; true @@ -341,7 +422,7 @@ impl Orchestrator

{ self.send(UIEvent::ToolApprovalRequest { tool_use_id: tool_use_id.to_string(), tool_name: tool_name.to_string(), - input_summary: input_summary.clone(), + display, }) .await; @@ -361,9 +442,11 @@ impl Orchestrator

{ let tool = self.tool_registry.get(tool_name).unwrap(); match tool.execute(input, &self.sandbox).await { Ok(output) => { + let result_display = self.build_result_display(tool_name, input, &output.content); self.send(UIEvent::ToolResult { + tool_use_id: tool_use_id.to_string(), tool_name: tool_name.to_string(), - output_summary: truncate(&output.content, 200), + display: result_display, is_error: output.is_error, }) .await; @@ -372,8 +455,11 @@ impl Orchestrator

{ Err(e) => { let msg = e.to_string(); self.send(UIEvent::ToolResult { + tool_use_id: tool_use_id.to_string(), tool_name: tool_name.to_string(), - output_summary: msg.clone(), + display: ToolDisplay::Generic { + summary: msg.clone(), + }, is_error: true, }) .await; diff --git a/src/core/types.rs b/src/core/types.rs index 212aa82..0d1bfd1 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -39,6 +39,29 @@ pub enum UserAction { SetNetworkPolicy(bool), } +/// Structured display information for a tool invocation. +/// +/// Each variant carries the data needed to render a tool-specific view in the +/// TUI -- diffs for writes, command lines for shell exec, etc. The TUI's +/// `tool_display` module formats these into user-facing strings. +#[derive(Debug, Clone)] +pub enum ToolDisplay { + /// A file write with optional diff against previous content. + WriteFile { + path: String, + old_content: Option, + new_content: String, + }, + /// A shell command execution. + ShellExec { command: String }, + /// A directory listing. + ListDirectory { path: String }, + /// A file read. + ReadFile { path: String }, + /// Fallback for unknown or unstructured tools. + Generic { summary: String }, +} + /// An event sent from the core orchestrator to the TUI. #[derive(Debug)] pub enum UIEvent { @@ -48,17 +71,19 @@ pub enum UIEvent { ToolApprovalRequest { tool_use_id: String, tool_name: String, - input_summary: String, + display: ToolDisplay, }, /// A tool is being executed (informational, after approval or auto-approve). ToolExecuting { + tool_use_id: String, tool_name: String, - input_summary: String, + display: ToolDisplay, }, /// A tool has finished executing. ToolResult { + tool_use_id: String, tool_name: String, - output_summary: String, + display: ToolDisplay, is_error: bool, }, /// The network policy has changed (sent after `:net on/off` is processed). diff --git a/src/tui/events.rs b/src/tui/events.rs index 064c8bb..5673515 100644 --- a/src/tui/events.rs +++ b/src/tui/events.rs @@ -3,20 +3,25 @@ use tokio::sync::mpsc; use tracing::debug; -use super::AppState; +use super::{AppState, DisplayMessage}; use crate::core::types::{Role, StampedEvent, UIEvent}; +use crate::tui::tool_display; /// Drain all pending [`UIEvent`]s from `event_rx` and apply them to `state`. /// /// This is non-blocking: it processes all currently-available events and returns /// immediately when the channel is empty. /// +/// Tool events use in-place replacement: when a `ToolExecuting` or `ToolResult` +/// arrives, the handler searches `state.messages` for an existing entry with the +/// same `tool_use_id` and replaces its content rather than appending a new row. +/// /// | Event | Effect | /// |------------------------|------------------------------------------------------------| /// | `StreamDelta(s)` | Append `s` to last message if it's `Assistant`; else push | -/// | `ToolApprovalRequest` | Set `pending_approval` in state | -/// | `ToolExecuting` | Display tool execution info | -/// | `ToolResult` | Display tool result | +/// | `ToolApprovalRequest` | Push inline message with approval prompt, set pending | +/// | `ToolExecuting` | Replace approval message in-place (or push new) | +/// | `ToolResult` | Replace executing message in-place (or push new) | /// | `TurnComplete` | No structural change; logged at debug level | /// | `Error(msg)` | Push `(Assistant, "[error] {msg}")` | pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver, state: &mut AppState) { @@ -32,43 +37,57 @@ pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver, state } match stamped.event { UIEvent::StreamDelta(chunk) => { - if let Some((Role::Assistant, content)) = state.messages.last_mut() { - content.push_str(&chunk); + if let Some(msg) = state.messages.last_mut() { + if msg.role == Role::Assistant && msg.tool_use_id.is_none() { + msg.content.push_str(&chunk); + } else { + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: chunk, + tool_use_id: None, + }); + } } else { - state.messages.push((Role::Assistant, chunk)); + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: chunk, + tool_use_id: None, + }); } state.content_changed = true; } UIEvent::ToolApprovalRequest { tool_use_id, tool_name, - input_summary, + display, } => { - state.pending_approval = Some(PendingApproval { - tool_use_id, - tool_name, - input_summary, + let mut content = tool_display::format_executing(&tool_name, &display); + content.push_str("\n[y] approve [n] deny"); + state.messages.push(DisplayMessage { + role: Role::Assistant, + content, + tool_use_id: Some(tool_use_id.clone()), }); + state.pending_approval = Some(PendingApproval { tool_use_id }); + state.content_changed = true; } UIEvent::ToolExecuting { + tool_use_id, tool_name, - input_summary, + display, } => { - state - .messages - .push((Role::Assistant, format!("[{tool_name}] {input_summary}"))); + let content = tool_display::format_executing(&tool_name, &display); + replace_or_push(state, &tool_use_id, content); state.content_changed = true; } UIEvent::ToolResult { + tool_use_id, tool_name, - output_summary, + display, is_error, } => { - let prefix = if is_error { "error" } else { "result" }; - state.messages.push(( - Role::Assistant, - format!("[{tool_name} {prefix}] {output_summary}"), - )); + let content = tool_display::format_result(&tool_name, &display, is_error); + replace_or_push(state, &tool_use_id, content); state.content_changed = true; } UIEvent::TurnComplete => { @@ -78,26 +97,46 @@ pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver, state state.network_allowed = allowed; } UIEvent::Error(msg) => { - state - .messages - .push((Role::Assistant, format!("[error] {msg}"))); + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: format!("[error] {msg}"), + tool_use_id: None, + }); state.content_changed = true; } } } } +/// Find the message with the given `tool_use_id` and replace its content, +/// or push a new message if not found. +fn replace_or_push(state: &mut AppState, tool_use_id: &str, content: String) { + if let Some(msg) = state + .messages + .iter_mut() + .rev() + .find(|m| m.tool_use_id.as_deref() == Some(tool_use_id)) + { + msg.content = content; + } else { + state.messages.push(DisplayMessage { + role: Role::Assistant, + content, + tool_use_id: Some(tool_use_id.to_string()), + }); + } +} + /// A pending tool approval request waiting for user input. #[derive(Debug, Clone)] pub struct PendingApproval { pub tool_use_id: String, - pub tool_name: String, - pub input_summary: String, } #[cfg(test)] mod tests { use super::*; + use crate::core::types::ToolDisplay; /// Wrap a [`UIEvent`] in a [`StampedEvent`] at epoch 0 for tests. fn stamp(event: UIEvent) -> StampedEvent { @@ -108,63 +147,86 @@ mod tests { async fn drain_appends_to_existing_assistant() { let (tx, mut rx) = tokio::sync::mpsc::channel(8); let mut state = AppState::new(); - state.messages.push((Role::Assistant, "hello".to_string())); + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: "hello".to_string(), + tool_use_id: None, + }); tx.send(stamp(UIEvent::StreamDelta(" world".to_string()))) .await .unwrap(); drop(tx); drain_ui_events(&mut rx, &mut state); - assert_eq!(state.messages.last().unwrap().1, "hello world"); + assert_eq!(state.messages.last().unwrap().content, "hello world"); } #[tokio::test] async fn drain_creates_assistant_on_user_last() { let (tx, mut rx) = tokio::sync::mpsc::channel(8); let mut state = AppState::new(); - state.messages.push((Role::User, "hi".to_string())); + state.messages.push(DisplayMessage { + role: Role::User, + content: "hi".to_string(), + tool_use_id: None, + }); tx.send(stamp(UIEvent::StreamDelta("hello".to_string()))) .await .unwrap(); drop(tx); drain_ui_events(&mut rx, &mut state); assert_eq!(state.messages.len(), 2); - assert_eq!(state.messages[1].0, Role::Assistant); - assert_eq!(state.messages[1].1, "hello"); + assert_eq!(state.messages[1].role, Role::Assistant); + assert_eq!(state.messages[1].content, "hello"); } #[tokio::test] - async fn drain_tool_approval_sets_pending() { + async fn drain_tool_approval_sets_pending_and_adds_message() { let (tx, mut rx) = tokio::sync::mpsc::channel(8); let mut state = AppState::new(); tx.send(stamp(UIEvent::ToolApprovalRequest { tool_use_id: "t1".to_string(), - tool_name: "write_file".to_string(), - input_summary: "path: foo.txt".to_string(), + tool_name: "shell_exec".to_string(), + display: ToolDisplay::ShellExec { + command: "cargo test".to_string(), + }, })) .await .unwrap(); drop(tx); drain_ui_events(&mut rx, &mut state); assert!(state.pending_approval.is_some()); - let approval = state.pending_approval.unwrap(); - assert_eq!(approval.tool_name, "write_file"); + assert_eq!(state.pending_approval.as_ref().unwrap().tool_use_id, "t1"); + // Message should be inline with approval prompt. + assert_eq!(state.messages.len(), 1); + assert!(state.messages[0].content.contains("[y] approve")); + assert_eq!(state.messages[0].tool_use_id.as_deref(), Some("t1")); } #[tokio::test] - async fn drain_tool_result_adds_message() { + async fn drain_tool_result_replaces_existing_message() { let (tx, mut rx) = tokio::sync::mpsc::channel(8); let mut state = AppState::new(); + // Simulate an existing executing message. + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: "$ cargo test".to_string(), + tool_use_id: Some("t1".to_string()), + }); tx.send(stamp(UIEvent::ToolResult { - tool_name: "read_file".to_string(), - output_summary: "file contents...".to_string(), + tool_use_id: "t1".to_string(), + tool_name: "shell_exec".to_string(), + display: ToolDisplay::ShellExec { + command: "cargo test\nok".to_string(), + }, is_error: false, })) .await .unwrap(); drop(tx); drain_ui_events(&mut rx, &mut state); + // Should replace, not append. assert_eq!(state.messages.len(), 1); - assert!(state.messages[0].1.contains("read_file result")); + assert!(state.messages[0].content.contains("cargo test")); } #[tokio::test] @@ -172,14 +234,12 @@ mod tests { let (tx, mut rx) = tokio::sync::mpsc::channel(8); let mut state = AppState::new(); state.epoch = 2; - // Event from epoch 1 should be discarded. tx.send(StampedEvent { epoch: 1, event: UIEvent::StreamDelta("ghost".to_string()), }) .await .unwrap(); - // Event from epoch 2 should be accepted. tx.send(StampedEvent { epoch: 2, event: UIEvent::StreamDelta("real".to_string()), @@ -189,7 +249,7 @@ mod tests { drop(tx); drain_ui_events(&mut rx, &mut state); assert_eq!(state.messages.len(), 1); - assert_eq!(state.messages[0].1, "real"); + assert_eq!(state.messages[0].content, "real"); } #[tokio::test] diff --git a/src/tui/input.rs b/src/tui/input.rs index 0d86d9e..ddd8926 100644 --- a/src/tui/input.rs +++ b/src/tui/input.rs @@ -28,25 +28,57 @@ pub(super) fn handle_key(key: Option, state: &mut AppState) -> Option< // Clear any transient status error on the next keypress. state.status_error = None; - // If a tool approval is pending, intercept y/n before normal key handling. + // If a tool approval is pending, intercept y/n but allow scrolling. if let Some(approval) = &state.pending_approval { let tool_use_id = approval.tool_use_id.clone(); + let is_ctrl = key.modifiers.contains(KeyModifiers::CONTROL); match key.code { - KeyCode::Char('y') | KeyCode::Char('Y') => { + KeyCode::Char('y') | KeyCode::Char('Y') if !is_ctrl => { state.pending_approval = None; return Some(LoopControl::ToolApproval { tool_use_id, approved: true, }); } - KeyCode::Char('n') | KeyCode::Char('N') => { + KeyCode::Char('n') | KeyCode::Char('N') if !is_ctrl => { state.pending_approval = None; return Some(LoopControl::ToolApproval { tool_use_id, approved: false, }); } - _ => return None, // ignore other keys while approval pending + // Allow scrolling while approval is pending. + KeyCode::Char('j') if !is_ctrl => { + state.scroll = state.scroll.saturating_add(1); + return None; + } + KeyCode::Char('k') if !is_ctrl => { + state.scroll = state.scroll.saturating_sub(1); + return None; + } + KeyCode::Char('G') if !is_ctrl => { + state.scroll = u16::MAX; + return None; + } + KeyCode::Char('g') if !is_ctrl => { + state.pending_keys.push('g'); + if state.pending_keys.ends_with(&['g', 'g']) { + state.scroll = 0; + state.pending_keys.clear(); + } + return None; + } + KeyCode::Char('d') if is_ctrl => { + let half = (state.viewport_height / 2).max(1); + state.scroll = state.scroll.saturating_add(half); + return None; + } + KeyCode::Char('u') if is_ctrl => { + let half = (state.viewport_height / 2).max(1); + state.scroll = state.scroll.saturating_sub(half); + return None; + } + _ => return None, } } @@ -179,7 +211,11 @@ fn handle_insert_key(key: KeyEvent, state: &mut AppState) -> Option if msg.is_empty() { None } else { - state.messages.push((Role::User, msg.clone())); + state.messages.push(crate::tui::DisplayMessage { + role: Role::User, + content: msg.clone(), + tool_use_id: None, + }); Some(LoopControl::SendMessage(msg)) } } @@ -502,8 +538,16 @@ mod tests { #[test] fn command_clear_empties_messages() { let mut state = AppState::new(); - state.messages.push((Role::User, "hi".to_string())); - state.messages.push((Role::Assistant, "hello".to_string())); + state.messages.push(crate::tui::DisplayMessage { + role: Role::User, + content: "hi".to_string(), + tool_use_id: None, + }); + state.messages.push(crate::tui::DisplayMessage { + role: Role::Assistant, + content: "hello".to_string(), + tool_use_id: None, + }); state.scroll = 10; let result = execute_command("clear", &mut state); assert!(matches!(result, Some(LoopControl::ClearHistory))); diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 98fd726..87df0e8 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -12,6 +12,7 @@ mod events; mod input; mod render; +pub(crate) mod tool_display; use std::io::{self, Stdout}; use std::time::Duration; @@ -54,10 +55,27 @@ pub enum Mode { Command, } +/// A single message in the TUI's display list. +/// +/// Unlike `ConversationMessage` (which models the API wire format), this struct +/// represents a rendered row in the conversation pane. Tool invocations get their +/// own `DisplayMessage` with a `tool_use_id` so that in-place replacement works: +/// the approval message becomes the executing message, then the result message. +#[derive(Debug, Clone)] +pub struct DisplayMessage { + /// Who authored this message (tool messages use `Assistant`). + pub role: Role, + /// Pre-formatted content for rendering. + pub content: String, + /// When set, this message can be replaced in-place by a later event carrying + /// the same tool-use ID (e.g. executing -> result). + pub tool_use_id: Option, +} + /// The UI-layer view of a conversation: rendered messages and the current input buffer. pub struct AppState { - /// All conversation turns rendered as (role, content) pairs. - pub messages: Vec<(Role, String)>, + /// All conversation turns rendered as display messages. + pub messages: Vec, /// The current contents of the input box. pub input: String, /// Vertical scroll offset for the output pane (lines from top). diff --git a/src/tui/render.rs b/src/tui/render.rs index bb32b57..e38e051 100644 --- a/src/tui/render.rs +++ b/src/tui/render.rs @@ -25,9 +25,9 @@ pub(super) fn update_scroll(state: &mut AppState, area: Rect) { let width = area.width.max(1) as usize; let mut total_lines: u16 = 0; - for (_, content) in &state.messages { + for msg in &state.messages { total_lines = total_lines.saturating_add(1); // role header - for line in content.lines() { + for line in msg.content.lines() { let chars = line.chars().count(); let wrapped = chars.div_ceil(width).max(1) as u16; total_lines = total_lines.saturating_add(wrapped); @@ -93,14 +93,23 @@ pub(super) fn render(frame: &mut Frame, state: &AppState) { // --- Output pane --- let mut lines: Vec = Vec::new(); - for (role, content) in &state.messages { - let (label, color) = match role { + for msg in &state.messages { + let (label, color) = match msg.role { Role::User => ("You:", Color::Cyan), Role::Assistant => ("Assistant:", Color::Green), }; lines.push(Line::from(Span::styled(label, Style::default().fg(color)))); - for body_line in content.lines() { - lines.push(Line::from(body_line.to_string())); + for body_line in msg.content.lines() { + let styled = if body_line.starts_with('+') && !body_line.starts_with("+++") { + Line::from(Span::styled(body_line, Style::default().fg(Color::Green))) + } else if body_line.starts_with('-') && !body_line.starts_with("---") { + Line::from(Span::styled(body_line, Style::default().fg(Color::Red))) + } else if body_line.starts_with("@@") { + Line::from(Span::styled(body_line, Style::default().fg(Color::Cyan))) + } else { + Line::from(body_line.to_string()) + }; + lines.push(styled); } lines.push(Line::from("")); // blank separator } @@ -111,31 +120,6 @@ pub(super) fn render(frame: &mut Frame, state: &AppState) { let output_area = chunks[0]; frame.render_widget(output, output_area); - // --- Tool approval overlay --- - if let Some(ref approval) = state.pending_approval { - let overlay_w = (output_area.width / 2).max(60).min(output_area.width); - let overlay_h: u16 = 5; - let overlay_x = output_area.x + (output_area.width.saturating_sub(overlay_w)) / 2; - let overlay_y = output_area.y + output_area.height.saturating_sub(overlay_h) / 2; - let overlay_area = Rect { - x: overlay_x, - y: overlay_y, - width: overlay_w, - height: overlay_h.min(output_area.height), - }; - frame.render_widget(Clear, overlay_area); - let text = format!( - "{}: {}\n\ny = approve, n = deny", - approval.tool_name, approval.input_summary - ); - let overlay = Paragraph::new(text).block( - Block::bordered() - .border_style(Style::default().fg(Color::Yellow)) - .title("Tool Approval"), - ); - frame.render_widget(overlay, overlay_area); - } - // --- Command overlay (floating box centered on output pane) --- if state.mode == Mode::Command { let overlay_area = command_overlay_rect(output_area); @@ -247,17 +231,25 @@ pub(super) fn render(frame: &mut Frame, state: &AppState) { #[cfg(test)] mod tests { + use super::super::DisplayMessage; use super::*; use ratatui::Terminal; use ratatui::backend::TestBackend; + fn dmsg(role: Role, content: &str) -> DisplayMessage { + DisplayMessage { + role, + content: content.to_string(), + tool_use_id: None, + } + } + #[test] fn render_smoke_test() { let backend = TestBackend::new(80, 24); let mut terminal = Terminal::new(backend).unwrap(); let state = AppState::new(); terminal.draw(|frame| render(frame, &state)).unwrap(); - // no panic is the assertion } #[test] @@ -265,8 +257,8 @@ mod tests { let backend = TestBackend::new(80, 24); let mut terminal = Terminal::new(backend).unwrap(); let mut state = AppState::new(); - state.messages.push((Role::User, "hi".to_string())); - state.messages.push((Role::Assistant, "hello".to_string())); + state.messages.push(dmsg(Role::User, "hi")); + state.messages.push(dmsg(Role::Assistant, "hello")); terminal.draw(|frame| render(frame, &state)).unwrap(); let buf = terminal.backend().buffer().clone(); let all_text: String = buf @@ -284,12 +276,10 @@ mod tests { ); } - // --- update_scroll tests --- - #[test] fn update_scroll_zero_when_fits() { let mut state = AppState::new(); - state.messages.push((Role::User, "hello".to_string())); + state.messages.push(dmsg(Role::User, "hello")); let area = Rect::new(0, 0, 80, 24); update_scroll(&mut state, area); assert_eq!(state.scroll, 0); @@ -299,16 +289,16 @@ mod tests { fn update_scroll_positive_when_overflow() { let mut state = AppState::new(); for i in 0..50 { - state.messages.push((Role::User, format!("message {i}"))); + state + .messages + .push(dmsg(Role::User, &format!("message {i}"))); } - state.content_changed = true; // simulate new content arriving + state.content_changed = true; let area = Rect::new(0, 0, 80, 24); update_scroll(&mut state, area); assert!(state.scroll > 0, "expected scroll > 0 with 50 messages"); } - // --- render snapshot tests --- - #[test] fn render_status_bar_normal_mode() { let backend = TestBackend::new(80, 24); @@ -333,7 +323,7 @@ mod tests { fn render_status_bar_insert_mode() { let backend = TestBackend::new(80, 24); let mut terminal = Terminal::new(backend).unwrap(); - let state = AppState::new(); // starts in Insert + let state = AppState::new(); terminal.draw(|frame| render(frame, &state)).unwrap(); let buf = terminal.backend().buffer().clone(); let all_text: String = buf @@ -388,7 +378,6 @@ mod tests { state.mode = Mode::Normal; terminal.draw(|frame| render(frame, &state)).unwrap(); let buf = terminal.backend().buffer().clone(); - // Row 1 should not have a ":" prefix from the overlay let row1: String = (0..80) .map(|x| buf.cell((x, 1)).unwrap().symbol().to_string()) .collect(); @@ -437,14 +426,18 @@ mod tests { } #[test] - fn render_approval_overlay_visible() { + fn render_approval_inline_visible() { let backend = TestBackend::new(80, 24); let mut terminal = Terminal::new(backend).unwrap(); let mut state = AppState::new(); + // Inline approval message in the message stream. + state.messages.push(DisplayMessage { + role: Role::Assistant, + content: "$ cargo test\n[y] approve [n] deny".to_string(), + tool_use_id: Some("t1".to_string()), + }); state.pending_approval = Some(super::super::events::PendingApproval { tool_use_id: "t1".to_string(), - tool_name: "write_file".to_string(), - input_summary: "path: foo.txt".to_string(), }); terminal.draw(|frame| render(frame, &state)).unwrap(); let buf = terminal.backend().buffer().clone(); @@ -454,12 +447,12 @@ mod tests { .map(|c| c.symbol().to_string()) .collect(); assert!( - all_text.contains("Tool Approval"), - "expected 'Tool Approval' overlay" + all_text.contains("approve"), + "expected approval prompt in buffer" ); assert!( - all_text.contains("write_file"), - "expected tool name in overlay" + all_text.contains("cargo test"), + "expected tool info in buffer" ); } @@ -467,7 +460,7 @@ mod tests { fn render_status_bar_shows_net_off() { let backend = TestBackend::new(80, 24); let mut terminal = Terminal::new(backend).unwrap(); - let state = AppState::new(); // network_allowed defaults to false + let state = AppState::new(); terminal.draw(|frame| render(frame, &state)).unwrap(); let buf = terminal.backend().buffer().clone(); let all_text: String = buf diff --git a/src/tui/tool_display.rs b/src/tui/tool_display.rs new file mode 100644 index 0000000..39af479 --- /dev/null +++ b/src/tui/tool_display.rs @@ -0,0 +1,133 @@ +//! Formatting functions for tool-specific display in the TUI. +//! +//! Each tool type has a structured [`ToolDisplay`] variant carrying the data +//! needed for rich rendering. These functions turn that data into the +//! user-facing strings shown in the conversation pane. + +use similar::{ChangeTag, TextDiff}; + +use crate::core::types::ToolDisplay; + +/// Format a tool that is currently executing (or awaiting approval). +pub fn format_executing(name: &str, display: &ToolDisplay) -> String { + match display { + ToolDisplay::WriteFile { + path, + old_content, + new_content, + } => { + let mut out = format!("write {path}\n"); + out.push_str(&unified_diff( + old_content.as_deref().unwrap_or(""), + new_content, + )); + out + } + ToolDisplay::ShellExec { command } => format!("$ {command}"), + ToolDisplay::ListDirectory { path } => format!("ls {path}"), + ToolDisplay::ReadFile { path } => format!("read {path}"), + ToolDisplay::Generic { summary } => format!("[{name}] {summary}"), + } +} + +/// Format a tool result after execution completes. +pub fn format_result(name: &str, display: &ToolDisplay, is_error: bool) -> String { + let prefix = if is_error { "error" } else { "result" }; + match display { + ToolDisplay::WriteFile { + path, + old_content, + new_content, + } => { + let mut out = format!("write {path}\n"); + out.push_str(&unified_diff( + old_content.as_deref().unwrap_or(""), + new_content, + )); + out.push_str(&format!("\nWrote {} bytes", new_content.len())); + out + } + ToolDisplay::ShellExec { command } => { + // For results, the command field carries "command\nstdout\nstderr". + format!("$ {command}") + } + ToolDisplay::ListDirectory { path } => format!("ls {path}"), + ToolDisplay::ReadFile { path } => format!("read {path}"), + ToolDisplay::Generic { summary } => format!("[{name} {prefix}] {summary}"), + } +} + +/// Produce a unified diff between `old` and `new` text. +/// +/// Uses the `similar` crate to compute line-level changes and formats them +/// with the conventional `+`/`-`/` ` prefix markers. +fn unified_diff(old: &str, new: &str) -> String { + let diff = TextDiff::from_lines(old, new); + let mut out = String::new(); + for change in diff.iter_all_changes() { + let marker = match change.tag() { + ChangeTag::Delete => "-", + ChangeTag::Insert => "+", + ChangeTag::Equal => " ", + }; + out.push_str(marker); + out.push_str(change.as_str().unwrap_or("")); + if !change.as_str().unwrap_or("").ends_with('\n') { + out.push('\n'); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_executing_shell() { + let display = ToolDisplay::ShellExec { + command: "cargo test".to_string(), + }; + assert_eq!(format_executing("shell_exec", &display), "$ cargo test"); + } + + #[test] + fn format_executing_write_with_diff() { + let display = ToolDisplay::WriteFile { + path: "src/lib.rs".to_string(), + old_content: Some("fn hello() {\n println!(\"hello\");\n}\n".to_string()), + new_content: "fn hello() {\n println!(\"hello world\");\n}\n".to_string(), + }; + let output = format_executing("write_file", &display); + assert!(output.starts_with("write src/lib.rs\n")); + assert!(output.contains("- println!(\"hello\");")); + assert!(output.contains("+ println!(\"hello world\");")); + } + + #[test] + fn format_result_write_shows_byte_count() { + let display = ToolDisplay::WriteFile { + path: "foo.txt".to_string(), + old_content: None, + new_content: "hello".to_string(), + }; + let output = format_result("write_file", &display, false); + assert!(output.contains("Wrote 5 bytes")); + } + + #[test] + fn format_result_generic_error() { + let display = ToolDisplay::Generic { + summary: "something failed".to_string(), + }; + let output = format_result("unknown_tool", &display, true); + assert_eq!(output, "[unknown_tool error] something failed"); + } + + #[test] + fn unified_diff_empty_old() { + let diff = unified_diff("", "line1\nline2\n"); + assert!(diff.contains("+line1")); + assert!(diff.contains("+line2")); + } +} -- 2.49.1