Refactor history_cell for clarity, intent, and safety

codex-rs/tui/Cargo.toml

@@ -90,6 +90,7 @@ unicode-width = { workspace = true }
 url = { workspace = true }
 
 codex-windows-sandbox = { workspace = true }
+indoc = "2.0.7"
 
 [target.'cfg(unix)'.dependencies]
 libc = { workspace = true }

codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_error_message.snap

@@ -1,5 +1,7 @@
 ---
 source: tui/src/chatwidget/tests.rs
+assertion_line: 1412
 expression: last
 ---
-■ Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.
+■ Conversation interrupted - tell the model what to do differently. Something
+went wrong? Hit `/feedback` to report the issue.

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

@@ -4,15 +4,18 @@ use std::time::Instant;
 use codex_core::protocol::ExecCommandSource;
 use codex_protocol::parse_command::ParsedCommand;
 
+/// Output captured from a completed exec call, including exit code and combined streams.
 #[derive(Clone, Debug, Default)]
 pub(crate) struct CommandOutput {
+    /// The exit status returned by the command.
     pub(crate) exit_code: i32,
     /// 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,
 }
 
+/// Single exec invocation (shell or tool) as it flows through the history cell.
 #[derive(Debug, Clone)]
 pub(crate) struct ExecCall {
     pub(crate) call_id: String,
@@ -25,20 +28,38 @@ pub(crate) struct ExecCall {
     pub(crate) interaction_input: Option<String>,
 }
 
+/// History cell that renders exec/search/read calls with status and wrapped output.
+///
+/// Exploring calls collapse search/read/list steps under an "Exploring"/"Explored" header with a
+/// spinner or bullet. Non-exploration runs render a status bullet plus wrapped command, then a
+/// tree-prefixed output block that truncates middle lines when necessary.
+///
+/// # Output
+///
+/// ```plain
+/// • Ran bash -lc "rg term"
+///   │ Search shimmer_spans in .
+///   └ (no output)
+/// ```
 #[derive(Debug)]
 pub(crate) struct ExecCell {
     pub(crate) calls: Vec<ExecCall>,
     animations_enabled: bool,
 }
 
 impl ExecCell {
+    /// Create a new cell with a single active call and control over spinner animation.
     pub(crate) fn new(call: ExecCall, animations_enabled: bool) -> Self {
         Self {
             calls: vec![call],
             animations_enabled,
         }
     }
 
+    /// Append an additional exploring call to the cell if it belongs to the same batch.
+    ///
+    /// Exploring calls render together (search/list/read), so when a new call is also exploring we
+    /// coalesce it into the existing cell to avoid noisy standalone entries.
     pub(crate) fn with_added_call(
         &self,
         call_id: String,
@@ -67,6 +88,7 @@ impl ExecCell {
         }
     }
 
+    /// Mark a call as completed with captured output and duration, replacing any spinner.
     pub(crate) fn complete_call(
         &mut self,
         call_id: &str,
@@ -80,10 +102,12 @@ impl ExecCell {
         }
     }
 
+    /// Return true when the cell has only exploring calls and every call has finished.
     pub(crate) fn should_flush(&self) -> bool {
         !self.is_exploring_cell() && self.calls.iter().all(|c| c.output.is_some())
     }
 
+    /// Mark in-flight calls as failed, preserving how long they were running.
     pub(crate) fn mark_failed(&mut self) {
         for call in self.calls.iter_mut() {
             if call.output.is_none() {
@@ -102,29 +126,35 @@ impl ExecCell {
         }
     }
 
+    /// Whether all calls are exploratory (search/list/read) and should render together.
     pub(crate) fn is_exploring_cell(&self) -> bool {
         self.calls.iter().all(Self::is_exploring_call)
     }
 
+    /// True if any call is still active.
     pub(crate) fn is_active(&self) -> bool {
         self.calls.iter().any(|c| c.output.is_none())
     }
 
+    /// Start time of the first active call, used to drive spinners.
     pub(crate) fn active_start_time(&self) -> Option<Instant> {
         self.calls
             .iter()
             .find(|c| c.output.is_none())
             .and_then(|c| c.start_time)
     }
 
+    /// Whether animated spinners are enabled for active calls.
     pub(crate) fn animations_enabled(&self) -> bool {
         self.animations_enabled
     }
 
+    /// Iterate over contained calls in order for rendering.
     pub(crate) fn iter_calls(&self) -> impl Iterator<Item = &ExecCall> {
         self.calls.iter()
     }
 
+    /// Detect whether a call is exploratory (read/list/search) for coalescing.
     pub(super) fn is_exploring_call(call: &ExecCall) -> bool {
         !matches!(call.source, ExecCommandSource::UserShell)
             && !call.parsed.is_empty()
@@ -140,10 +170,12 @@ impl ExecCell {
 }
 
 impl ExecCall {
+    /// Whether the invocation originated from a user shell command.
     pub(crate) fn is_user_shell_command(&self) -> bool {
         matches!(self.source, ExecCommandSource::UserShell)
     }
 
+    /// Whether the invocation expects user input back (unified exec interaction).
     pub(crate) fn is_unified_exec_interaction(&self) -> bool {
         matches!(self.source, ExecCommandSource::UnifiedExecInteraction)
     }

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

@@ -17,23 +17,26 @@ use codex_common::elapsed::format_duration;
 use codex_core::protocol::ExecCommandSource;
 use codex_protocol::parse_command::ParsedCommand;
 use itertools::Itertools;
-use ratatui::prelude::*;
 use ratatui::style::Modifier;
 use ratatui::style::Stylize;
+use ratatui::text::Line;
+use ratatui::text::Span;
 use textwrap::WordSplitter;
 use unicode_width::UnicodeWidthStr;
 
 pub(crate) const TOOL_CALL_MAX_LINES: usize = 5;
 const USER_SHELL_TOOL_CALL_MAX_LINES: usize = 50;
 const MAX_INTERACTION_PREVIEW_CHARS: usize = 80;
 
+/// How much output to include when rendering the output block.
 pub(crate) struct OutputLinesParams {
     pub(crate) line_limit: usize,
     pub(crate) only_err: bool,
     pub(crate) include_angle_pipe: bool,
     pub(crate) include_prefix: bool,
 }
 
+/// Build a new active exec command cell that animates while running.
 pub(crate) fn new_active_exec_command(
     call_id: String,
     command: Vec<String>,
@@ -57,6 +60,7 @@ pub(crate) fn new_active_exec_command(
     )
 }
 
+/// Format the unified exec message shown when the agent interacts with a tool.
 fn format_unified_exec_interaction(command: &[String], input: Option<&str>) -> String {
     let command_display = command.join(" ");
     match input {
@@ -68,6 +72,7 @@ fn format_unified_exec_interaction(command: &[String], input: Option<&str>) -> S
     }
 }
 
+/// Trim interaction input to a short, single-line preview for the history.
 fn summarize_interaction_input(input: &str) -> String {
     let single_line = input.replace('\n', "\\n");
     let sanitized = single_line.replace('`', "\\`");
@@ -89,6 +94,7 @@ pub(crate) struct OutputLines {
     pub(crate) omitted: Option<usize>,
 }
 
+/// Render command output with optional truncation and tree prefixes.
 pub(crate) fn output_lines(
     output: Option<&CommandOutput>,
     params: OutputLinesParams,
@@ -172,6 +178,7 @@ pub(crate) fn output_lines(
     }
 }
 
+/// Spinner shown for active exec calls, respecting 16m color when available.
 pub(crate) fn spinner(start_time: Option<Instant>, animations_enabled: bool) -> Span<'static> {
     if !animations_enabled {
         return "•".dim();
@@ -189,6 +196,7 @@ pub(crate) fn spinner(start_time: Option<Instant>, animations_enabled: bool) ->
 }
 
 impl HistoryCell for ExecCell {
+    /// Render as either an "Exploring" grouped call list or single command/run output.
     fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
         if self.is_exploring_cell() {
             self.exploring_display_lines(width)
@@ -197,10 +205,12 @@ impl HistoryCell for ExecCell {
         }
     }
 
+    /// Transcript height matches raw line count because transcript rendering omits wrapping.
     fn desired_transcript_height(&self, width: u16) -> u16 {
         self.transcript_lines(width).len() as u16
     }
 
+    /// Render a transcript-friendly version of the exec calls without UI padding.
     fn transcript_lines(&self, width: u16) -> Vec<Line<'static>> {
         let mut lines: Vec<Line<'static>> = vec![];
         for (i, call) in self.iter_calls().enumerate() {
@@ -242,6 +252,10 @@ impl HistoryCell for ExecCell {
 }
 
 impl ExecCell {
+    /// Render exploring reads/searches as a grouped list under a shared header.
+    ///
+    /// Collapses sequential reads, dedupes filenames, and prefixes wrapped lines with `└`/spaces
+    /// so the block sits under the "Exploring"/"Explored" status line.
     fn exploring_display_lines(&self, width: u16) -> Vec<Line<'static>> {
         let mut out: Vec<Line<'static>> = Vec::new();
         out.push(Line::from(vec![
@@ -345,6 +359,10 @@ impl ExecCell {
         out
     }
 
+    /// Render a single command invocation with wrapped command and trimmed output.
+    ///
+    /// Uses colored bullets for running/success/error, wraps command lines with `│` prefixes, and
+    /// emits a tree-prefixed output block that truncates to the configured maximum lines.
     fn command_display_lines(&self, width: u16) -> Vec<Line<'static>> {
         let [call] = &self.calls.as_slice() else {
             panic!("Expected exactly one call in a command display cell");
@@ -481,6 +499,7 @@ impl ExecCell {
         lines
     }
 
+    /// Keep only the first `keep` lines, replacing the rest with an ellipsis entry.
     fn limit_lines_from_start(lines: &[Line<'static>], keep: usize) -> Vec<Line<'static>> {
         if lines.len() <= keep {
             return lines.to_vec();
@@ -494,6 +513,7 @@ impl ExecCell {
         out
     }
 
+    /// Replace the middle of a line list with an ellipsis, preserving head/tail edges.
     fn truncate_lines_middle(
         lines: &[Line<'static>],
         max: usize,
@@ -541,32 +561,37 @@ impl ExecCell {
         out
     }
 
+    /// Build a dimmed ellipsis line noting how many lines were hidden.
     fn ellipsis_line(omitted: usize) -> Line<'static> {
         Line::from(vec![format!("… +{omitted} lines").dim()])
     }
 }
 
+/// Prefix configuration for wrapped command/output sections.
 #[derive(Clone, Copy)]
 struct PrefixedBlock {
     initial_prefix: &'static str,
     subsequent_prefix: &'static str,
 }
 
 impl PrefixedBlock {
+    /// Define a block with separate first/subsequent prefixes for wrapped content.
     const fn new(initial_prefix: &'static str, subsequent_prefix: &'static str) -> Self {
         Self {
             initial_prefix,
             subsequent_prefix,
         }
     }
 
+    /// Calculate available wrap width after accounting for prefix width at the given terminal size.
     fn wrap_width(self, total_width: u16) -> usize {
         let prefix_width = UnicodeWidthStr::width(self.initial_prefix)
             .max(UnicodeWidthStr::width(self.subsequent_prefix));
         usize::from(total_width).saturating_sub(prefix_width).max(1)
     }
 }
 
+/// Layout knobs for command continuation and output sections.
 #[derive(Clone, Copy)]
 struct ExecDisplayLayout {
     command_continuation: PrefixedBlock,
@@ -576,6 +601,7 @@ struct ExecDisplayLayout {
 }
 
 impl ExecDisplayLayout {
+    /// Create a layout tying together command/output wrap options for exec rendering.
     const fn new(
         command_continuation: PrefixedBlock,
         command_continuation_max_lines: usize,