mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Align persisted TUI activity groups and reasoning with live output (#46711)
## Why Persisted transcripts rendered tool calls individually and displayed reasoning differently from live replay. Enabling raw reasoning also replaced the summary instead of retaining it. ## What changed - Group computer activity and compatible exploration commands across intervening reasoning, retaining details for the expanded transcript. Respect visible content, command failure boundaries, and turn boundaries. - Separate paginated history at every turn boundary before filtering hidden items, while adding completion metadata only for successful turns. - Keep persisted reasoning out of the compact display, preserve hyperlinks in detailed rendering, and append enabled raw reasoning to the summary. Preserve standalone reasoning Markdown in transcript exports. - Add helpers to rejoin activity groups and attach reasoning across page boundaries using persisted item identities and turn adjacency. ## Testing Add regression coverage for reasoning replay parity, computer groups split across history pages, turn boundaries, and grouped terminal input. Update export and raw-reasoning tests to verify retained summaries and Markdown. GitOrigin-RevId: a716db571440f891cc2c6894ae93fbe0e149aed4
This commit is contained in:
Eric Traut
@@ -0,0 +1,65 @@
|
||||
//! Attach transparent page-boundary details to a freshly rebuilt activity group.
|
||||
|
||||
use crate::exec_cell::ExecCell;
|
||||
use crate::history_cell::ComputerActivityCell;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::ReasoningSummaryCell;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn is_hidden_activity_detail(cell: &Arc<dyn HistoryCell>) -> bool {
|
||||
cell.as_any()
|
||||
.downcast_ref::<ReasoningSummaryCell>()
|
||||
.is_some_and(ReasoningSummaryCell::is_transcript_only)
|
||||
}
|
||||
|
||||
/// Fold standalone reasoning using exact persisted identities from one turn.
|
||||
///
|
||||
/// An older page can arrive after the final reasoning page even with no newer group or live tail.
|
||||
/// Rebuild only known completed calls and share their immutable details; never infer adjacency
|
||||
/// from identical reasoning text or attach a detail from the next turn.
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn fold_trailing_activity_details(
|
||||
older: &Arc<dyn HistoryCell>,
|
||||
details: &[Arc<dyn HistoryCell>],
|
||||
turns: &[codex_app_server_protocol::Turn],
|
||||
) -> Option<Arc<dyn HistoryCell>> {
|
||||
let (mut ids, retained) =
|
||||
if let Some(group) = older.as_any().downcast_ref::<ComputerActivityCell>() {
|
||||
(group.call_ids().collect::<Vec<_>>(), &group.group.details)
|
||||
} else {
|
||||
let group = older.as_any().downcast_ref::<ExecCell>()?;
|
||||
if !group.is_exploring_cell() || group.should_flush() {
|
||||
return None;
|
||||
}
|
||||
(
|
||||
group
|
||||
.iter_calls()
|
||||
.map(|call| call.call_id.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
&group.group.details,
|
||||
)
|
||||
};
|
||||
let count = ids.len();
|
||||
for detail in details {
|
||||
let detail = detail.as_any().downcast_ref::<ReasoningSummaryCell>()?;
|
||||
if !detail.is_transcript_only() {
|
||||
return None;
|
||||
}
|
||||
ids.push(detail.source_item_id()?);
|
||||
}
|
||||
let items = super::computer_groups::adjacent_activity_items(&ids, turns)?;
|
||||
let mut retained = retained.clone();
|
||||
for detail in details {
|
||||
retained.push(count, Arc::clone(detail));
|
||||
}
|
||||
if older.as_any().is::<ComputerActivityCell>() {
|
||||
let mut group = super::computer_groups::completed_group(items)?;
|
||||
group.group.details = retained;
|
||||
Some(Arc::new(group))
|
||||
} else {
|
||||
let mut group = super::exploration_groups::completed_group(items)?;
|
||||
group.group.details = retained;
|
||||
Some(Arc::new(group))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,111 @@
|
||||
//! Assemble computer activity across hidden reasoning and repair history-page group boundaries.
|
||||
|
||||
use super::tools::McpHistory;
|
||||
use crate::history_cell::ComputerActivityCell;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::McpToolCallCell;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use std::sync::Arc;
|
||||
|
||||
pub(super) fn append(group: &mut ComputerActivityCell, call: McpHistory) {
|
||||
let cell = McpToolCallCell::new(call.id, call.invocation, /*animations_enabled*/ false);
|
||||
group.complete(cell, call.duration, call.result);
|
||||
}
|
||||
|
||||
/// Rebuild only an adjacent computer group, using its persisted IDs to verify the turn boundary.
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn join_computer_groups(
|
||||
older: &Arc<dyn HistoryCell>,
|
||||
newer: &Arc<dyn HistoryCell>,
|
||||
turns: &[Turn],
|
||||
) -> Option<Arc<dyn HistoryCell>> {
|
||||
let older = older.as_any().downcast_ref::<ComputerActivityCell>()?;
|
||||
let newer = newer.as_any().downcast_ref::<ComputerActivityCell>()?;
|
||||
let mut group = completed_group(adjacent_items(older, newer, turns)?)?;
|
||||
group.group.details = newer.group.details.clone();
|
||||
group
|
||||
.group
|
||||
.details
|
||||
.prepend(older.group.details.clone(), older.call_ids().count());
|
||||
Some(Arc::new(group))
|
||||
}
|
||||
|
||||
/// Restore only the older half; the caller retains the live half and its pending calls/timers.
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn older_computer_group(
|
||||
older: &dyn HistoryCell,
|
||||
newer: &ComputerActivityCell,
|
||||
turns: &[Turn],
|
||||
) -> Option<ComputerActivityCell> {
|
||||
let older = older.as_any().downcast_ref::<ComputerActivityCell>()?;
|
||||
let items = adjacent_items(older, newer, turns)?;
|
||||
let older_items = items
|
||||
.iter()
|
||||
.filter(|item| !matches!(item, ThreadItem::Reasoning { .. }))
|
||||
.take(older.call_ids().count())
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
let mut group = completed_group(&older_items)?;
|
||||
group.group.details = older.group.details.clone();
|
||||
Some(group)
|
||||
}
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
fn adjacent_items<'a>(
|
||||
older: &ComputerActivityCell,
|
||||
newer: &ComputerActivityCell,
|
||||
turns: &'a [Turn],
|
||||
) -> Option<&'a [ThreadItem]> {
|
||||
let ids = older.call_ids().chain(newer.call_ids()).collect::<Vec<_>>();
|
||||
adjacent_activity_items(&ids, turns)
|
||||
}
|
||||
|
||||
/// Validate source adjacency without mistaking invisible reasoning for an activity boundary.
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(super) fn adjacent_activity_items<'a>(
|
||||
ids: &[&str],
|
||||
turns: &'a [Turn],
|
||||
) -> Option<&'a [ThreadItem]> {
|
||||
let first = *ids.first()?;
|
||||
let (turn, start) = turns.iter().find_map(|turn| {
|
||||
turn.items
|
||||
.iter()
|
||||
.position(|item| item.id() == first)
|
||||
.map(|start| (turn, start))
|
||||
})?;
|
||||
let mut matched = 0;
|
||||
for (offset, item) in turn.items[start..].iter().enumerate() {
|
||||
if item.id() != ids[matched] {
|
||||
if matches!(item, ThreadItem::Reasoning { .. }) {
|
||||
continue;
|
||||
}
|
||||
return None;
|
||||
}
|
||||
matched += 1;
|
||||
if matched == ids.len() {
|
||||
return Some(&turn.items[start..=start + offset]);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(super) fn completed_group(items: &[ThreadItem]) -> Option<ComputerActivityCell> {
|
||||
let mut group = None;
|
||||
for item in items {
|
||||
if matches!(item, ThreadItem::Reasoning { .. }) {
|
||||
continue;
|
||||
}
|
||||
let call = McpHistory::from_item(item.clone())?;
|
||||
if !call.invocation.is_computer_activity() {
|
||||
return None;
|
||||
}
|
||||
append(group.get_or_insert_default(), call);
|
||||
}
|
||||
group
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "computer_groups_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,173 @@
|
||||
//! Page boundaries must not change the compact or expanded rendering of computer activity.
|
||||
|
||||
use super::*;
|
||||
use crate::thread_transcript::RawReasoningVisibility;
|
||||
use crate::thread_transcript::TranscriptCells;
|
||||
use crate::thread_transcript::thread_items_to_transcript_cells;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::TurnItemsView;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn computer(id: &str) -> ThreadItem {
|
||||
serde_json::from_value(serde_json::json!({
|
||||
"type": "mcpToolCall", "id": id, "server": "cua_repl", "tool": "js",
|
||||
"status": "completed", "arguments": {"title": format!("Inspect {id}"), "code": "await cua.getState()"},
|
||||
"appContext": null, "pluginId": null, "readOnlyHint": null,
|
||||
"result": {"content": [{"type": "text", "text": format!("Result {id}")}], "structuredContent": null},
|
||||
"error": null, "durationMs": 10
|
||||
})).unwrap()
|
||||
}
|
||||
|
||||
fn turn(id: &str, items: Vec<ThreadItem>) -> Turn {
|
||||
Turn {
|
||||
id: id.to_string(),
|
||||
items,
|
||||
items_view: TurnItemsView::Full,
|
||||
status: TurnStatus::Completed,
|
||||
error: None,
|
||||
started_at: None,
|
||||
completed_at: None,
|
||||
duration_ms: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn project(items: &[ThreadItem]) -> TranscriptCells {
|
||||
thread_items_to_transcript_cells(
|
||||
/*thread_id*/ None,
|
||||
&AbsolutePathBuf::current_dir().unwrap(),
|
||||
items.to_vec(),
|
||||
RawReasoningVisibility::Hidden,
|
||||
/*config*/ None,
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_page_split_retains_the_same_rich_computer_group() {
|
||||
let turns = vec![turn(
|
||||
"turn",
|
||||
vec![
|
||||
computer("first"),
|
||||
ThreadItem::Reasoning {
|
||||
id: "reasoning-first".to_owned(),
|
||||
summary: vec!["Inspect the second screen".to_owned()],
|
||||
content: Vec::new(),
|
||||
},
|
||||
computer("second"),
|
||||
computer("third"),
|
||||
ThreadItem::Reasoning {
|
||||
id: "reasoning-second".to_owned(),
|
||||
summary: vec!["Confirm the final screen".to_owned()],
|
||||
content: Vec::new(),
|
||||
},
|
||||
computer("last"),
|
||||
],
|
||||
)];
|
||||
let items = &turns[0].items;
|
||||
let expected = project(items);
|
||||
for split in 1..items.len() {
|
||||
let older = project(&items[..split]);
|
||||
let newer = project(&items[split..]);
|
||||
let detail_count = newer
|
||||
.iter()
|
||||
.take_while(|cell| {
|
||||
crate::thread_transcript::activity_pages::is_hidden_activity_detail(cell)
|
||||
})
|
||||
.count();
|
||||
let older_group = if detail_count > 0 {
|
||||
crate::thread_transcript::activity_pages::fold_trailing_activity_details(
|
||||
&older[0],
|
||||
&newer[..detail_count],
|
||||
&turns,
|
||||
)
|
||||
.unwrap()
|
||||
} else {
|
||||
Arc::clone(&older[0])
|
||||
};
|
||||
let joined = join_computer_groups(&older_group, &newer[detail_count], &turns).unwrap();
|
||||
assert_eq!(
|
||||
joined.display_hyperlink_lines(/*width*/ 30),
|
||||
expected[0].display_hyperlink_lines(/*width*/ 30)
|
||||
);
|
||||
assert_eq!(
|
||||
joined.transcript_hyperlink_lines(/*width*/ 30),
|
||||
expected[0].transcript_hyperlink_lines(/*width*/ 30)
|
||||
);
|
||||
assert_eq!(joined.raw_lines(), expected[0].raw_lines());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn successive_single_item_pages_reunite_without_duplicating_calls() {
|
||||
let turns = vec![turn(
|
||||
"turn",
|
||||
(0..6).map(|i| computer(&i.to_string())).collect(),
|
||||
)];
|
||||
let items = &turns[0].items;
|
||||
let mut joined = project(&items[5..]).remove(/*index*/ 0);
|
||||
for item in items[..5].iter().rev() {
|
||||
let older = project(std::slice::from_ref(item));
|
||||
joined = join_computer_groups(&older[0], &joined, &turns).unwrap();
|
||||
}
|
||||
let expected = project(items);
|
||||
assert_eq!(
|
||||
joined.transcript_lines(/*width*/ 80),
|
||||
expected[0].transcript_lines(/*width*/ 80)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn joins_respect_actual_turns_and_intervening_items() {
|
||||
let first = computer("first");
|
||||
let last = computer("last");
|
||||
let older = project(std::slice::from_ref(&first));
|
||||
let newer = project(std::slice::from_ref(&last));
|
||||
let turns = vec![
|
||||
turn("first-turn", vec![first.clone()]),
|
||||
turn("last-turn", vec![last.clone()]),
|
||||
];
|
||||
assert!(join_computer_groups(&older[0], &newer[0], &turns).is_none());
|
||||
let grouped = project(&[
|
||||
first.clone(),
|
||||
ThreadItem::Sleep(codex_app_server_protocol::SleepItem {
|
||||
id: "sleep".into(),
|
||||
duration_ms: 10,
|
||||
}),
|
||||
last.clone(),
|
||||
]);
|
||||
let adjacent = project(&[first.clone(), last.clone()]);
|
||||
assert_eq!(grouped.len(), 1);
|
||||
assert_eq!(
|
||||
grouped[0].transcript_lines(/*width*/ 80),
|
||||
adjacent[0].transcript_lines(/*width*/ 80)
|
||||
);
|
||||
let mut turns = vec![turn(
|
||||
"turn",
|
||||
vec![
|
||||
first,
|
||||
ThreadItem::Reasoning {
|
||||
id: "reasoning".to_string(),
|
||||
summary: Vec::new(),
|
||||
content: Vec::new(),
|
||||
},
|
||||
last,
|
||||
],
|
||||
)];
|
||||
let joined = join_computer_groups(&older[0], &newer[0], &turns)
|
||||
.expect("hidden reasoning retains the adjacent computer group");
|
||||
let expected = project(&turns[0].items);
|
||||
assert_eq!(
|
||||
joined.transcript_lines(/*width*/ 80),
|
||||
expected[0].transcript_lines(/*width*/ 80),
|
||||
);
|
||||
turns[0].items[1] = ThreadItem::AgentMessage {
|
||||
id: "message".to_string(),
|
||||
text: "Visible boundary".to_string(),
|
||||
phase: None,
|
||||
memory_citation: None,
|
||||
delivery: None,
|
||||
questions: None,
|
||||
};
|
||||
assert!(join_computer_groups(&older[0], &newer[0], &turns).is_none());
|
||||
}
|
||||
@@ -0,0 +1,82 @@
|
||||
//! Apply live exploration compatibility to replay and repair groups split by history pages.
|
||||
|
||||
use super::tools::CommandHistory;
|
||||
use crate::exec_cell::ExecCell;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn join_exploration_groups(
|
||||
older: &Arc<dyn HistoryCell>,
|
||||
newer: &Arc<dyn HistoryCell>,
|
||||
turns: &[Turn],
|
||||
) -> Option<Arc<dyn HistoryCell>> {
|
||||
let older = older.as_any().downcast_ref::<ExecCell>()?;
|
||||
let newer = newer.as_any().downcast_ref::<ExecCell>()?;
|
||||
let items = adjacent_items(older, newer, turns)?;
|
||||
let mut group = completed_group(items)?;
|
||||
group.group.details = newer.group.details.clone();
|
||||
group
|
||||
.group
|
||||
.details
|
||||
.prepend(older.group.details.clone(), older.group.calls.len());
|
||||
Some(Arc::new(group))
|
||||
}
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(crate) fn older_exploration_group(
|
||||
older: &dyn HistoryCell,
|
||||
newer: &ExecCell,
|
||||
turns: &[Turn],
|
||||
) -> Option<ExecCell> {
|
||||
let older = older.as_any().downcast_ref::<ExecCell>()?;
|
||||
let items = adjacent_items(older, newer, turns)?;
|
||||
let older_items = items
|
||||
.iter()
|
||||
.filter(|item| !matches!(item, ThreadItem::Reasoning { .. }))
|
||||
.take(older.group.calls.len())
|
||||
.cloned()
|
||||
.collect::<Vec<_>>();
|
||||
let mut group = completed_group(&older_items)?;
|
||||
group.group.details = older.group.details.clone();
|
||||
Some(group)
|
||||
}
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
fn adjacent_items<'a>(
|
||||
older: &ExecCell,
|
||||
newer: &ExecCell,
|
||||
turns: &'a [Turn],
|
||||
) -> Option<&'a [ThreadItem]> {
|
||||
if !older.is_exploring_cell() || older.should_flush() || !newer.is_exploring_cell() {
|
||||
return None;
|
||||
}
|
||||
let ids = older
|
||||
.iter_calls()
|
||||
.chain(newer.iter_calls())
|
||||
.map(|call| call.call_id.as_str())
|
||||
.collect::<Vec<_>>();
|
||||
super::computer_groups::adjacent_activity_items(&ids, turns)
|
||||
}
|
||||
|
||||
#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")]
|
||||
pub(super) fn completed_group(items: &[ThreadItem]) -> Option<ExecCell> {
|
||||
let mut group: Option<ExecCell> = None;
|
||||
for item in items {
|
||||
if matches!(item, ThreadItem::Reasoning { .. }) {
|
||||
continue;
|
||||
}
|
||||
let call = CommandHistory::from_item(item.clone())?.into_cell();
|
||||
if !call.is_exploring_cell() {
|
||||
return None;
|
||||
}
|
||||
if let Some(group) = &mut group {
|
||||
group.append_completed(call).ok()?;
|
||||
} else {
|
||||
group = Some(call);
|
||||
}
|
||||
}
|
||||
group
|
||||
}
|
||||
Reference in New Issue
Block a user