From 3ab6028e80e362f0afb4e4ef948b8e40d2c5f993 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 23 Oct 2025 08:05:08 -0700 Subject: [PATCH] tui: show aggregated output in display (#5539) This shows the aggregated (stdout + stderr) buffer regardless of exit code. Many commands output useful / relevant info on stdout when returning a non-zero exit code, or the same on stderr when returning an exit code of 0. Often, useful info is present on both stdout AND stderr. Also, the model sees both. So it is confusing to see commands listed as "(no output)" that in fact do have output, just on the stream that doesn't match the exit status, or to see some sort of trivial output like "Tests failed" but lacking any information about the actual failure. As such, always display the aggregated output in the display. Transcript mode remains unchanged as it was already displaying the text that the model sees, which seems correct for transcript mode. --- codex-rs/tui/src/chatwidget.rs | 3 +- codex-rs/tui/src/chatwidget/tests.rs | 20 +++-- codex-rs/tui/src/exec_cell/model.rs | 10 +-- codex-rs/tui/src/exec_cell/render.rs | 16 +--- codex-rs/tui/src/history_cell.rs | 120 +++------------------------ codex-rs/tui/src/pager_overlay.rs | 3 +- 6 files changed, 36 insertions(+), 136 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c1a6dd67..c090d212 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -745,9 +745,8 @@ impl ChatWidget { &ev.call_id, CommandOutput { exit_code: ev.exit_code, - stdout: ev.stdout.clone(), - stderr: ev.stderr.clone(), formatted_output: ev.formatted_output.clone(), + aggregated_output: ev.aggregated_output.clone(), }, ev.duration, ); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 60f1ab04..738fc31e 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -71,18 +71,26 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json && let Some(m) = msg.as_object_mut() { let ty = m.get("type").and_then(|v| v.as_str()).unwrap_or(""); - if ty == "exec_command_end" && !m.contains_key("formatted_output") { + if ty == "exec_command_end" { let stdout = m.get("stdout").and_then(|v| v.as_str()).unwrap_or(""); let stderr = m.get("stderr").and_then(|v| v.as_str()).unwrap_or(""); - let formatted = if stderr.is_empty() { + let aggregated = if stderr.is_empty() { stdout.to_string() } else { format!("{stdout}{stderr}") }; - m.insert( - "formatted_output".to_string(), - serde_json::Value::String(formatted), - ); + if !m.contains_key("formatted_output") { + m.insert( + "formatted_output".to_string(), + serde_json::Value::String(aggregated.clone()), + ); + } + if !m.contains_key("aggregated_output") { + m.insert( + "aggregated_output".to_string(), + serde_json::Value::String(aggregated), + ); + } } } payload diff --git a/codex-rs/tui/src/exec_cell/model.rs b/codex-rs/tui/src/exec_cell/model.rs index 2893a005..cf9acc54 100644 --- a/codex-rs/tui/src/exec_cell/model.rs +++ b/codex-rs/tui/src/exec_cell/model.rs @@ -3,11 +3,12 @@ use std::time::Instant; use codex_protocol::parse_command::ParsedCommand; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub(crate) struct CommandOutput { pub(crate) exit_code: i32, - pub(crate) stdout: String, - pub(crate) stderr: String, + /// The aggregated stderr + stdout interleaved. + pub(crate) aggregated_output: String, + /// The formatted output of the command, as seen by the model. pub(crate) formatted_output: String, } @@ -82,9 +83,8 @@ impl ExecCell { call.duration = Some(elapsed); call.output = Some(CommandOutput { exit_code: 1, - stdout: String::new(), - stderr: String::new(), formatted_output: String::new(), + aggregated_output: String::new(), }); } } diff --git a/codex-rs/tui/src/exec_cell/render.rs b/codex-rs/tui/src/exec_cell/render.rs index 98d9d072..bc336b98 100644 --- a/codex-rs/tui/src/exec_cell/render.rs +++ b/codex-rs/tui/src/exec_cell/render.rs @@ -28,7 +28,6 @@ use unicode_width::UnicodeWidthStr; pub(crate) const TOOL_CALL_MAX_LINES: usize = 5; pub(crate) struct OutputLinesParams { - pub(crate) only_err: bool, pub(crate) include_angle_pipe: bool, pub(crate) include_prefix: bool, } @@ -59,22 +58,12 @@ pub(crate) fn output_lines( params: OutputLinesParams, ) -> OutputLines { let OutputLinesParams { - only_err, include_angle_pipe, include_prefix, } = params; let CommandOutput { - exit_code, - stdout, - stderr, - .. + aggregated_output, .. } = match output { - Some(output) if only_err && output.exit_code == 0 => { - return OutputLines { - lines: Vec::new(), - omitted: None, - }; - } Some(output) => output, None => { return OutputLines { @@ -84,7 +73,7 @@ pub(crate) fn output_lines( } }; - let src = if *exit_code == 0 { stdout } else { stderr }; + let src = aggregated_output; let lines: Vec<&str> = src.lines().collect(); let total = lines.len(); let limit = TOOL_CALL_MAX_LINES; @@ -398,7 +387,6 @@ impl ExecCell { let raw_output = output_lines( Some(output), OutputLinesParams { - only_err: false, include_angle_pipe: false, include_prefix: false, }, diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index fe85fb87..7228d39e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1293,12 +1293,10 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { let output = output_lines( Some(&CommandOutput { exit_code: 1, - stdout: String::new(), - stderr, formatted_output: String::new(), + aggregated_output: stderr, }), OutputLinesParams { - only_err: true, include_angle_pipe: true, include_prefix: true, }, @@ -1739,16 +1737,7 @@ mod tests { duration: None, }); // Mark call complete so markers are ✓ - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(80); let rendered = render_lines(&lines).join("\n"); @@ -1770,16 +1759,7 @@ mod tests { duration: None, }); // Call 1: Search only - cell.complete_call( - "c1", - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1)); // Call 2: Read A cell = cell .with_added_call( @@ -1792,16 +1772,7 @@ mod tests { }], ) .unwrap(); - cell.complete_call( - "c2", - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call("c2", CommandOutput::default(), Duration::from_millis(1)); // Call 3: Read B cell = cell .with_added_call( @@ -1814,16 +1785,7 @@ mod tests { }], ) .unwrap(); - cell.complete_call( - "c3", - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call("c3", CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(80); let rendered = render_lines(&lines).join("\n"); @@ -1856,16 +1818,7 @@ mod tests { start_time: Some(Instant::now()), duration: None, }); - cell.complete_call( - "c1", - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(80); let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); @@ -1885,16 +1838,7 @@ mod tests { duration: None, }); // Mark call complete so it renders as "Ran" - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); // Small width to force wrapping on both lines let width: u16 = 28; @@ -1914,16 +1858,7 @@ mod tests { start_time: Some(Instant::now()), duration: None, }); - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); // Wide enough that it fits inline let lines = cell.display_lines(80); let rendered = render_lines(&lines).join("\n"); @@ -1942,16 +1877,7 @@ mod tests { start_time: Some(Instant::now()), duration: None, }); - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(24); let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); @@ -1969,16 +1895,7 @@ mod tests { start_time: Some(Instant::now()), duration: None, }); - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(80); let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); @@ -1997,16 +1914,7 @@ mod tests { start_time: Some(Instant::now()), duration: None, }); - cell.complete_call( - &call_id, - CommandOutput { - exit_code: 0, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }, - Duration::from_millis(1), - ); + cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1)); let lines = cell.display_lines(28); let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); @@ -2033,9 +1941,8 @@ mod tests { &call_id, CommandOutput { exit_code: 1, - stdout: String::new(), - stderr, formatted_output: String::new(), + aggregated_output: stderr, }, Duration::from_millis(1), ); @@ -2077,9 +1984,8 @@ mod tests { &call_id, CommandOutput { exit_code: 1, - stdout: String::new(), - stderr, formatted_output: String::new(), + aggregated_output: stderr, }, Duration::from_millis(5), ); diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 84c325b6..b02e02ef 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -724,8 +724,7 @@ mod tests { "exec-1", CommandOutput { exit_code: 0, - stdout: "src\nREADME.md\n".into(), - stderr: String::new(), + aggregated_output: "src\nREADME.md\n".into(), formatted_output: "src\nREADME.md\n".into(), }, Duration::from_millis(420),