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.
This commit is contained in:
@@ -745,9 +745,8 @@ impl ChatWidget {
|
|||||||
&ev.call_id,
|
&ev.call_id,
|
||||||
CommandOutput {
|
CommandOutput {
|
||||||
exit_code: ev.exit_code,
|
exit_code: ev.exit_code,
|
||||||
stdout: ev.stdout.clone(),
|
|
||||||
stderr: ev.stderr.clone(),
|
|
||||||
formatted_output: ev.formatted_output.clone(),
|
formatted_output: ev.formatted_output.clone(),
|
||||||
|
aggregated_output: ev.aggregated_output.clone(),
|
||||||
},
|
},
|
||||||
ev.duration,
|
ev.duration,
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -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 Some(m) = msg.as_object_mut()
|
||||||
{
|
{
|
||||||
let ty = m.get("type").and_then(|v| v.as_str()).unwrap_or("");
|
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 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 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()
|
stdout.to_string()
|
||||||
} else {
|
} else {
|
||||||
format!("{stdout}{stderr}")
|
format!("{stdout}{stderr}")
|
||||||
};
|
};
|
||||||
m.insert(
|
if !m.contains_key("formatted_output") {
|
||||||
"formatted_output".to_string(),
|
m.insert(
|
||||||
serde_json::Value::String(formatted),
|
"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
|
payload
|
||||||
|
|||||||
@@ -3,11 +3,12 @@ use std::time::Instant;
|
|||||||
|
|
||||||
use codex_protocol::parse_command::ParsedCommand;
|
use codex_protocol::parse_command::ParsedCommand;
|
||||||
|
|
||||||
#[derive(Clone, Debug)]
|
#[derive(Clone, Debug, Default)]
|
||||||
pub(crate) struct CommandOutput {
|
pub(crate) struct CommandOutput {
|
||||||
pub(crate) exit_code: i32,
|
pub(crate) exit_code: i32,
|
||||||
pub(crate) stdout: String,
|
/// The aggregated stderr + stdout interleaved.
|
||||||
pub(crate) stderr: String,
|
pub(crate) aggregated_output: String,
|
||||||
|
/// The formatted output of the command, as seen by the model.
|
||||||
pub(crate) formatted_output: String,
|
pub(crate) formatted_output: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -82,9 +83,8 @@ impl ExecCell {
|
|||||||
call.duration = Some(elapsed);
|
call.duration = Some(elapsed);
|
||||||
call.output = Some(CommandOutput {
|
call.output = Some(CommandOutput {
|
||||||
exit_code: 1,
|
exit_code: 1,
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
formatted_output: String::new(),
|
||||||
|
aggregated_output: String::new(),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ use unicode_width::UnicodeWidthStr;
|
|||||||
pub(crate) const TOOL_CALL_MAX_LINES: usize = 5;
|
pub(crate) const TOOL_CALL_MAX_LINES: usize = 5;
|
||||||
|
|
||||||
pub(crate) struct OutputLinesParams {
|
pub(crate) struct OutputLinesParams {
|
||||||
pub(crate) only_err: bool,
|
|
||||||
pub(crate) include_angle_pipe: bool,
|
pub(crate) include_angle_pipe: bool,
|
||||||
pub(crate) include_prefix: bool,
|
pub(crate) include_prefix: bool,
|
||||||
}
|
}
|
||||||
@@ -59,22 +58,12 @@ pub(crate) fn output_lines(
|
|||||||
params: OutputLinesParams,
|
params: OutputLinesParams,
|
||||||
) -> OutputLines {
|
) -> OutputLines {
|
||||||
let OutputLinesParams {
|
let OutputLinesParams {
|
||||||
only_err,
|
|
||||||
include_angle_pipe,
|
include_angle_pipe,
|
||||||
include_prefix,
|
include_prefix,
|
||||||
} = params;
|
} = params;
|
||||||
let CommandOutput {
|
let CommandOutput {
|
||||||
exit_code,
|
aggregated_output, ..
|
||||||
stdout,
|
|
||||||
stderr,
|
|
||||||
..
|
|
||||||
} = match output {
|
} = match output {
|
||||||
Some(output) if only_err && output.exit_code == 0 => {
|
|
||||||
return OutputLines {
|
|
||||||
lines: Vec::new(),
|
|
||||||
omitted: None,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
Some(output) => output,
|
Some(output) => output,
|
||||||
None => {
|
None => {
|
||||||
return OutputLines {
|
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 lines: Vec<&str> = src.lines().collect();
|
||||||
let total = lines.len();
|
let total = lines.len();
|
||||||
let limit = TOOL_CALL_MAX_LINES;
|
let limit = TOOL_CALL_MAX_LINES;
|
||||||
@@ -398,7 +387,6 @@ impl ExecCell {
|
|||||||
let raw_output = output_lines(
|
let raw_output = output_lines(
|
||||||
Some(output),
|
Some(output),
|
||||||
OutputLinesParams {
|
OutputLinesParams {
|
||||||
only_err: false,
|
|
||||||
include_angle_pipe: false,
|
include_angle_pipe: false,
|
||||||
include_prefix: false,
|
include_prefix: false,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1293,12 +1293,10 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
|
|||||||
let output = output_lines(
|
let output = output_lines(
|
||||||
Some(&CommandOutput {
|
Some(&CommandOutput {
|
||||||
exit_code: 1,
|
exit_code: 1,
|
||||||
stdout: String::new(),
|
|
||||||
stderr,
|
|
||||||
formatted_output: String::new(),
|
formatted_output: String::new(),
|
||||||
|
aggregated_output: stderr,
|
||||||
}),
|
}),
|
||||||
OutputLinesParams {
|
OutputLinesParams {
|
||||||
only_err: true,
|
|
||||||
include_angle_pipe: true,
|
include_angle_pipe: true,
|
||||||
include_prefix: true,
|
include_prefix: true,
|
||||||
},
|
},
|
||||||
@@ -1739,16 +1737,7 @@ mod tests {
|
|||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
// Mark call complete so markers are ✓
|
// Mark call complete so markers are ✓
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
|
|
||||||
let lines = cell.display_lines(80);
|
let lines = cell.display_lines(80);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
@@ -1770,16 +1759,7 @@ mod tests {
|
|||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
// Call 1: Search only
|
// Call 1: Search only
|
||||||
cell.complete_call(
|
cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1));
|
||||||
"c1",
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
// Call 2: Read A
|
// Call 2: Read A
|
||||||
cell = cell
|
cell = cell
|
||||||
.with_added_call(
|
.with_added_call(
|
||||||
@@ -1792,16 +1772,7 @@ mod tests {
|
|||||||
}],
|
}],
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
cell.complete_call(
|
cell.complete_call("c2", CommandOutput::default(), Duration::from_millis(1));
|
||||||
"c2",
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
// Call 3: Read B
|
// Call 3: Read B
|
||||||
cell = cell
|
cell = cell
|
||||||
.with_added_call(
|
.with_added_call(
|
||||||
@@ -1814,16 +1785,7 @@ mod tests {
|
|||||||
}],
|
}],
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
cell.complete_call(
|
cell.complete_call("c3", CommandOutput::default(), Duration::from_millis(1));
|
||||||
"c3",
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
|
|
||||||
let lines = cell.display_lines(80);
|
let lines = cell.display_lines(80);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
@@ -1856,16 +1818,7 @@ mod tests {
|
|||||||
start_time: Some(Instant::now()),
|
start_time: Some(Instant::now()),
|
||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
cell.complete_call(
|
cell.complete_call("c1", CommandOutput::default(), Duration::from_millis(1));
|
||||||
"c1",
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
let lines = cell.display_lines(80);
|
let lines = cell.display_lines(80);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
insta::assert_snapshot!(rendered);
|
insta::assert_snapshot!(rendered);
|
||||||
@@ -1885,16 +1838,7 @@ mod tests {
|
|||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
// Mark call complete so it renders as "Ran"
|
// Mark call complete so it renders as "Ran"
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
|
|
||||||
// Small width to force wrapping on both lines
|
// Small width to force wrapping on both lines
|
||||||
let width: u16 = 28;
|
let width: u16 = 28;
|
||||||
@@ -1914,16 +1858,7 @@ mod tests {
|
|||||||
start_time: Some(Instant::now()),
|
start_time: Some(Instant::now()),
|
||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
// Wide enough that it fits inline
|
// Wide enough that it fits inline
|
||||||
let lines = cell.display_lines(80);
|
let lines = cell.display_lines(80);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
@@ -1942,16 +1877,7 @@ mod tests {
|
|||||||
start_time: Some(Instant::now()),
|
start_time: Some(Instant::now()),
|
||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
let lines = cell.display_lines(24);
|
let lines = cell.display_lines(24);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
insta::assert_snapshot!(rendered);
|
insta::assert_snapshot!(rendered);
|
||||||
@@ -1969,16 +1895,7 @@ mod tests {
|
|||||||
start_time: Some(Instant::now()),
|
start_time: Some(Instant::now()),
|
||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
let lines = cell.display_lines(80);
|
let lines = cell.display_lines(80);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
insta::assert_snapshot!(rendered);
|
insta::assert_snapshot!(rendered);
|
||||||
@@ -1997,16 +1914,7 @@ mod tests {
|
|||||||
start_time: Some(Instant::now()),
|
start_time: Some(Instant::now()),
|
||||||
duration: None,
|
duration: None,
|
||||||
});
|
});
|
||||||
cell.complete_call(
|
cell.complete_call(&call_id, CommandOutput::default(), Duration::from_millis(1));
|
||||||
&call_id,
|
|
||||||
CommandOutput {
|
|
||||||
exit_code: 0,
|
|
||||||
stdout: String::new(),
|
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: String::new(),
|
|
||||||
},
|
|
||||||
Duration::from_millis(1),
|
|
||||||
);
|
|
||||||
let lines = cell.display_lines(28);
|
let lines = cell.display_lines(28);
|
||||||
let rendered = render_lines(&lines).join("\n");
|
let rendered = render_lines(&lines).join("\n");
|
||||||
insta::assert_snapshot!(rendered);
|
insta::assert_snapshot!(rendered);
|
||||||
@@ -2033,9 +1941,8 @@ mod tests {
|
|||||||
&call_id,
|
&call_id,
|
||||||
CommandOutput {
|
CommandOutput {
|
||||||
exit_code: 1,
|
exit_code: 1,
|
||||||
stdout: String::new(),
|
|
||||||
stderr,
|
|
||||||
formatted_output: String::new(),
|
formatted_output: String::new(),
|
||||||
|
aggregated_output: stderr,
|
||||||
},
|
},
|
||||||
Duration::from_millis(1),
|
Duration::from_millis(1),
|
||||||
);
|
);
|
||||||
@@ -2077,9 +1984,8 @@ mod tests {
|
|||||||
&call_id,
|
&call_id,
|
||||||
CommandOutput {
|
CommandOutput {
|
||||||
exit_code: 1,
|
exit_code: 1,
|
||||||
stdout: String::new(),
|
|
||||||
stderr,
|
|
||||||
formatted_output: String::new(),
|
formatted_output: String::new(),
|
||||||
|
aggregated_output: stderr,
|
||||||
},
|
},
|
||||||
Duration::from_millis(5),
|
Duration::from_millis(5),
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -724,8 +724,7 @@ mod tests {
|
|||||||
"exec-1",
|
"exec-1",
|
||||||
CommandOutput {
|
CommandOutput {
|
||||||
exit_code: 0,
|
exit_code: 0,
|
||||||
stdout: "src\nREADME.md\n".into(),
|
aggregated_output: "src\nREADME.md\n".into(),
|
||||||
stderr: String::new(),
|
|
||||||
formatted_output: "src\nREADME.md\n".into(),
|
formatted_output: "src\nREADME.md\n".into(),
|
||||||
},
|
},
|
||||||
Duration::from_millis(420),
|
Duration::from_millis(420),
|
||||||
|
|||||||
Reference in New Issue
Block a user