mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Render streamed command output through preview iterators (#34357)
## What changed - Encapsulate aggregated command output behind APIs for total and retained line counts, reversible preview iteration, and full transcript iteration. - Build truncated command previews from the retained head and tail lines while preserving the logical omitted-line count. - Cover head/tail rendering for long streamed agent and user-shell output. GitOrigin-RevId: 54b23a19e4639e24c5e4778082edbbc6484ecf80
This commit is contained in:
@@ -380,15 +380,9 @@ impl ChatWidget {
|
||||
// Unified exec interaction rows intentionally hide command output text in the exec cell and
|
||||
// instead render the interaction-specific content elsewhere in the UI.
|
||||
let output = if is_unified_exec_interaction {
|
||||
CommandOutput {
|
||||
exit_code,
|
||||
aggregated_output: String::new(),
|
||||
}
|
||||
CommandOutput::new(exit_code, String::new())
|
||||
} else {
|
||||
CommandOutput {
|
||||
exit_code,
|
||||
aggregated_output,
|
||||
}
|
||||
CommandOutput::new(exit_code, aggregated_output)
|
||||
};
|
||||
|
||||
match end_target {
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
//! end events into the right cell, and it treats "call id not found" as a real signal (for
|
||||
//! example, an orphan end that should render as a separate history entry).
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
@@ -15,7 +16,32 @@ use codex_protocol::parse_command::ParsedCommand;
|
||||
pub(crate) struct CommandOutput {
|
||||
pub(crate) exit_code: i32,
|
||||
/// The aggregated stderr + stdout interleaved.
|
||||
pub(crate) aggregated_output: String,
|
||||
aggregated_output: String,
|
||||
}
|
||||
|
||||
impl CommandOutput {
|
||||
pub(crate) fn new(exit_code: i32, aggregated_output: String) -> Self {
|
||||
Self {
|
||||
exit_code,
|
||||
aggregated_output,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the total number of logical lines and the number retained for rendering.
|
||||
pub(super) fn line_counts(&self) -> (usize, usize) {
|
||||
let total = self.aggregated_output.lines().count();
|
||||
(total, total)
|
||||
}
|
||||
|
||||
/// Returns retained preview lines with reverse traversal for efficient tail rendering.
|
||||
pub(super) fn lines(&self) -> impl DoubleEndedIterator<Item = Cow<'_, str>> {
|
||||
self.aggregated_output.lines().map(Cow::Borrowed)
|
||||
}
|
||||
|
||||
/// Returns lines for the expanded transcript, including any storage-level omission marker.
|
||||
pub(super) fn transcript_lines(&self) -> impl Iterator<Item = Cow<'_, str>> {
|
||||
self.aggregated_output.lines().map(Cow::Borrowed)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
||||
@@ -110,9 +110,7 @@ pub(crate) fn output_lines(
|
||||
include_angle_pipe,
|
||||
include_prefix,
|
||||
} = params;
|
||||
let CommandOutput {
|
||||
aggregated_output, ..
|
||||
} = match output {
|
||||
let output = match output {
|
||||
Some(output) if only_err && output.exit_code == 0 => {
|
||||
return OutputLines {
|
||||
lines: Vec::new(),
|
||||
@@ -128,12 +126,12 @@ pub(crate) fn output_lines(
|
||||
}
|
||||
};
|
||||
|
||||
let total = aggregated_output.lines().count();
|
||||
let (total, retained) = output.line_counts();
|
||||
let mut out: Vec<Line<'static>> = Vec::new();
|
||||
|
||||
let head_end = total.min(line_limit);
|
||||
for (i, raw) in aggregated_output.lines().take(head_end).enumerate() {
|
||||
let mut line = ansi_escape_line(raw);
|
||||
let head_end = total.min(line_limit).min(retained);
|
||||
for (i, raw) in output.lines().take(head_end).enumerate() {
|
||||
let mut line = ansi_escape_line(raw.as_ref());
|
||||
let prefix = if !include_prefix {
|
||||
""
|
||||
} else if i == 0 && include_angle_pipe {
|
||||
@@ -148,24 +146,19 @@ pub(crate) fn output_lines(
|
||||
out.push(line);
|
||||
}
|
||||
|
||||
let show_ellipsis = total > 2 * line_limit;
|
||||
let omitted = if show_ellipsis {
|
||||
Some(total - 2 * line_limit)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if show_ellipsis {
|
||||
let omitted = total - 2 * line_limit;
|
||||
let tail_len = total
|
||||
.saturating_sub(head_end)
|
||||
.min(line_limit)
|
||||
.min(retained.saturating_sub(head_end));
|
||||
let omitted = total.saturating_sub(head_end + tail_len);
|
||||
let omitted = (omitted > 0).then_some(omitted);
|
||||
if let Some(omitted) = omitted {
|
||||
out.push(ExecCell::output_ellipsis_line(omitted));
|
||||
}
|
||||
|
||||
let tail_start = if show_ellipsis {
|
||||
total - line_limit
|
||||
} else {
|
||||
head_end
|
||||
};
|
||||
for raw in aggregated_output.lines().skip(tail_start) {
|
||||
let mut line = ansi_escape_line(raw);
|
||||
let tail = output.lines().rev().take(tail_len).collect_vec();
|
||||
for raw in tail.into_iter().rev() {
|
||||
let mut line = ansi_escape_line(raw.as_ref());
|
||||
if include_prefix {
|
||||
line.spans.insert(0, " ".into());
|
||||
}
|
||||
@@ -219,7 +212,10 @@ impl HistoryCell for ExecCell {
|
||||
if !call.is_unified_exec_interaction() {
|
||||
let wrap_width = width.max(1) as usize;
|
||||
let wrap_opts = RtOptions::new(wrap_width);
|
||||
for unwrapped in output.aggregated_output.lines().map(ansi_escape_line) {
|
||||
for unwrapped in output
|
||||
.transcript_lines()
|
||||
.map(|line| ansi_escape_line(line.as_ref()))
|
||||
{
|
||||
let wrapped = adaptive_wrap_line(&unwrapped, wrap_opts.clone());
|
||||
push_owned_lines(&wrapped, &mut lines);
|
||||
}
|
||||
@@ -726,10 +722,7 @@ mod tests {
|
||||
|
||||
// Baseline: how many screen lines would we get if we simply wrapped
|
||||
// all logical lines without any truncation?
|
||||
let output = CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output,
|
||||
};
|
||||
let output = CommandOutput::new(/*exit_code*/ 0, aggregated_output);
|
||||
let width = 20;
|
||||
let layout = EXEC_DISPLAY_LAYOUT;
|
||||
let raw_output = output_lines(
|
||||
@@ -851,10 +844,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn output_lines_ellipsis_includes_transcript_hint() {
|
||||
let output = CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output: (1..=7).map(|n| n.to_string()).join("\n"),
|
||||
};
|
||||
let output = CommandOutput::new(
|
||||
/*exit_code*/ 0,
|
||||
(1..=7).map(|n| n.to_string()).join("\n"),
|
||||
);
|
||||
|
||||
let rendered: Vec<String> = output_lines(
|
||||
Some(&output),
|
||||
@@ -884,10 +877,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn output_lines_handles_newline_dense_output_without_materializing_every_line() {
|
||||
let output = CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output: "\n".repeat(100_000),
|
||||
};
|
||||
let output = CommandOutput::new(/*exit_code*/ 0, "\n".repeat(100_000));
|
||||
|
||||
let rendered = output_lines(
|
||||
Some(&output),
|
||||
@@ -903,6 +893,50 @@ mod tests {
|
||||
assert_eq!(rendered.omitted, Some(99_990));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn streamed_output_renders_head_tail_previews() {
|
||||
let mut cell = new_active_exec_command(
|
||||
"call-id".to_string(),
|
||||
vec!["bash".into(), "-lc".into(), "echo output".into()],
|
||||
Vec::new(),
|
||||
ExecCommandSource::Agent,
|
||||
/*interaction_input*/ None,
|
||||
/*animations_enabled*/ false,
|
||||
);
|
||||
for line in 1..=160 {
|
||||
assert!(cell.append_output("call-id", &format!("line {line}\n")));
|
||||
}
|
||||
let output = cell.calls[0].output.as_ref().expect("streamed output");
|
||||
|
||||
let agent = output_lines(
|
||||
Some(output),
|
||||
OutputLinesParams {
|
||||
line_limit: TOOL_CALL_MAX_LINES,
|
||||
only_err: false,
|
||||
include_angle_pipe: false,
|
||||
include_prefix: false,
|
||||
},
|
||||
);
|
||||
assert_eq!(agent.lines.len(), 11);
|
||||
assert_eq!(agent.omitted, Some(150));
|
||||
assert_eq!(render_line_text(&agent.lines[0]), "line 1");
|
||||
assert_eq!(render_line_text(&agent.lines[10]), "line 160");
|
||||
|
||||
let user_shell = output_lines(
|
||||
Some(output),
|
||||
OutputLinesParams {
|
||||
line_limit: USER_SHELL_TOOL_CALL_MAX_LINES,
|
||||
only_err: false,
|
||||
include_angle_pipe: false,
|
||||
include_prefix: false,
|
||||
},
|
||||
);
|
||||
assert_eq!(user_shell.lines.len(), 101);
|
||||
assert_eq!(user_shell.omitted, Some(60));
|
||||
assert_eq!(render_line_text(&user_shell.lines[0]), "line 1");
|
||||
assert_eq!(render_line_text(&user_shell.lines[100]), "line 160");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_truncation_ellipsis_does_not_include_transcript_hint() {
|
||||
let truncated = ExecCell::limit_lines_from_start(
|
||||
@@ -1050,10 +1084,7 @@ mod tests {
|
||||
call_id: "call-id".to_string(),
|
||||
command: vec!["bash".into(), "-lc".into(), "echo done".into()],
|
||||
parsed: Vec::new(),
|
||||
output: Some(CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output: url.to_string(),
|
||||
}),
|
||||
output: Some(CommandOutput::new(/*exit_code*/ 0, url.to_string())),
|
||||
source: ExecCommandSource::UserShell,
|
||||
start_time: None,
|
||||
duration: None,
|
||||
@@ -1086,10 +1117,7 @@ mod tests {
|
||||
call_id: "call-id".to_string(),
|
||||
command: vec!["bash".into(), "-lc".into(), "echo done".into()],
|
||||
parsed: Vec::new(),
|
||||
output: Some(CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output: url.to_string(),
|
||||
}),
|
||||
output: Some(CommandOutput::new(/*exit_code*/ 0, url.to_string())),
|
||||
source: ExecCommandSource::Agent,
|
||||
start_time: None,
|
||||
duration: None,
|
||||
|
||||
@@ -43,10 +43,7 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
|
||||
|
||||
if !stderr.trim().is_empty() {
|
||||
let output = output_lines(
|
||||
Some(&CommandOutput {
|
||||
exit_code: 1,
|
||||
aggregated_output: stderr,
|
||||
}),
|
||||
Some(&CommandOutput::new(/*exit_code*/ 1, stderr)),
|
||||
OutputLinesParams {
|
||||
line_limit: TOOL_CALL_MAX_LINES,
|
||||
only_err: true,
|
||||
|
||||
@@ -1912,10 +1912,7 @@ fn stderr_tail_more_than_five_lines_snapshot() {
|
||||
.join("\n");
|
||||
cell.complete_call(
|
||||
&call_id,
|
||||
CommandOutput {
|
||||
exit_code: 1,
|
||||
aggregated_output: stderr,
|
||||
},
|
||||
CommandOutput::new(/*exit_code*/ 1, stderr),
|
||||
Duration::from_millis(1),
|
||||
);
|
||||
|
||||
@@ -1959,10 +1956,7 @@ fn ran_cell_multiline_with_stderr_snapshot() {
|
||||
let stderr = "error: first line on stderr\nerror: second line on stderr".to_string();
|
||||
cell.complete_call(
|
||||
&call_id,
|
||||
CommandOutput {
|
||||
exit_code: 1,
|
||||
aggregated_output: stderr,
|
||||
},
|
||||
CommandOutput::new(/*exit_code*/ 1, stderr),
|
||||
Duration::from_millis(5),
|
||||
);
|
||||
|
||||
|
||||
@@ -1254,10 +1254,7 @@ mod tests {
|
||||
);
|
||||
exec_cell.complete_call(
|
||||
"exec-1",
|
||||
CommandOutput {
|
||||
exit_code: 0,
|
||||
aggregated_output: "src\nREADME.md\n".into(),
|
||||
},
|
||||
CommandOutput::new(/*exit_code*/ 0, "src\nREADME.md\n".into()),
|
||||
Duration::from_millis(420),
|
||||
);
|
||||
let exec_cell: Arc<dyn HistoryCell> = Arc::new(exec_cell);
|
||||
|
||||
Reference in New Issue
Block a user