Preserve reasoning order in TUI activity groups (#46565)

## Why

Reasoning summaries need to retain their position among computer actions and exploration commands, including when they arrive before a running call completes.

## What changed

Share ordered call and reasoning storage through `ActivityGroup` and let history cells accept reasoning through `append_reasoning`. Computer activity and exploration groups now retain reasoning while calls are running, showing it in order in the expanded transcript while keeping compact previews and raw output free of reasoning.

## Testing

Add regression snapshots for reasoning during a follow-up exploration command and for computer activity with multiple intervening and trailing summaries. Verify identical live and replayed computer activity across compact, expanded, and raw output.

GitOrigin-RevId: 92607b38fd4b0379616c1ae50b5da1068c8534c3
This commit is contained in:
Eric Traut
2026-09-19 01:46:14 +00:00
committed by copyberry
parent b948cfa88d
commit de33f2d22a
13 changed files with 293 additions and 71 deletions
@@ -451,7 +451,7 @@ impl ChatWidget {
&& cell.is_exploring_cell()
{
// Replayed commands have completion events without matching starts.
active.calls.extend(cell.calls);
active.group.calls.extend(cell.group.calls);
self.bump_active_cell_revision();
self.request_redraw();
} else {
+7 -13
View File
@@ -350,19 +350,13 @@ impl ChatWidget {
if !self.reasoning_summary_parts.is_empty() {
let reasoning_parts = std::mem::take(&mut self.reasoning_summary_parts);
let cell = history_cell::new_reasoning_summary_block(reasoning_parts, &self.config.cwd);
if let Some(exec) = self
.transcript
.active_cell
.as_mut()
.and_then(|cell| cell.as_any_mut().downcast_mut::<ExecCell>())
&& exec.is_exploring_cell()
&& !exec.is_active()
{
// Keep adjacent exploration grouped while retaining reasoning in transcript order.
exec.reasoning.push((exec.calls.len(), cell));
self.bump_active_cell_revision();
} else {
self.add_boxed_history(cell);
let result = match self.transcript.active_cell.as_mut() {
Some(active) => active.append_reasoning(cell),
None => Err(cell),
};
match result {
Ok(()) => self.bump_active_cell_revision(),
Err(cell) => self.add_boxed_history(cell),
}
}
self.reasoning_buffer.clear();
@@ -1,4 +1,5 @@
use super::*;
use pretty_assertions::assert_eq;
fn computer_item(
id: &str,
@@ -58,6 +59,70 @@ async fn computer_activity_live_and_replay_group_identically() {
);
}
#[tokio::test]
async fn computer_activity_keeps_reasoning_in_order_live_and_replayed() {
use codex_app_server_protocol::McpToolCallStatus;
let mut outputs = Vec::new();
for replay in [false, true] {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
for id in ["1", "2"] {
let item = computer_item(id, McpToolCallStatus::Completed);
if replay {
chat.replay_thread_item(
item.clone(),
"turn-1".to_string(),
ReplayKind::ThreadSnapshot,
);
} else {
chat.on_mcp_tool_call_started(computer_item(id, McpToolCallStatus::InProgress));
if id == "1" {
chat.on_mcp_tool_call_completed(item.clone());
}
}
for part in ["Inspecting", "Checking"] {
let summary = format!("{part} action {id}");
if replay {
chat.replay_thread_item(
AppServerThreadItem::Reasoning {
id: summary.clone(),
summary: vec![summary],
content: Vec::new(),
},
"turn-1".to_string(),
ReplayKind::ThreadSnapshot,
);
} else {
chat.on_agent_reasoning_delta(summary);
chat.on_agent_reasoning_final();
}
}
if !replay && id == "2" {
// Reasoning can arrive before a running call completes.
chat.on_mcp_tool_call_completed(item);
}
}
assert!(drain_insert_history(&mut rx).is_empty());
// A visible message commits the whole group, including trailing reasoning.
chat.prepare_assistant_message();
let cells = std::iter::from_fn(|| rx.try_recv().ok())
.filter_map(|event| match event {
AppEvent::InsertHistoryCell(cell) => Some(cell),
_ => None,
})
.collect::<Vec<_>>();
assert_eq!(cells.len(), 1);
let cell = &cells[0];
outputs.push(format!(
"Compact:\n{}\nExpanded:\n{}\nRaw:\n{}",
lines_to_single_string(&cell.display_lines(/*width*/ 80)),
lines_to_single_string(&cell.transcript_lines(/*width*/ 100)),
lines_to_single_string(&cell.raw_lines()),
));
}
assert_eq!(outputs[0], outputs[1]);
insta::assert_snapshot!(outputs[0]);
}
#[tokio::test]
async fn computer_activity_preserves_boundaries_and_expanded_output() {
use codex_app_server_protocol::McpToolCallStatus;
@@ -196,6 +196,22 @@ async fn exploration_nonzero_exits_remain_visible_beside_successful_reads() {
);
}
#[tokio::test]
async fn exploration_reasoning_during_followup_command_stays_ordered() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let first = begin_exec(&mut chat, "first", "ls");
end_exec(&mut chat, first, "file.txt\n", "", /*exit_code*/ 0);
chat.on_agent_reasoning_delta("Read the file next.".to_string());
chat.on_agent_reasoning_final();
let second = begin_exec(&mut chat, "second", "cat file.txt");
chat.on_agent_reasoning_delta("Waiting for the file.".to_string());
chat.on_agent_reasoning_final();
assert!(drain_insert_history(&mut rx).is_empty());
end_exec(&mut chat, second, "contents\n", "", /*exit_code*/ 0);
let transcript = chat.active_cell_transcript_lines(/*width*/ 80).unwrap();
insta::assert_snapshot!(lines_to_single_string(&transcript));
}
#[tokio::test]
async fn adjacent_exploration_groups_across_reasoning_live_and_replayed() {
let mut renders = Vec::new();
@@ -0,0 +1,29 @@
---
source: tui/src/chatwidget/tests/computer_activity_tests.rs
assertion_line: 116
expression: "outputs[0]"
---
Compact:
• Used computer · 2 actions
├ Inspect page 1
└ Inspect page 2
Expanded:
• Called cua_repl.js({"title":"Inspect page 1","code":"await cua.getState()"})
└ Full output for 1
• Inspecting action 1
• Checking action 1
• Called cua_repl.js({"title":"Inspect page 2","code":"await cua.getState()"})
└ Full output for 2
• Inspecting action 2
• Checking action 2
Raw:
• Called cua_repl.js({"title":"Inspect page 1","code":"await cua.getState()"})
└ Full output for 1
• Called cua_repl.js({"title":"Inspect page 2","code":"await cua.getState()"})
└ Full output for 2
@@ -0,0 +1,16 @@
---
source: tui/src/chatwidget/tests/exec_flow.rs
assertion_line: 212
expression: lines_to_single_string(&transcript)
---
$ ls
file.txt
✓ • 5ms
• Read the file next.
$ cat file.txt
contents
✓ • 5ms
• Waiting for the file.
+26 -16
View File
@@ -4,7 +4,7 @@
//! list/search commands. The chat widget relies on stable `call_id` matching to route progress and
//! 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).
//! Transcript-only reasoning stays inside completed exploration groups so it does not split their
//! Transcript-only reasoning stays inside exploration groups so it does not split their
//! compact display, while the expanded transcript retains its position between commands.
use std::borrow::Cow;
@@ -12,7 +12,7 @@ use std::time::Duration;
use std::time::Instant;
use super::live_output::LiveCommandOutput;
use crate::history_cell::HistoryCell;
use crate::history_cell::ActivityGroup;
use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource;
use codex_protocol::parse_command::ParsedCommand;
use itertools::Either;
@@ -77,17 +77,14 @@ pub(crate) struct ExecCall {
#[derive(Debug)]
pub(crate) struct ExecCell {
pub(crate) calls: Vec<ExecCall>,
/// Transcript-only reasoning, paired with the number of calls preceding it.
pub(crate) reasoning: Vec<(usize, Box<dyn HistoryCell>)>,
pub(crate) group: ActivityGroup<ExecCall>,
animations_enabled: bool,
}
impl ExecCell {
pub(crate) fn new(call: ExecCall, animations_enabled: bool) -> Self {
Self {
calls: vec![call],
reasoning: Vec::new(),
group: ActivityGroup::new(vec![call]),
animations_enabled,
}
}
@@ -111,7 +108,7 @@ impl ExecCell {
interaction_input,
};
if self.is_exploring_cell() && Self::is_exploring_call(&call) {
self.calls.push(call);
self.group.calls.push(call);
true
} else {
false
@@ -129,7 +126,13 @@ impl ExecCell {
output: CommandOutput,
duration: Duration,
) -> bool {
let Some(call) = self.calls.iter_mut().rev().find(|c| c.call_id == call_id) else {
let Some(call) = self
.group
.calls
.iter_mut()
.rev()
.find(|c| c.call_id == call_id)
else {
return false;
};
call.output = Some(output);
@@ -140,11 +143,11 @@ impl ExecCell {
pub(crate) fn should_flush(&self) -> bool {
// Exploration stays open for adjacent calls, including after a failed read/list/search.
!self.is_exploring_cell() && self.calls.iter().all(|c| c.duration.is_some())
!self.is_exploring_cell() && self.group.calls.iter().all(|c| c.duration.is_some())
}
pub(crate) fn mark_failed(&mut self) {
for call in self.calls.iter_mut() {
for call in self.group.calls.iter_mut() {
if call.duration.is_none() {
let elapsed = call
.start_time
@@ -160,15 +163,16 @@ impl ExecCell {
}
pub(crate) fn is_exploring_cell(&self) -> bool {
self.calls.iter().all(Self::is_exploring_call)
self.group.calls.iter().all(Self::is_exploring_call)
}
pub(crate) fn is_active(&self) -> bool {
self.calls.iter().any(|c| c.duration.is_none())
self.group.calls.iter().any(|c| c.duration.is_none())
}
pub(crate) fn active_start_time(&self) -> Option<Instant> {
self.calls
self.group
.calls
.iter()
.find(|c| c.duration.is_none())
.and_then(|c| c.start_time)
@@ -183,14 +187,20 @@ impl ExecCell {
}
pub(crate) fn iter_calls(&self) -> impl Iterator<Item = &ExecCall> {
self.calls.iter()
self.group.calls.iter()
}
pub(crate) fn append_output(&mut self, call_id: &str, chunk: &str) -> bool {
if chunk.is_empty() {
return false;
}
let Some(call) = self.calls.iter_mut().rev().find(|c| c.call_id == call_id) else {
let Some(call) = self
.group
.calls
.iter_mut()
.rev()
.find(|c| c.call_id == call_id)
else {
return false;
};
let output = call.output.get_or_insert_with(CommandOutput::default);
+16 -4
View File
@@ -195,6 +195,15 @@ fn activity_marker(start_time: Option<Instant>, animations_enabled: bool) -> Spa
}
impl HistoryCell for ExecCell {
fn append_reasoning(&mut self, cell: Box<dyn HistoryCell>) -> Result<(), Box<dyn HistoryCell>> {
if self.is_exploring_cell() {
self.group.push_reasoning(cell);
Ok(())
} else {
Err(cell)
}
}
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
if self.is_exploring_cell() {
self.exploring_display_lines(width)
@@ -237,7 +246,7 @@ impl ExecCell {
},
]));
let mut calls = self.calls.as_slice();
let mut calls = self.group.calls.as_slice();
let mut out_indented = Vec::new();
let nonzero_exit = |call: &ExecCall| {
call.duration
@@ -352,7 +361,7 @@ impl ExecCell {
}
fn command_display_lines(&self, width: u16) -> Vec<Line<'static>> {
let [call] = &self.calls.as_slice() else {
let [call] = &self.group.calls.as_slice() else {
panic!("Expected exactly one call in a command display cell");
};
let layout = EXEC_DISPLAY_LAYOUT;
@@ -908,7 +917,10 @@ mod tests {
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 output = cell.group.calls[0]
.output
.as_ref()
.expect("streamed output");
let agent = output_lines(
Some(output),
@@ -956,7 +968,7 @@ mod tests {
assert!(cell.append_output("call-id", &output));
let preview = cell.display_lines(/*width*/ 60);
cell.calls[0].start_time = None;
cell.group.calls[0].start_time = None;
cell.mark_failed();
let transcript = cell.transcript_lines(/*width*/ 60);
+3 -17
View File
@@ -17,9 +17,7 @@ use ratatui::prelude::*;
impl ExecCell {
pub(super) fn detailed_lines(&self, width: u16, mode: HistoryRenderMode) -> Vec<Line<'static>> {
let mut lines: Vec<Line<'static>> = vec![];
let mut reasoning = self.reasoning.iter().peekable();
for (i, call) in self.iter_calls().enumerate() {
self.group.transcript_lines(width, mode, |i, call, lines| {
if i > 0 {
lines.push("".into());
}
@@ -42,7 +40,7 @@ impl ExecCell {
.map(|line| ansi_escape_line(line.as_ref()))
{
let wrapped = adaptive_wrap_line(&unwrapped, wrap_opts.clone());
push_owned_lines(&wrapped, &mut lines);
push_owned_lines(&wrapped, lines);
}
}
if let Some(duration) = call.duration {
@@ -59,18 +57,6 @@ impl ExecCell {
lines.push(result);
}
}
if mode == HistoryRenderMode::Rich {
while let Some((_, cell)) =
reasoning.next_if(|(after_calls, _)| *after_calls == i + 1)
{
let reasoning_lines = cell.transcript_lines(width);
if !reasoning_lines.is_empty() {
lines.push("".into());
lines.extend(reasoning_lines);
}
}
}
}
lines
})
}
}
@@ -0,0 +1,52 @@
//! Ordered calls and transcript-only reasoning for adjacent activity groups.
//!
//! Tool cells own call matching and compact previews. Reasoning is attached after the calls
//! already started, preserving its position as they complete without breaking the compact display.
use super::HistoryCell;
use super::HistoryRenderMode;
use ratatui::text::Line;
#[derive(Debug)]
pub(crate) struct ActivityGroup<T> {
pub(crate) calls: Vec<T>,
reasoning: Vec<(usize, Box<dyn HistoryCell>)>,
}
impl<T> ActivityGroup<T> {
pub(crate) fn new(calls: Vec<T>) -> Self {
Self {
calls,
reasoning: Vec::new(),
}
}
pub(crate) fn push_reasoning(&mut self, cell: Box<dyn HistoryCell>) {
self.reasoning.push((self.calls.len(), cell));
}
pub(crate) fn transcript_lines(
&self,
width: u16,
mode: HistoryRenderMode,
mut render_call: impl FnMut(usize, &T, &mut Vec<Line<'static>>),
) -> Vec<Line<'static>> {
let mut lines = Vec::new();
let mut reasoning = self.reasoning.iter().peekable();
for (i, call) in self.calls.iter().enumerate() {
render_call(i, call, &mut lines);
if mode == HistoryRenderMode::Rich {
while let Some((_, cell)) =
reasoning.next_if(|(after_calls, _)| *after_calls == i + 1)
{
let reasoning_lines = cell.transcript_lines(width);
if !reasoning_lines.is_empty() {
lines.push("".into());
lines.extend(reasoning_lines);
}
}
}
}
lines
}
}
@@ -1,23 +1,33 @@
//! Compact adjacent computer calls without discarding their chronological transcript details.
//!
//! Only CUA calls enter this cell. Other history items and turn boundaries end the group.
//! CUA calls and intervening transcript-only reasoning share this cell. Visible history items
//! and turn boundaries end the group.
//! Preview selection favors failures and images, but never changes the order of retained rows.
use super::*;
#[derive(Debug, Default)]
#[derive(Debug)]
pub(crate) struct ComputerActivityCell {
calls: Vec<McpToolCallCell>,
group: ActivityGroup<McpToolCallCell>,
}
impl Default for ComputerActivityCell {
fn default() -> Self {
Self {
group: ActivityGroup::new(Vec::new()),
}
}
}
impl ComputerActivityCell {
pub(crate) fn start(&mut self, call: McpToolCallCell) {
if !self
.group
.calls
.iter()
.any(|existing| existing.call_id == call.call_id)
{
self.calls.push(call);
self.group.calls.push(call);
}
}
@@ -30,17 +40,17 @@ impl ComputerActivityCell {
) {
let id = call.call_id.clone();
self.start(call);
if let Some(call) = self.calls.iter_mut().find(|call| call.call_id == id) {
if let Some(call) = self.group.calls.iter_mut().find(|call| call.call_id == id) {
call.complete(duration, result);
}
}
pub(crate) fn is_active(&self) -> bool {
self.calls.iter().any(|call| call.result.is_none())
self.group.calls.iter().any(|call| call.result.is_none())
}
pub(crate) fn mark_failed(&mut self) {
for call in &mut self.calls {
for call in &mut self.group.calls {
if call.result.is_none() {
call.mark_failed();
}
@@ -91,20 +101,34 @@ fn error_preview(call: &McpToolCallCell) -> Option<&str> {
}
impl HistoryCell for ComputerActivityCell {
fn append_reasoning(&mut self, cell: Box<dyn HistoryCell>) -> Result<(), Box<dyn HistoryCell>> {
if self.group.calls.is_empty() {
Err(cell)
} else {
self.group.push_reasoning(cell);
Ok(())
}
}
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
if width == 0 {
return Vec::new();
}
let active = self.calls.iter().rposition(|call| call.result.is_none());
let active = self
.group
.calls
.iter()
.rposition(|call| call.result.is_none());
let failures = self
.group
.calls
.iter()
.filter(|call| call.success() == Some(false))
.count();
let count = self.calls.len();
let count = self.group.calls.len();
let bullet = active
.and_then(|index| {
let call = &self.calls[index];
let call = &self.group.calls[index];
activity_indicator(
Some(call.start_time),
MotionMode::from_animations_enabled(call.animations_enabled),
@@ -139,7 +163,7 @@ impl HistoryCell for ComputerActivityCell {
} else {
let mut indices = (0..count).collect::<Vec<_>>();
indices.sort_by_key(|&index| {
let call = &self.calls[index];
let call = &self.group.calls[index];
std::cmp::Reverse((call.success() == Some(false), has_image(call), index))
});
indices.truncate(if count > 3 { 2 } else { 3 });
@@ -149,7 +173,7 @@ impl HistoryCell for ComputerActivityCell {
let hidden = count - selected.len();
let visible = selected.len();
for (row_index, index) in selected.into_iter().enumerate() {
let call = &self.calls[index];
let call = &self.group.calls[index];
let title = call
.invocation
.arguments
@@ -193,18 +217,25 @@ impl HistoryCell for ComputerActivityCell {
}
fn transcript_lines(&self, width: u16) -> Vec<Line<'static>> {
self.calls
.iter()
.flat_map(|call| call.transcript_lines(width))
.collect()
self.group
.transcript_lines(width, HistoryRenderMode::Rich, |_, call, lines| {
lines.extend(call.transcript_lines(width));
})
}
fn raw_lines(&self) -> Vec<Line<'static>> {
plain_lines(self.transcript_lines(u16::MAX))
plain_lines(self.group.transcript_lines(
u16::MAX,
HistoryRenderMode::Raw,
|_, call, lines| {
lines.extend(call.transcript_lines(u16::MAX));
},
))
}
fn transcript_animation_tick(&self) -> Option<u64> {
self.calls
self.group
.calls
.iter()
.filter_map(HistoryCell::transcript_animation_tick)
.max()
@@ -42,7 +42,8 @@ fn computer_activity_active_and_interrupted() {
cell.mark_failed();
insta::assert_snapshot!("computer_activity_interrupted", render(&cell, /*width*/ 80));
assert_eq!(
cell.calls
cell.group
.calls
.iter()
.map(McpToolCallCell::success)
.collect::<Vec<_>>(),
@@ -125,7 +126,8 @@ fn computer_activity_completed_and_transport_errors_keep_full_details() {
);
cell.mark_failed();
assert_eq!(
cell.calls
cell.group
.calls
.iter()
.map(McpToolCallCell::success)
.collect::<Vec<_>>(),
+9
View File
@@ -10,6 +10,9 @@
//! bumps the active-cell revision tracked by `ChatWidget`, so the cache key changes whenever the
//! rendered transcript output can change.
mod activity_group;
pub(crate) use activity_group::ActivityGroup;
use crate::diff_model::FileChange;
use crate::diff_render::create_diff_summary;
use crate::exec_cell::CommandOutput;
@@ -182,6 +185,12 @@ pub(crate) fn plain_lines(lines: impl IntoIterator<Item = Line<'static>>) -> Vec
/// heights when they apply additional layout logic beyond what
/// `Paragraph::line_count` captures.
pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any {
/// Retain reasoning inside an activity group, or return it for normal insertion.
/// Call positions stay fixed as in-flight calls complete; other history cells decline it.
fn append_reasoning(&mut self, cell: Box<dyn HistoryCell>) -> Result<(), Box<dyn HistoryCell>> {
Err(cell)
}
/// Returns the logical lines for the main chat viewport.
fn display_lines(&self, width: u16) -> Vec<Line<'static>>;