From b82f5a0c96ecbe0914f809d3645c5f1ba80c8782 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Jan 2026 05:00:35 +0000 Subject: [PATCH 1/2] Add render tests for all tool blocks Add comprehensive render tests for each tool block type to enable safe refactoring of shared rendering code: - Add lines_to_string() helper in transcript.rs for testing - Add render tests for ReadFileBlock (pending, running, complete, denied, bg, error) - Add render tests for WriteFileBlock - Add render tests for EditFileBlock (including single/multiple edits) - Add render tests for ShellBlock (including working_dir display) - Add render tests for FetchUrlBlock - Add render tests for FetchHtmlBlock - Add render tests for WebSearchBlock - Add render tests for OpenFileBlock (including line number display) - Add render tests for SpawnAgentBlock (including long task truncation) - Add render tests for ListBackgroundTasksBlock - Add render tests for GetBackgroundTaskBlock Each block type is tested for all status states (Pending, Running, Complete, Error, Denied) and background mode rendering. These tests capture the current rendering format to enable refactoring to shared rendering utilities. --- src/tools/impls/background_tasks.rs | 157 ++++++++++++++++++++++++++++ src/tools/impls/edit_file.rs | 123 ++++++++++++++++++++++ src/tools/impls/fetch_html.rs | 88 ++++++++++++++++ src/tools/impls/fetch_url.rs | 87 +++++++++++++++ src/tools/impls/open_file.rs | 99 ++++++++++++++++++ src/tools/impls/read_file.rs | 82 +++++++++++++++ src/tools/impls/shell.rs | 78 ++++++++++++++ src/tools/impls/spawn_agent.rs | 102 ++++++++++++++++++ src/tools/impls/web_search.rs | 90 ++++++++++++++++ src/tools/impls/write_file.rs | 87 +++++++++++++++ src/transcript.rs | 25 +++++ 11 files changed, 1018 insertions(+) diff --git a/src/tools/impls/background_tasks.rs b/src/tools/impls/background_tasks.rs index 219e8ba..b62e3b1 100644 --- a/src/tools/impls/background_tasks.rs +++ b/src/tools/impls/background_tasks.rs @@ -275,3 +275,160 @@ impl Tool for GetBackgroundTaskTool { Box::new(GetBackgroundTaskBlock::new(call_id, self.name(), params, background)) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::transcript::lines_to_string; + use serde_json::json; + + // ========================================================================= + // ListBackgroundTasksBlock render tests + // ========================================================================= + + #[test] + fn test_list_render_pending() { + let block = ListBackgroundTasksBlock::new( + "call_1", + "mcp_list_background_tasks", + json!({}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? list_background_tasks()\n [y]es [n]o"); + } + + #[test] + fn test_list_render_running() { + let mut block = ListBackgroundTasksBlock::new( + "call_1", + "mcp_list_background_tasks", + json!({}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ list_background_tasks()"); + } + + #[test] + fn test_list_render_complete_with_output() { + let mut block = ListBackgroundTasksBlock::new( + "call_1", + "mcp_list_background_tasks", + json!({}), + false, + ); + block.status = Status::Complete; + block.text = "task_1: shell (Complete)\ntask_2: read_file (Running)".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ list_background_tasks()\n task_1: shell (Complete)\n task_2: read_file (Running)"); + } + + #[test] + fn test_list_render_denied() { + let mut block = ListBackgroundTasksBlock::new( + "call_1", + "mcp_list_background_tasks", + json!({}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ list_background_tasks()\n Denied by user"); + } + + #[test] + fn test_list_render_background() { + let block = ListBackgroundTasksBlock::new( + "call_1", + "mcp_list_background_tasks", + json!({}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] list_background_tasks()\n [y]es [n]o"); + } + + // ========================================================================= + // GetBackgroundTaskBlock render tests + // ========================================================================= + + #[test] + fn test_get_render_pending() { + let block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? get_background_task(task_123)\n [y]es [n]o"); + } + + #[test] + fn test_get_render_running() { + let mut block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ get_background_task(task_123)"); + } + + #[test] + fn test_get_render_complete_with_output() { + let mut block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + false, + ); + block.status = Status::Complete; + block.text = "file1.txt\nfile2.txt".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ get_background_task(task_123)\n file1.txt\n file2.txt"); + } + + #[test] + fn test_get_render_denied() { + let mut block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ get_background_task(task_123)\n Denied by user"); + } + + #[test] + fn test_get_render_background() { + let block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] get_background_task(task_123)\n [y]es [n]o"); + } + + #[test] + fn test_get_render_error() { + let mut block = GetBackgroundTaskBlock::new( + "call_1", + "mcp_get_background_task", + json!({"task_id": "task_123"}), + false, + ); + block.status = Status::Error; + block.text = "Task not found".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ get_background_task(task_123)\n Task not found"); + } +} diff --git a/src/tools/impls/edit_file.rs b/src/tools/impls/edit_file.rs index 69dba46..51e80e2 100644 --- a/src/tools/impls/edit_file.rs +++ b/src/tools/impls/edit_file.rs @@ -307,6 +307,129 @@ mod tests { use super::*; use crate::tools::{ToolCall, ToolDecision, ToolEvent, ToolExecutor, ToolRegistry}; + use crate::transcript::lines_to_string; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending_single_edit() { + let block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? edit_file(/src/main.rs, 1 edit)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_multiple_edits() { + let block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [ + {"old_string": "foo", "new_string": "bar"}, + {"old_string": "baz", "new_string": "qux"} + ] + }), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? edit_file(/src/main.rs, 2 edits)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ edit_file(/src/main.rs, 1 edit)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + false, + ); + block.status = Status::Complete; + block.text = "Successfully applied 1 edit(s)".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ edit_file(/src/main.rs, 1 edit)\n Successfully applied 1 edit(s)"); + } + + #[test] + fn test_render_denied() { + let mut block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ edit_file(/src/main.rs, 1 edit)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] edit_file(/src/main.rs, 1 edit)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = EditFileBlock::new( + "call_1", + "mcp_edit_file", + json!({ + "path": "/src/main.rs", + "edits": [{"old_string": "foo", "new_string": "bar"}] + }), + false, + ); + block.status = Status::Error; + block.text = "old_string not found".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ edit_file(/src/main.rs, 1 edit)\n old_string not found"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= /// Helper to run a tool to completion, auto-responding to Delegate events async fn run_to_completion(executor: &mut ToolExecutor) -> ToolEvent { diff --git a/src/tools/impls/fetch_html.rs b/src/tools/impls/fetch_html.rs index e2b4986..3837f84 100644 --- a/src/tools/impls/fetch_html.rs +++ b/src/tools/impls/fetch_html.rs @@ -172,6 +172,94 @@ impl Tool for FetchHtmlTool { mod tests { use super::*; use crate::tools::{ToolCall, ToolDecision, ToolExecutor, ToolRegistry}; + use crate::transcript::lines_to_string; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? fetch_html(https://example.com)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ fetch_html(https://example.com)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + false, + ); + block.status = Status::Complete; + // Use text without blank lines to avoid render_result preserving them + block.text = "# Example Domain\nThis is an example.".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ fetch_html(https://example.com)\n # Example Domain\n This is an example."); + } + + #[test] + fn test_render_denied() { + let mut block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ fetch_html(https://example.com)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] fetch_html(https://example.com)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = FetchHtmlBlock::new( + "call_1", + "mcp_fetch_html", + json!({"url": "https://example.com"}), + false, + ); + block.status = Status::Error; + block.text = "Page not found".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ fetch_html(https://example.com)\n Page not found"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= #[tokio::test] async fn test_fetch_html_invalid_url() { diff --git a/src/tools/impls/fetch_url.rs b/src/tools/impls/fetch_url.rs index 854178f..7a5aa44 100644 --- a/src/tools/impls/fetch_url.rs +++ b/src/tools/impls/fetch_url.rs @@ -162,6 +162,93 @@ impl Tool for FetchUrlTool { mod tests { use super::*; use crate::tools::{ToolExecutor, ToolRegistry, ToolCall, ToolDecision}; + use crate::transcript::lines_to_string; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? fetch_url(https://example.com/api)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ fetch_url(https://example.com/api)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + false, + ); + block.status = Status::Complete; + block.text = "{\"status\": \"ok\"}".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ fetch_url(https://example.com/api)\n {\"status\": \"ok\"}"); + } + + #[test] + fn test_render_denied() { + let mut block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ fetch_url(https://example.com/api)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] fetch_url(https://example.com/api)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = FetchUrlBlock::new( + "call_1", + "mcp_fetch_url", + json!({"url": "https://example.com/api"}), + false, + ); + block.status = Status::Error; + block.text = "Connection refused".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ fetch_url(https://example.com/api)\n Connection refused"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= #[tokio::test] async fn test_fetch_invalid_url() { diff --git a/src/tools/impls/open_file.rs b/src/tools/impls/open_file.rs index 3ea32c0..536856a 100644 --- a/src/tools/impls/open_file.rs +++ b/src/tools/impls/open_file.rs @@ -157,3 +157,102 @@ impl Block for OpenFileBlock { Some(&self.params) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::transcript::lines_to_string; + use serde_json::json; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? open_file(/src/main.rs)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_with_line() { + let block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs", "line": 42}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? open_file(/src/main.rs:42)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ open_file(/src/main.rs)"); + } + + #[test] + fn test_render_complete() { + let mut block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs", "line": 42}), + false, + ); + block.status = Status::Complete; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ open_file(/src/main.rs:42)"); + } + + #[test] + fn test_render_denied() { + let mut block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ open_file(/src/main.rs)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] open_file(/src/main.rs)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = OpenFileBlock::new( + "call_1", + "mcp_open_file", + json!({"path": "/src/main.rs"}), + false, + ); + block.status = Status::Error; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ open_file(/src/main.rs)"); + } +} diff --git a/src/tools/impls/read_file.rs b/src/tools/impls/read_file.rs index a915d76..8a60b3c 100644 --- a/src/tools/impls/read_file.rs +++ b/src/tools/impls/read_file.rs @@ -182,9 +182,91 @@ impl Tool for ReadFileTool { mod tests { use super::*; use crate::tools::{ToolExecutor, ToolRegistry, ToolCall, ToolDecision}; + use crate::transcript::lines_to_string; use std::fs; use tempfile::tempdir; + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), false); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? read_file(/src/main.rs)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_with_range() { + let block = ReadFileBlock::new( + "call_1", + "mcp_read_file", + json!({"path": "/src/main.rs", "start_line": 10, "end_line": 20}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? read_file(/src/main.rs:10:20)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_with_start_only() { + let block = ReadFileBlock::new( + "call_1", + "mcp_read_file", + json!({"path": "/src/main.rs", "start_line": 10}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? read_file(/src/main.rs:10:)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), false); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ read_file(/src/main.rs)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), false); + block.status = Status::Complete; + block.text = " 1│fn main() {\n 2│ println!(\"Hello\");\n 3│}".to_string(); + let output = lines_to_string(&block.render(80)); + // render_result adds " " prefix to each line + assert_eq!(output, "✓ read_file(/src/main.rs)\n 1│fn main() {\n 2│ println!(\"Hello\");\n 3│}"); + } + + #[test] + fn test_render_denied() { + let mut block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), false); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ read_file(/src/main.rs)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), true); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] read_file(/src/main.rs)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = ReadFileBlock::new("call_1", "mcp_read_file", json!({"path": "/src/main.rs"}), false); + block.status = Status::Error; + block.text = "File not found".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ read_file(/src/main.rs)\n File not found"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= + #[tokio::test] async fn test_read_file() { let dir = tempdir().unwrap(); diff --git a/src/tools/impls/shell.rs b/src/tools/impls/shell.rs index c8dc6a3..ec83e4d 100644 --- a/src/tools/impls/shell.rs +++ b/src/tools/impls/shell.rs @@ -187,6 +187,84 @@ impl Tool for ShellTool { mod tests { use super::*; use crate::tools::{ToolExecutor, ToolRegistry, ToolCall, ToolDecision}; + use crate::transcript::lines_to_string; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "ls -la"}), false); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? shell(ls -la)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_with_working_dir() { + let block = ShellBlock::new( + "call_1", + "mcp_shell", + json!({"command": "ls -la", "working_dir": "/home/user"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? shell(ls -la, in /home/user)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "cargo build"}), false); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ shell(cargo build)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "echo hello"}), false); + block.status = Status::Complete; + block.text = "hello".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ shell(echo hello)\n hello"); + } + + #[test] + fn test_render_complete_with_multiline_output() { + let mut block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "ls"}), false); + block.status = Status::Complete; + block.text = "file1.txt\nfile2.txt\nfile3.txt".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ shell(ls)\n file1.txt\n file2.txt\n file3.txt"); + } + + #[test] + fn test_render_denied() { + let mut block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "rm -rf /"}), false); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ shell(rm -rf /)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "make build"}), true); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] shell(make build)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = ShellBlock::new("call_1", "mcp_shell", json!({"command": "invalid_cmd"}), false); + block.status = Status::Error; + block.text = "Command not found".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ shell(invalid_cmd)\n Command not found"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= #[tokio::test] async fn test_shell_echo() { diff --git a/src/tools/impls/spawn_agent.rs b/src/tools/impls/spawn_agent.rs index ac0795b..d74504a 100644 --- a/src/tools/impls/spawn_agent.rs +++ b/src/tools/impls/spawn_agent.rs @@ -266,3 +266,105 @@ impl EffectHandler for RunAgent { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::transcript::lines_to_string; + use serde_json::json; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? spawn_agent(Find all TODO comments)\n [y]es [n]o"); + } + + #[test] + fn test_render_pending_long_task() { + let block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "This is a very long task description that should be truncated after sixty characters"}), + false, + ); + let output = lines_to_string(&block.render(80)); + // Truncation happens at 60 chars (57 chars + "...") + assert_eq!(output, "? spawn_agent(This is a very long task description that should be trunc...)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ spawn_agent(Find all TODO comments)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + false, + ); + block.status = Status::Complete; + block.text = "Found 5 TODO comments".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ spawn_agent(Find all TODO comments)\n Found 5 TODO comments"); + } + + #[test] + fn test_render_denied() { + let mut block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ spawn_agent(Find all TODO comments)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] spawn_agent(Find all TODO comments)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = SpawnAgentBlock::new( + "call_1", + "mcp_spawn_agent", + json!({"task": "Find all TODO comments"}), + false, + ); + block.status = Status::Error; + block.text = "Agent context not initialized".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ spawn_agent(Find all TODO comments)\n Agent context not initialized"); + } +} diff --git a/src/tools/impls/web_search.rs b/src/tools/impls/web_search.rs index ee571c2..a15c9b2 100644 --- a/src/tools/impls/web_search.rs +++ b/src/tools/impls/web_search.rs @@ -161,3 +161,93 @@ impl Tool for WebSearchTool { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::transcript::lines_to_string; + + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? web_search(\"rust async await\")\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ web_search(\"rust async await\")"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + false, + ); + block.status = Status::Complete; + block.text = "Result 1\nResult 2\nResult 3".to_string(); + let output = lines_to_string(&block.render(80)); + // WebSearchBlock shows "N results." instead of the actual results + assert_eq!(output, "✓ web_search(\"rust async await\")\n 3 results."); + } + + #[test] + fn test_render_denied() { + let mut block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ web_search(\"rust async await\")\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] web_search(\"rust async await\")\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = WebSearchBlock::new( + "call_1", + "mcp_web_search", + json!({"query": "rust async await"}), + false, + ); + block.status = Status::Error; + block.text = "API key not configured".to_string(); + let output = lines_to_string(&block.render(80)); + // WebSearchBlock uses render_result which shows "N results." for any text + assert_eq!(output, "✗ web_search(\"rust async await\")\n 1 results."); + } +} diff --git a/src/tools/impls/write_file.rs b/src/tools/impls/write_file.rs index 31cfb18..b46b170 100644 --- a/src/tools/impls/write_file.rs +++ b/src/tools/impls/write_file.rs @@ -200,9 +200,96 @@ impl Tool for WriteFileTool { mod tests { use super::*; use crate::tools::{ToolExecutor, ToolRegistry, ToolCall, ToolDecision, ToolEvent}; + use crate::transcript::lines_to_string; use std::fs; use tempfile::tempdir; + // ========================================================================= + // Render tests + // ========================================================================= + + #[test] + fn test_render_pending() { + let block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + false, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? write_file(/src/new_file.rs, 12 bytes)\n [y]es [n]o"); + } + + #[test] + fn test_render_running() { + let mut block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + false, + ); + block.status = Status::Running; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⚙ write_file(/src/new_file.rs, 12 bytes)"); + } + + #[test] + fn test_render_complete_with_output() { + let mut block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + false, + ); + block.status = Status::Complete; + block.text = "Created file: /src/new_file.rs".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✓ write_file(/src/new_file.rs, 12 bytes)\n Created file: /src/new_file.rs"); + } + + #[test] + fn test_render_denied() { + let mut block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + false, + ); + block.status = Status::Denied; + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "⊘ write_file(/src/new_file.rs, 12 bytes)\n Denied by user"); + } + + #[test] + fn test_render_background() { + let block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + true, + ); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "? [bg] write_file(/src/new_file.rs, 12 bytes)\n [y]es [n]o"); + } + + #[test] + fn test_render_error() { + let mut block = WriteFileBlock::new( + "call_1", + "mcp_write_file", + json!({"path": "/src/new_file.rs", "content": "fn main() {}"}), + false, + ); + block.status = Status::Error; + block.text = "File already exists".to_string(); + let output = lines_to_string(&block.render(80)); + assert_eq!(output, "✗ write_file(/src/new_file.rs, 12 bytes)\n File already exists"); + } + + // ========================================================================= + // Execution tests + // ========================================================================= + /// Helper to run a tool to completion, auto-responding to Delegate events async fn run_to_completion(executor: &mut ToolExecutor) -> ToolEvent { loop { diff --git a/src/transcript.rs b/src/transcript.rs index 7602e82..23bfb68 100644 --- a/src/transcript.rs +++ b/src/transcript.rs @@ -688,10 +688,35 @@ impl Transcript { } } +/// Convert ratatui Lines to a plain string representation for testing. +/// Each Line becomes a string with spans concatenated, joined by newlines. +#[cfg(test)] +pub fn lines_to_string(lines: &[Line<'_>]) -> String { + lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_lines_to_string() { + let lines = vec![ + Line::from(vec![Span::raw("Hello "), Span::raw("World")]), + Line::from(vec![Span::raw("Line 2")]), + ]; + assert_eq!(lines_to_string(&lines), "Hello World\nLine 2"); + } + #[test] fn test_text_block_render() { let block = TextBlock::new("Hello\nWorld"); From c4737c29e8a9a3a4b03c9b572854eff12bb34d47 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Jan 2026 05:12:04 +0000 Subject: [PATCH 2/2] Refactor tool blocks to use shared render_tool_block() Extract common rendering logic into render_tool_block() in transcript.rs, eliminating repetitive code across all 11 tool block implementations. Each tool's render() method now only needs to compute its specific args (as styled Spans) and pass them to the shared function which handles: - Status icon rendering - Background prefix [bg] - Tool name and argument formatting - Approval prompt for pending status - Result text output - Denied message display Net reduction of ~221 lines of duplicated rendering code. --- src/tools/impls/background_tasks.rs | 68 ++------------------------- src/tools/impls/edit_file.rs | 42 +++-------------- src/tools/impls/fetch_html.rs | 34 ++------------ src/tools/impls/fetch_url.rs | 33 ++----------- src/tools/impls/open_file.rs | 27 ++--------- src/tools/impls/read_file.rs | 38 +++------------ src/tools/impls/shell.rs | 38 ++------------- src/tools/impls/spawn_agent.rs | 34 ++------------ src/tools/impls/web_search.rs | 40 ++++------------ src/tools/impls/write_file.rs | 34 ++------------ src/transcript.rs | 73 +++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 341 deletions(-) diff --git a/src/tools/impls/background_tasks.rs b/src/tools/impls/background_tasks.rs index b62e3b1..ed911a0 100644 --- a/src/tools/impls/background_tasks.rs +++ b/src/tools/impls/background_tasks.rs @@ -11,7 +11,7 @@ use serde_json::json; use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, Status}; // ============================================================================= // ListBackgroundTasks block @@ -47,36 +47,7 @@ impl Block for ListBackgroundTasksBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - - // Format: list_background_tasks() - let spans = vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("list_background_tasks", Style::default().fg(Color::Magenta)), - Span::styled("()", Style::default().fg(Color::DarkGray)), - ]; - lines.push(Line::from(spans)); - - // Approval prompt if pending - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - // Output if completed - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 10)); - } - - // Denied message - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + render_tool_block(self.status, self.background, "list_background_tasks", vec![], &self.text, 10) } fn call_id(&self) -> Option<&str> { @@ -126,40 +97,9 @@ impl Block for GetBackgroundTaskBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let task_id = self.params["task_id"].as_str().unwrap_or(""); - - // Format: get_background_task(task_id) - let spans = vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("get_background_task", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(task_id, Style::default().fg(Color::White)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ]; - lines.push(Line::from(spans)); - - // Approval prompt if pending - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - // Output if completed - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 10)); - } - - // Denied message - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(task_id.to_string(), Style::default().fg(Color::White))]; + render_tool_block(self.status, self.background, "get_background_task", args, &self.text, 10) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/edit_file.rs b/src/tools/impls/edit_file.rs index 51e80e2..0ae5f1c 100644 --- a/src/tools/impls/edit_file.rs +++ b/src/tools/impls/edit_file.rs @@ -28,9 +28,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::ide::Edit; use crate::impl_base_block; use crate::tools::pipeline::{EffectHandler, Step}; -use crate::transcript::{ - render_approval_prompt, render_prefix, render_result, Block, BlockType, Status, ToolBlock, -}; +use crate::transcript::{render_tool_block, Block, BlockType, Status, ToolBlock}; // ============================================================================= // Edit-specific validation handler @@ -116,8 +114,6 @@ impl Block for EditFileBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let path = self.params["path"].as_str().unwrap_or(""); let edit_count = self .params @@ -126,40 +122,14 @@ impl Block for EditFileBlock { .map(|a| a.len()) .unwrap_or(0); - // Format: edit_file(path, N edits) - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("edit_file", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(path, Style::default().fg(Color::Yellow)), + let args = vec![ + Span::styled(path.to_string(), Style::default().fg(Color::Yellow)), Span::styled( - format!( - ", {} edit{}", - edit_count, - if edit_count == 1 { "" } else { "s" } - ), + format!(", {} edit{}", edit_count, if edit_count == 1 { "" } else { "s" }), Style::default().fg(Color::DarkGray), ), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 5)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + ]; + render_tool_block(self.status, self.background, "edit_file", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/fetch_html.rs b/src/tools/impls/fetch_html.rs index 3837f84..35a7b38 100644 --- a/src/tools/impls/fetch_html.rs +++ b/src/tools/impls/fetch_html.rs @@ -12,9 +12,7 @@ use serde_json::json; use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{ - render_approval_prompt, render_prefix, render_result, Block, BlockType, Status, ToolBlock, -}; +use crate::transcript::{render_tool_block, Block, BlockType, Status, ToolBlock}; /// Fetch HTML display block #[derive(Debug, Clone, Serialize, Deserialize)] @@ -56,35 +54,9 @@ impl Block for FetchHtmlBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let url = self.params["url"].as_str().unwrap_or(""); - - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("fetch_html", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(url, Style::default().fg(Color::Blue)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 5)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(url.to_string(), Style::default().fg(Color::Blue))]; + render_tool_block(self.status, self.background, "fetch_html", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/fetch_url.rs b/src/tools/impls/fetch_url.rs index 7a5aa44..7c43c9a 100644 --- a/src/tools/impls/fetch_url.rs +++ b/src/tools/impls/fetch_url.rs @@ -2,7 +2,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, ToolBlock, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, ToolBlock, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -45,36 +45,9 @@ impl Block for FetchUrlBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let url = self.params["url"].as_str().unwrap_or(""); - - // Format: fetch_url(url) - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("fetch_url", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(url, Style::default().fg(Color::Blue)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 5)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(url.to_string(), Style::default().fg(Color::Blue))]; + render_tool_block(self.status, self.background, "fetch_url", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/open_file.rs b/src/tools/impls/open_file.rs index 536856a..a5a8ece 100644 --- a/src/tools/impls/open_file.rs +++ b/src/tools/impls/open_file.rs @@ -1,7 +1,7 @@ //! Open file tool - opens a file in the IDE at a specific line use super::{handlers, Tool, ToolPipeline}; -use crate::transcript::{render_approval_prompt, render_prefix, Block, BlockType, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -116,33 +116,12 @@ impl Block for OpenFileBlock { fn render(&self, _width: u16) -> Vec> { let path = self.params["path"].as_str().unwrap_or(""); let line = self.params.get("line").and_then(|v| v.as_u64()); - let location = match line { Some(l) => format!("{}:{}", path, l), None => path.to_string(), }; - - let mut lines = vec![Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("open_file", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(location, Style::default().fg(Color::Cyan)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])]; - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(location, Style::default().fg(Color::Cyan))]; + render_tool_block(self.status, self.background, "open_file", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/read_file.rs b/src/tools/impls/read_file.rs index 8a60b3c..1724098 100644 --- a/src/tools/impls/read_file.rs +++ b/src/tools/impls/read_file.rs @@ -2,7 +2,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, ToolBlock, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, ToolBlock, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -46,46 +46,22 @@ impl Block for ReadFileBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let path = self.params["path"].as_str().unwrap_or(""); let start_line = self.params.get("start_line").and_then(|v| v.as_i64()); let end_line = self.params.get("end_line").and_then(|v| v.as_i64()); - // Format: read_file(path:start-end) or read_file(path) - let range_str = match (start_line, end_line) { + let range_suffix = match (start_line, end_line) { (Some(s), Some(e)) => format!(":{}:{}", s, e), (Some(s), None) => format!(":{}:", s), (None, Some(e)) => format!(":{}", e), (None, None) => String::new(), }; - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("read_file", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(path, Style::default().fg(Color::Cyan)), - Span::styled(range_str, Style::default().fg(Color::DarkGray)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 10)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![ + Span::styled(path.to_string(), Style::default().fg(Color::Cyan)), + Span::styled(range_suffix, Style::default().fg(Color::DarkGray)), + ]; + render_tool_block(self.status, self.background, "read_file", args, &self.text, 10) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/shell.rs b/src/tools/impls/shell.rs index ec83e4d..dcf61ef 100644 --- a/src/tools/impls/shell.rs +++ b/src/tools/impls/shell.rs @@ -2,7 +2,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, ToolBlock, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, ToolBlock, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -46,44 +46,14 @@ impl Block for ShellBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let command = self.params["command"].as_str().unwrap_or(""); let working_dir = self.params.get("working_dir").and_then(|v| v.as_str()); - // Format: shell(command) or shell(command, in dir) - let mut spans = vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("shell", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(command, Style::default().fg(Color::White)), - ]; + let mut args = vec![Span::styled(command.to_string(), Style::default().fg(Color::White))]; if let Some(dir) = working_dir { - spans.push(Span::styled(format!(", in {}", dir), Style::default().fg(Color::DarkGray))); - } - spans.push(Span::styled(")", Style::default().fg(Color::DarkGray))); - lines.push(Line::from(spans)); - - // Approval prompt if pending - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - // Output if completed - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 10)); - } - - // Denied message - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); + args.push(Span::styled(format!(", in {}", dir), Style::default().fg(Color::DarkGray))); } - - lines + render_tool_block(self.status, self.background, "shell", args, &self.text, 10) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/spawn_agent.rs b/src/tools/impls/spawn_agent.rs index d74504a..1bce35a 100644 --- a/src/tools/impls/spawn_agent.rs +++ b/src/tools/impls/spawn_agent.rs @@ -11,7 +11,7 @@ use crate::impl_base_block; use crate::llm::{Agent, RequestMode}; use crate::llm::background::run_agent; use crate::tools::pipeline::{EffectHandler, Step}; -use crate::transcript::{render_approval_prompt, render_prefix, Block, BlockType, ToolBlock, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, ToolBlock, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -94,8 +94,6 @@ impl Block for SpawnAgentBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let task = self.params["task"].as_str().unwrap_or(""); // Truncate task for display let task_display = if task.len() > 60 { @@ -104,34 +102,8 @@ impl Block for SpawnAgentBlock { task.to_string() }; - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("spawn_agent", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(task_display, Style::default().fg(Color::Yellow)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.push(Line::from(Span::styled( - format!(" {}", self.text), - Style::default().fg(Color::DarkGray), - ))); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(task_display, Style::default().fg(Color::Yellow))]; + render_tool_block(self.status, self.background, "spawn_agent", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/web_search.rs b/src/tools/impls/web_search.rs index a15c9b2..eb7e683 100644 --- a/src/tools/impls/web_search.rs +++ b/src/tools/impls/web_search.rs @@ -2,7 +2,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, Status, ToolBlock}; +use crate::transcript::{render_tool_block, Block, BlockType, Status, ToolBlock}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -45,37 +45,15 @@ impl Block for WebSearchBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let query = self.params["query"].as_str().unwrap_or(""); - - // Format: web_search(query) - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("web_search", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(format!("\"{}\"", query), Style::default().fg(Color::Green)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result( - &format!("{} results.", self.text.split("\n").count()), 1)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + let args = vec![Span::styled(format!("\"{}\"", query), Style::default().fg(Color::Green))]; + // Show result count instead of full results + let result_text = if self.text.is_empty() { + String::new() + } else { + format!("{} results.", self.text.lines().count()) + }; + render_tool_block(self.status, self.background, "web_search", args, &result_text, 1) } fn call_id(&self) -> Option<&str> { diff --git a/src/tools/impls/write_file.rs b/src/tools/impls/write_file.rs index b46b170..6c3c8c4 100644 --- a/src/tools/impls/write_file.rs +++ b/src/tools/impls/write_file.rs @@ -14,7 +14,7 @@ use super::{handlers, Tool, ToolPipeline}; use crate::ide::ToolPreview; use crate::impl_base_block; -use crate::transcript::{render_approval_prompt, render_prefix, render_result, Block, BlockType, ToolBlock, Status}; +use crate::transcript::{render_tool_block, Block, BlockType, ToolBlock, Status}; use ratatui::{ style::{Color, Style}, text::{Line, Span}, @@ -58,38 +58,14 @@ impl Block for WriteFileBlock { impl_base_block!(BlockType::Tool); fn render(&self, _width: u16) -> Vec> { - let mut lines = Vec::new(); - let path = self.params["path"].as_str().unwrap_or(""); let content_len = self.params.get("content").and_then(|v| v.as_str()).map(|s| s.len()).unwrap_or(0); - // Format: write_file(path, N bytes) - lines.push(Line::from(vec![ - self.render_status(), - render_prefix(self.background), - Span::styled("write_file", Style::default().fg(Color::Magenta)), - Span::styled("(", Style::default().fg(Color::DarkGray)), - Span::styled(path, Style::default().fg(Color::Green)), + let args = vec![ + Span::styled(path.to_string(), Style::default().fg(Color::Green)), Span::styled(format!(", {} bytes", content_len), Style::default().fg(Color::DarkGray)), - Span::styled(")", Style::default().fg(Color::DarkGray)), - ])); - - if self.status == Status::Pending { - lines.push(render_approval_prompt()); - } - - if !self.text.is_empty() { - lines.extend(render_result(&self.text, 5)); - } - - if self.status == Status::Denied { - lines.push(Line::from(Span::styled( - " Denied by user", - Style::default().fg(Color::DarkGray), - ))); - } - - lines + ]; + render_tool_block(self.status, self.background, "write_file", args, &self.text, 5) } fn call_id(&self) -> Option<&str> { diff --git a/src/transcript.rs b/src/transcript.rs index 23bfb68..66ddb6c 100644 --- a/src/transcript.rs +++ b/src/transcript.rs @@ -368,6 +368,79 @@ pub fn render_result(result: &str, max_lines: usize) -> Vec> { lines } +/// Helper: render denied message +pub fn render_denied() -> Line<'static> { + Line::from(Span::styled( + " Denied by user", + Style::default().fg(Color::DarkGray), + )) +} + +/// Render a tool block with the standard layout: +/// - Header: status + [bg]? + name(args) +/// - Approval prompt if pending +/// - Result lines if text present +/// - Denied message if denied +/// +/// Arguments: +/// - `status`: Current block status +/// - `background`: Whether this is a background tool +/// - `name`: Display name (e.g., "read_file", "shell") +/// - `args`: Argument spans to display inside parentheses +/// - `text`: Result text to display +/// - `max_lines`: Maximum result lines to show +pub fn render_tool_block( + status: Status, + background: bool, + name: &str, + args: Vec>, + text: &str, + max_lines: usize, +) -> Vec> { + let mut lines = Vec::new(); + + // Build header: status + [bg]? + name + (args) + let mut header_spans = vec![ + render_status_icon(status), + render_prefix(background), + Span::styled(name.to_string(), Style::default().fg(Color::Magenta)), + Span::styled("(", Style::default().fg(Color::DarkGray)), + ]; + header_spans.extend(args); + header_spans.push(Span::styled(")", Style::default().fg(Color::DarkGray))); + lines.push(Line::from(header_spans)); + + // Approval prompt if pending + if status == Status::Pending { + lines.push(render_approval_prompt()); + } + + // Result lines if text present + if !text.is_empty() { + lines.extend(render_result(text, max_lines)); + } + + // Denied message if denied + if status == Status::Denied { + lines.push(render_denied()); + } + + lines +} + +/// Render status icon with appropriate color (standalone version for render_tool_block) +fn render_status_icon(status: Status) -> Span<'static> { + let (icon, color) = match status { + Status::Pending => ("? ", Color::Yellow), + Status::Running => ("⚙ ", Color::Blue), + Status::Complete => ("✓ ", Color::Green), + Status::Error => ("✗ ", Color::Red), + Status::Denied => ("⊘ ", Color::DarkGray), + Status::Cancelled => ("⊘ ", Color::Yellow), + }; + Span::styled(icon, Style::default().fg(color)) +} + /// A turn in the conversation - one user or assistant response #[derive(Serialize, Deserialize)] pub struct Turn {