Skip to content

Commit 1d43bbc

Browse files
nornagon-openaiJeffCarpenter
authored andcommitted
tui: show aggregated output in display (openai#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.
1 parent a8ed762 commit 1d43bbc

File tree

6 files changed

+36
-136
lines changed

6 files changed

+36
-136
lines changed

codex-rs/tui/src/chatwidget.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -745,9 +745,8 @@ impl ChatWidget {
745745
&ev.call_id,
746746
CommandOutput {
747747
exit_code: ev.exit_code,
748-
stdout: ev.stdout.clone(),
749-
stderr: ev.stderr.clone(),
750748
formatted_output: ev.formatted_output.clone(),
749+
aggregated_output: ev.aggregated_output.clone(),
751750
},
752751
ev.duration,
753752
);

codex-rs/tui/src/chatwidget/tests.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,26 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json
7171
&& let Some(m) = msg.as_object_mut()
7272
{
7373
let ty = m.get("type").and_then(|v| v.as_str()).unwrap_or("");
74-
if ty == "exec_command_end" && !m.contains_key("formatted_output") {
74+
if ty == "exec_command_end" {
7575
let stdout = m.get("stdout").and_then(|v| v.as_str()).unwrap_or("");
7676
let stderr = m.get("stderr").and_then(|v| v.as_str()).unwrap_or("");
77-
let formatted = if stderr.is_empty() {
77+
let aggregated = if stderr.is_empty() {
7878
stdout.to_string()
7979
} else {
8080
format!("{stdout}{stderr}")
8181
};
82-
m.insert(
83-
"formatted_output".to_string(),
84-
serde_json::Value::String(formatted),
85-
);
82+
if !m.contains_key("formatted_output") {
83+
m.insert(
84+
"formatted_output".to_string(),
85+
serde_json::Value::String(aggregated.clone()),
86+
);
87+
}
88+
if !m.contains_key("aggregated_output") {
89+
m.insert(
90+
"aggregated_output".to_string(),
91+
serde_json::Value::String(aggregated),
92+
);
93+
}
8694
}
8795
}
8896
payload

codex-rs/tui/src/exec_cell/model.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ use std::time::Instant;
33

44
use codex_protocol::parse_command::ParsedCommand;
55

6-
#[derive(Clone, Debug)]
6+
#[derive(Clone, Debug, Default)]
77
pub(crate) struct CommandOutput {
88
pub(crate) exit_code: i32,
9-
pub(crate) stdout: String,
10-
pub(crate) stderr: String,
9+
/// The aggregated stderr + stdout interleaved.
10+
pub(crate) aggregated_output: String,
11+
/// The formatted output of the command, as seen by the model.
1112
pub(crate) formatted_output: String,
1213
}
1314

@@ -82,9 +83,8 @@ impl ExecCell {
8283
call.duration = Some(elapsed);
8384
call.output = Some(CommandOutput {
8485
exit_code: 1,
85-
stdout: String::new(),
86-
stderr: String::new(),
8786
formatted_output: String::new(),
87+
aggregated_output: String::new(),
8888
});
8989
}
9090
}

codex-rs/tui/src/exec_cell/render.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ use unicode_width::UnicodeWidthStr;
2828
pub(crate) const TOOL_CALL_MAX_LINES: usize = 5;
2929

3030
pub(crate) struct OutputLinesParams {
31-
pub(crate) only_err: bool,
3231
pub(crate) include_angle_pipe: bool,
3332
pub(crate) include_prefix: bool,
3433
}
@@ -59,22 +58,12 @@ pub(crate) fn output_lines(
5958
params: OutputLinesParams,
6059
) -> OutputLines {
6160
let OutputLinesParams {
62-
only_err,
6361
include_angle_pipe,
6462
include_prefix,
6563
} = params;
6664
let CommandOutput {
67-
exit_code,
68-
stdout,
69-
stderr,
70-
..
65+
aggregated_output, ..
7166
} = match output {
72-
Some(output) if only_err && output.exit_code == 0 => {
73-
return OutputLines {
74-
lines: Vec::new(),
75-
omitted: None,
76-
};
77-
}
7867
Some(output) => output,
7968
None => {
8069
return OutputLines {
@@ -84,7 +73,7 @@ pub(crate) fn output_lines(
8473
}
8574
};
8675

87-
let src = if *exit_code == 0 { stdout } else { stderr };
76+
let src = aggregated_output;
8877
let lines: Vec<&str> = src.lines().collect();
8978
let total = lines.len();
9079
let limit = TOOL_CALL_MAX_LINES;
@@ -398,7 +387,6 @@ impl ExecCell {
398387
let raw_output = output_lines(
399388
Some(output),
400389
OutputLinesParams {
401-
only_err: false,
402390
include_angle_pipe: false,
403391
include_prefix: false,
404392
},

codex-rs/tui/src/history_cell.rs

Lines changed: 13 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,12 +1293,10 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
12931293
let output = output_lines(
12941294
Some(&CommandOutput {
12951295
exit_code: 1,
1296-
stdout: String::new(),
1297-
stderr,
12981296
formatted_output: String::new(),
1297+
aggregated_output: stderr,
12991298
}),
13001299
OutputLinesParams {
1301-
only_err: true,
13021300
include_angle_pipe: true,
13031301
include_prefix: true,
13041302
},
@@ -1739,16 +1737,7 @@ mod tests {
17391737
duration: None,
17401738
});
17411739
// Mark call complete so markers are ✓
1742-
cell.complete_call(
1743-
&call_id,
1744-
CommandOutput {
1745-
exit_code: 0,
1746-
stdout: String::new(),
1747-
stderr: String::new(),
1748-
formatted_output: String::new(),
1749-
},
1750-
Duration::from_millis(1),
1751-
);
1740+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
17521741

17531742
let lines = cell.display_lines(80);
17541743
let rendered = render_lines(&lines).join("\n");
@@ -1770,16 +1759,7 @@ mod tests {
17701759
duration: None,
17711760
});
17721761
// Call 1: Search only
1773-
cell.complete_call(
1774-
"c1",
1775-
CommandOutput {
1776-
exit_code: 0,
1777-
stdout: String::new(),
1778-
stderr: String::new(),
1779-
formatted_output: String::new(),
1780-
},
1781-
Duration::from_millis(1),
1782-
);
1762+
cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1));
17831763
// Call 2: Read A
17841764
cell = cell
17851765
.with_added_call(
@@ -1792,16 +1772,7 @@ mod tests {
17921772
}],
17931773
)
17941774
.unwrap();
1795-
cell.complete_call(
1796-
"c2",
1797-
CommandOutput {
1798-
exit_code: 0,
1799-
stdout: String::new(),
1800-
stderr: String::new(),
1801-
formatted_output: String::new(),
1802-
},
1803-
Duration::from_millis(1),
1804-
);
1775+
cell.complete_call("c2", CommandOutput::default(), Duration::from_millis(1));
18051776
// Call 3: Read B
18061777
cell = cell
18071778
.with_added_call(
@@ -1814,16 +1785,7 @@ mod tests {
18141785
}],
18151786
)
18161787
.unwrap();
1817-
cell.complete_call(
1818-
"c3",
1819-
CommandOutput {
1820-
exit_code: 0,
1821-
stdout: String::new(),
1822-
stderr: String::new(),
1823-
formatted_output: String::new(),
1824-
},
1825-
Duration::from_millis(1),
1826-
);
1788+
cell.complete_call("c3", CommandOutput::default(), Duration::from_millis(1));
18271789

18281790
let lines = cell.display_lines(80);
18291791
let rendered = render_lines(&lines).join("\n");
@@ -1856,16 +1818,7 @@ mod tests {
18561818
start_time: Some(Instant::now()),
18571819
duration: None,
18581820
});
1859-
cell.complete_call(
1860-
"c1",
1861-
CommandOutput {
1862-
exit_code: 0,
1863-
stdout: String::new(),
1864-
stderr: String::new(),
1865-
formatted_output: String::new(),
1866-
},
1867-
Duration::from_millis(1),
1868-
);
1821+
cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1));
18691822
let lines = cell.display_lines(80);
18701823
let rendered = render_lines(&lines).join("\n");
18711824
insta::assert_snapshot!(rendered);
@@ -1885,16 +1838,7 @@ mod tests {
18851838
duration: None,
18861839
});
18871840
// Mark call complete so it renders as "Ran"
1888-
cell.complete_call(
1889-
&call_id,
1890-
CommandOutput {
1891-
exit_code: 0,
1892-
stdout: String::new(),
1893-
stderr: String::new(),
1894-
formatted_output: String::new(),
1895-
},
1896-
Duration::from_millis(1),
1897-
);
1841+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
18981842

18991843
// Small width to force wrapping on both lines
19001844
let width: u16 = 28;
@@ -1914,16 +1858,7 @@ mod tests {
19141858
start_time: Some(Instant::now()),
19151859
duration: None,
19161860
});
1917-
cell.complete_call(
1918-
&call_id,
1919-
CommandOutput {
1920-
exit_code: 0,
1921-
stdout: String::new(),
1922-
stderr: String::new(),
1923-
formatted_output: String::new(),
1924-
},
1925-
Duration::from_millis(1),
1926-
);
1861+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
19271862
// Wide enough that it fits inline
19281863
let lines = cell.display_lines(80);
19291864
let rendered = render_lines(&lines).join("\n");
@@ -1942,16 +1877,7 @@ mod tests {
19421877
start_time: Some(Instant::now()),
19431878
duration: None,
19441879
});
1945-
cell.complete_call(
1946-
&call_id,
1947-
CommandOutput {
1948-
exit_code: 0,
1949-
stdout: String::new(),
1950-
stderr: String::new(),
1951-
formatted_output: String::new(),
1952-
},
1953-
Duration::from_millis(1),
1954-
);
1880+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
19551881
let lines = cell.display_lines(24);
19561882
let rendered = render_lines(&lines).join("\n");
19571883
insta::assert_snapshot!(rendered);
@@ -1969,16 +1895,7 @@ mod tests {
19691895
start_time: Some(Instant::now()),
19701896
duration: None,
19711897
});
1972-
cell.complete_call(
1973-
&call_id,
1974-
CommandOutput {
1975-
exit_code: 0,
1976-
stdout: String::new(),
1977-
stderr: String::new(),
1978-
formatted_output: String::new(),
1979-
},
1980-
Duration::from_millis(1),
1981-
);
1898+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
19821899
let lines = cell.display_lines(80);
19831900
let rendered = render_lines(&lines).join("\n");
19841901
insta::assert_snapshot!(rendered);
@@ -1997,16 +1914,7 @@ mod tests {
19971914
start_time: Some(Instant::now()),
19981915
duration: None,
19991916
});
2000-
cell.complete_call(
2001-
&call_id,
2002-
CommandOutput {
2003-
exit_code: 0,
2004-
stdout: String::new(),
2005-
stderr: String::new(),
2006-
formatted_output: String::new(),
2007-
},
2008-
Duration::from_millis(1),
2009-
);
1917+
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
20101918
let lines = cell.display_lines(28);
20111919
let rendered = render_lines(&lines).join("\n");
20121920
insta::assert_snapshot!(rendered);
@@ -2033,9 +1941,8 @@ mod tests {
20331941
&call_id,
20341942
CommandOutput {
20351943
exit_code: 1,
2036-
stdout: String::new(),
2037-
stderr,
20381944
formatted_output: String::new(),
1945+
aggregated_output: stderr,
20391946
},
20401947
Duration::from_millis(1),
20411948
);
@@ -2077,9 +1984,8 @@ mod tests {
20771984
&call_id,
20781985
CommandOutput {
20791986
exit_code: 1,
2080-
stdout: String::new(),
2081-
stderr,
20821987
formatted_output: String::new(),
1988+
aggregated_output: stderr,
20831989
},
20841990
Duration::from_millis(5),
20851991
);

codex-rs/tui/src/pager_overlay.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,8 +724,7 @@ mod tests {
724724
"exec-1",
725725
CommandOutput {
726726
exit_code: 0,
727-
stdout: "src\nREADME.md\n".into(),
728-
stderr: String::new(),
727+
aggregated_output: "src\nREADME.md\n".into(),
729728
formatted_output: "src\nREADME.md\n".into(),
730729
},
731730
Duration::from_millis(420),

0 commit comments

Comments
 (0)