Skip to content

Commit a67a67f

Browse files
authored
codex-rs: make tool calls prettier (openai#1211)
This PR overhauls how active tool calls and completed tool calls are displayed: 1. More use of colour to indicate success/failure and distinguish between components like tool name+arguments 2. Previously, the entire `CallToolResult` was serialized to JSON and pretty-printed. Now, we extract each individual `CallToolResultContent` and print those 1. The previous solution was wasting space by unnecessarily showing details of the `CallToolResult` struct to users, without formatting the actual tool call results nicely 2. We're now able to show users more information from tool results in less space, with nicer formatting when tools return JSON results ### Before: <img width="1251" alt="Screenshot 2025-06-03 at 11 24 26" src="https://github.com/user-attachments/assets/5a58f222-219c-4c53-ace7-d887194e30cf" /> ### After: <img width="1265" alt="image" src="https://github.com/user-attachments/assets/99fe54d0-9ebe-406a-855b-7aa529b91274" /> ## Future Work 1. Integrate image tool result handling better. We should be able to display images even if they're not the first `CallToolResultContent` 2. Users should have some way to view the full version of truncated tool results 3. It would be nice to add some left padding for tool results, make it more clear that they are results. This is doable, just a little fiddly due to the way `first_visible_line` scrolling works 4. There's almost certainly a better way to format JSON than "all on 1 line with spaces to make Ratatui wrapping work". But I think that works OK for now.
1 parent c6fcec5 commit a67a67f

File tree

7 files changed

+352
-50
lines changed

7 files changed

+352
-50
lines changed

codex-rs/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/mcp-types/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,7 @@ pub enum ServerRequest {
11441144

11451145
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
11461146
#[serde(untagged)]
1147+
#[allow(clippy::large_enum_variant)]
11471148
pub enum ServerResult {
11481149
Result(Result),
11491150
InitializeResult(InitializeResult),

codex-rs/tui/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ ratatui = { version = "0.29.0", features = [
3434
] }
3535
ratatui-image = "8.0.0"
3636
regex-lite = "0.1"
37-
serde_json = "1"
37+
serde_json = { version = "1", features = ["preserve_order"] }
3838
shlex = "1.3.0"
3939
strum = "0.27.1"
4040
strum_macros = "0.27.1"
@@ -51,6 +51,7 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
5151
tui-input = "0.11.1"
5252
tui-markdown = "0.3.3"
5353
tui-textarea = "0.7.0"
54+
unicode-segmentation = "1.12.0"
5455
uuid = "1"
5556

5657
[dev-dependencies]

codex-rs/tui/src/conversation_history_widget.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,14 @@ impl ConversationHistoryWidget {
299299
for entry in self.entries.iter_mut() {
300300
if let HistoryCell::ActiveMcpToolCall {
301301
call_id: history_id,
302-
fq_tool_name,
303302
invocation,
304303
start,
305304
..
306305
} = &entry.cell
307306
{
308307
if &call_id == history_id {
309308
let completed = HistoryCell::new_completed_mcp_tool_call(
310-
fq_tool_name.clone(),
309+
width,
311310
invocation.clone(),
312311
*start,
313312
success,

codex-rs/tui/src/history_cell.rs

Lines changed: 78 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::cell_widget::CellWidget;
22
use crate::exec_command::escape_command;
33
use crate::markdown::append_markdown;
44
use crate::text_block::TextBlock;
5+
use crate::text_formatting::format_and_truncate_tool_result;
56
use base64::Engine;
67
use codex_ansi_escape::ansi_escape_line;
78
use codex_common::elapsed::format_duration;
@@ -14,6 +15,7 @@ use image::DynamicImage;
1415
use image::GenericImageView;
1516
use image::ImageReader;
1617
use lazy_static::lazy_static;
18+
use mcp_types::EmbeddedResourceResource;
1719
use ratatui::prelude::*;
1820
use ratatui::style::Color;
1921
use ratatui::style::Modifier;
@@ -73,18 +75,14 @@ pub(crate) enum HistoryCell {
7375
/// An MCP tool call that has not finished yet.
7476
ActiveMcpToolCall {
7577
call_id: String,
76-
/// `server.tool` fully-qualified name so we can show a concise label
77-
fq_tool_name: String,
78-
/// Formatted invocation that mirrors the `$ cmd ...` style of exec
79-
/// commands. We keep this around so the completed state can reuse the
80-
/// exact same text without re-formatting.
81-
invocation: String,
78+
/// Formatted line that shows the command name and arguments
79+
invocation: Line<'static>,
8280
start: Instant,
8381
view: TextBlock,
8482
},
8583

8684
/// Completed MCP tool call where we show the result serialized as JSON.
87-
CompletedMcpToolCallWithTextOutput { view: TextBlock },
85+
CompletedMcpToolCall { view: TextBlock },
8886

8987
/// Completed MCP tool call where the result is an image.
9088
/// Admittedly, [mcp_types::CallToolResult] can have multiple content types,
@@ -289,8 +287,6 @@ impl HistoryCell {
289287
tool: String,
290288
arguments: Option<serde_json::Value>,
291289
) -> Self {
292-
let fq_tool_name = format!("{server}.{tool}");
293-
294290
// Format the arguments as compact JSON so they roughly fit on one
295291
// line. If there are no arguments we keep it empty so the invocation
296292
// mirrors a function-style call.
@@ -302,29 +298,30 @@ impl HistoryCell {
302298
})
303299
.unwrap_or_default();
304300

305-
let invocation = if args_str.is_empty() {
306-
format!("{fq_tool_name}()")
307-
} else {
308-
format!("{fq_tool_name}({args_str})")
309-
};
301+
let invocation_spans = vec![
302+
Span::styled(server, Style::default().fg(Color::Blue)),
303+
Span::raw("."),
304+
Span::styled(tool, Style::default().fg(Color::Blue)),
305+
Span::raw("("),
306+
Span::styled(args_str, Style::default().fg(Color::Gray)),
307+
Span::raw(")"),
308+
];
309+
let invocation = Line::from(invocation_spans);
310310

311311
let start = Instant::now();
312312
let title_line = Line::from(vec!["tool".magenta(), " running...".dim()]);
313-
let lines: Vec<Line<'static>> = vec![
314-
title_line,
315-
Line::from(format!("$ {invocation}")),
316-
Line::from(""),
317-
];
313+
let lines: Vec<Line<'static>> = vec![title_line, invocation.clone(), Line::from("")];
318314

319315
HistoryCell::ActiveMcpToolCall {
320316
call_id,
321-
fq_tool_name,
322317
invocation,
323318
start,
324319
view: TextBlock::new(lines),
325320
}
326321
}
327322

323+
/// If the first content is an image, return a new cell with the image.
324+
/// TODO(rgwood-dd): Handle images properly even if they're not the first result.
328325
fn try_new_completed_mcp_tool_call_with_image_output(
329326
result: &Result<mcp_types::CallToolResult, String>,
330327
) -> Option<Self> {
@@ -370,8 +367,8 @@ impl HistoryCell {
370367
}
371368

372369
pub(crate) fn new_completed_mcp_tool_call(
373-
fq_tool_name: String,
374-
invocation: String,
370+
num_cols: u16,
371+
invocation: Line<'static>,
375372
start: Instant,
376373
success: bool,
377374
result: Result<mcp_types::CallToolResult, String>,
@@ -384,36 +381,70 @@ impl HistoryCell {
384381
let status_str = if success { "success" } else { "failed" };
385382
let title_line = Line::from(vec![
386383
"tool".magenta(),
387-
format!(" {fq_tool_name} ({status_str}, duration: {})", duration).dim(),
384+
" ".into(),
385+
if success {
386+
status_str.green()
387+
} else {
388+
status_str.red()
389+
},
390+
format!(", duration: {duration}").gray(),
388391
]);
389392

390393
let mut lines: Vec<Line<'static>> = Vec::new();
391394
lines.push(title_line);
392-
lines.push(Line::from(format!("$ {invocation}")));
393-
394-
// Convert result into serde_json::Value early so we don't have to
395-
// worry about lifetimes inside the match arm.
396-
let result_val = result.map(|r| {
397-
serde_json::to_value(r)
398-
.unwrap_or_else(|_| serde_json::Value::String("<serialization error>".into()))
399-
});
400-
401-
if let Ok(res_val) = result_val {
402-
let json_pretty =
403-
serde_json::to_string_pretty(&res_val).unwrap_or_else(|_| res_val.to_string());
404-
let mut iter = json_pretty.lines();
405-
for raw in iter.by_ref().take(TOOL_CALL_MAX_LINES) {
406-
lines.push(Line::from(raw.to_string()).dim());
395+
lines.push(invocation);
396+
397+
match result {
398+
Ok(mcp_types::CallToolResult { content, .. }) => {
399+
if !content.is_empty() {
400+
lines.push(Line::from(""));
401+
402+
for tool_call_result in content {
403+
let line_text = match tool_call_result {
404+
mcp_types::CallToolResultContent::TextContent(text) => {
405+
format_and_truncate_tool_result(
406+
&text.text,
407+
TOOL_CALL_MAX_LINES,
408+
num_cols as usize,
409+
)
410+
}
411+
mcp_types::CallToolResultContent::ImageContent(_) => {
412+
// TODO show images even if they're not the first result, will require a refactor of `CompletedMcpToolCall`
413+
"<image content>".to_string()
414+
}
415+
mcp_types::CallToolResultContent::AudioContent(_) => {
416+
"<audio content>".to_string()
417+
}
418+
mcp_types::CallToolResultContent::EmbeddedResource(resource) => {
419+
let uri = match resource.resource {
420+
EmbeddedResourceResource::TextResourceContents(text) => {
421+
text.uri
422+
}
423+
EmbeddedResourceResource::BlobResourceContents(blob) => {
424+
blob.uri
425+
}
426+
};
427+
format!("embedded resource: {uri}")
428+
}
429+
};
430+
lines.push(Line::styled(line_text, Style::default().fg(Color::Gray)));
431+
}
432+
}
433+
434+
lines.push(Line::from(""));
407435
}
408-
let remaining = iter.count();
409-
if remaining > 0 {
410-
lines.push(Line::from(format!("... {} additional lines", remaining)).dim());
436+
Err(e) => {
437+
lines.push(Line::from(vec![
438+
Span::styled(
439+
"Error: ",
440+
Style::default().fg(Color::Red).add_modifier(Modifier::BOLD),
441+
),
442+
Span::raw(e),
443+
]));
411444
}
412-
}
413-
414-
lines.push(Line::from(""));
445+
};
415446

416-
HistoryCell::CompletedMcpToolCallWithTextOutput {
447+
HistoryCell::CompletedMcpToolCall {
417448
view: TextBlock::new(lines),
418449
}
419450
}
@@ -520,7 +551,7 @@ impl CellWidget for HistoryCell {
520551
| HistoryCell::ErrorEvent { view }
521552
| HistoryCell::SessionInfo { view }
522553
| HistoryCell::CompletedExecCommand { view }
523-
| HistoryCell::CompletedMcpToolCallWithTextOutput { view }
554+
| HistoryCell::CompletedMcpToolCall { view }
524555
| HistoryCell::PendingPatch { view }
525556
| HistoryCell::ActiveExecCommand { view, .. }
526557
| HistoryCell::ActiveMcpToolCall { view, .. } => view.height(width),
@@ -541,7 +572,7 @@ impl CellWidget for HistoryCell {
541572
| HistoryCell::ErrorEvent { view }
542573
| HistoryCell::SessionInfo { view }
543574
| HistoryCell::CompletedExecCommand { view }
544-
| HistoryCell::CompletedMcpToolCallWithTextOutput { view }
575+
| HistoryCell::CompletedMcpToolCall { view }
545576
| HistoryCell::PendingPatch { view }
546577
| HistoryCell::ActiveExecCommand { view, .. }
547578
| HistoryCell::ActiveMcpToolCall { view, .. } => {

codex-rs/tui/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ mod scroll_event_helper;
3434
mod slash_command;
3535
mod status_indicator_widget;
3636
mod text_block;
37+
mod text_formatting;
3738
mod tui;
3839
mod user_approval_widget;
3940

0 commit comments

Comments
 (0)