mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Fix reasoning summary restoration in the TUI (#59698)
## Why Reasoning summaries can arrive with item start or completion events rather than streamed deltas. The TUI can miss or truncate these summaries, and replay after a voice handoff can hide reasoning that started before the handoff. ## What changed - Initialize reasoning from item start payloads and reconcile live and replayed completions with their full summaries. Preserve streamed text when completion payloads are empty, and continue honoring `show_raw_agent_reasoning`. - Retain active reasoning payloads across refreshes without resetting an already active item when its start event is replayed. - Preserve the typed origin of reasoning started before a voice handoff and filter later delegated reasoning starts from buffered replay. ## Testing Extend regression coverage for summaries supplied at start or completion, partial streamed summaries, active reasoning after refresh, and replay that retains pre-handoff summaries while hiding delegated reasoning even after the handoff marker is evicted. GitOrigin-RevId: 724b9fc58e10f4d87dabd3d9930e252270855a4f
This commit is contained in:
@@ -38,6 +38,14 @@ impl ReasoningReplay {
|
||||
}
|
||||
|
||||
pub(super) fn before_event(&mut self, event: &ThreadBufferedEvent, chat: &mut ChatWidget) {
|
||||
// The store retains only typed reasoning starts. Restore their origin even if a
|
||||
// later handoff has already marked the whole turn as delegated in the new widget.
|
||||
if let ThreadBufferedEvent::Notification(notification) = event
|
||||
&& let ServerNotification::ItemStarted(started) = notification.as_ref()
|
||||
&& let ThreadItemPayload::Reasoning { id, .. } = &started.item.payload
|
||||
{
|
||||
chat.remember_typed_reasoning_item(started.turn_id.clone(), id);
|
||||
}
|
||||
if self.started.as_ref().is_some_and(|started| {
|
||||
matches!(event, ThreadBufferedEvent::Notification(notification) if match notification.as_ref() {
|
||||
ServerNotification::ItemStarted(item) => {
|
||||
|
||||
@@ -58,7 +58,7 @@ async fn refreshed_active_reasoning_accepts_later_deltas_and_complete_summary()
|
||||
turn.id.clone(),
|
||||
ThreadItemPayload::Reasoning {
|
||||
id: "reasoning".into(),
|
||||
summary: Vec::new(),
|
||||
summary: vec!["**First heading**".into()],
|
||||
content: Vec::new(),
|
||||
},
|
||||
/*started_at_ms*/ 0,
|
||||
@@ -69,7 +69,7 @@ async fn refreshed_active_reasoning_accepts_later_deltas_and_complete_summary()
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: turn.id.clone(),
|
||||
item_id: "reasoning".into(),
|
||||
delta: "**First heading**\nOriginal analysis paragraph".into(),
|
||||
delta: "\nOriginal analysis paragraph".into(),
|
||||
summary_index: 0,
|
||||
},
|
||||
));
|
||||
@@ -111,6 +111,9 @@ async fn refreshed_active_reasoning_accepts_later_deltas_and_complete_summary()
|
||||
}
|
||||
assert!(snapshot.active_reasoning_item.is_some());
|
||||
app.replay_thread_snapshot(snapshot, /*resume_restored_queue*/ false);
|
||||
if refresh && !refreshed_item_is_partial && !voice_handoff {
|
||||
assert!(render_bottom_popup(&app.chat_widget, /*width*/ 80).contains("First heading"));
|
||||
}
|
||||
app.chat_widget.handle_server_notification(
|
||||
ServerNotification::ReasoningSummaryTextDelta(ReasoningSummaryTextDeltaNotification {
|
||||
thread_id: thread_id.to_string(),
|
||||
@@ -472,7 +475,7 @@ fn snapshot_keeps_typed_item_completed_after_voice_handoff() {
|
||||
}],
|
||||
};
|
||||
let mut store = ThreadEventStore::new(/*capacity*/ 8);
|
||||
for item in [typed.clone(), marker, private.clone()] {
|
||||
for item in [typed.clone(), marker.clone(), private.clone()] {
|
||||
store.push_notification(ServerNotification::ItemStarted(
|
||||
codex_app_server_protocol::ItemStartedNotification::new(
|
||||
"thread".into(),
|
||||
@@ -493,45 +496,99 @@ fn snapshot_keeps_typed_item_completed_after_voice_handoff() {
|
||||
),
|
||||
));
|
||||
}
|
||||
let completed = store
|
||||
let visible = store
|
||||
.snapshot()
|
||||
.events
|
||||
.into_iter()
|
||||
.filter_map(|event| match event {
|
||||
ThreadBufferedEvent::Notification(notification) => match *notification {
|
||||
ServerNotification::ItemStarted(n) => Some(n.item.payload),
|
||||
ServerNotification::ItemCompleted(n) => Some(n.item.payload),
|
||||
_ => None,
|
||||
},
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(completed, vec![typed]);
|
||||
assert_eq!(visible, vec![typed.clone(), marker, typed]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn evicted_voice_marker_survives_widget_snapshot_for_late_reasoning() {
|
||||
async fn voice_handoff_replay_keeps_public_reasoning_and_hides_private_reasoning() {
|
||||
let thread_id = ThreadId::new();
|
||||
let mut store = ThreadEventStore::new(/*capacity*/ 1);
|
||||
store.push_notification(ServerNotification::ItemStarted(
|
||||
codex_app_server_protocol::ItemStartedNotification::new(
|
||||
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
||||
let mut store = ThreadEventStore::new_with_session(
|
||||
/*capacity*/ 5,
|
||||
test_thread_session(thread_id, app.config.cwd.to_path_buf()),
|
||||
vec![test_turn("voice-turn", TurnStatus::InProgress, Vec::new())],
|
||||
);
|
||||
let reasoning = |id: &str, summary: Vec<String>| ThreadItemPayload::Reasoning {
|
||||
id: id.into(),
|
||||
summary,
|
||||
content: Vec::new(),
|
||||
};
|
||||
let start = |item| {
|
||||
ServerNotification::ItemStarted(ItemStartedNotification::new(
|
||||
thread_id.to_string(),
|
||||
"voice-turn".into(),
|
||||
ThreadItemPayload::UserMessage {
|
||||
id: "user".into(),
|
||||
client_id: None,
|
||||
content: vec![codex_app_server_protocol::UserInput::Text {
|
||||
text: "<realtime_delegation><input>question</input></realtime_delegation>"
|
||||
.into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
},
|
||||
item,
|
||||
/*started_at_ms*/ 0,
|
||||
))
|
||||
};
|
||||
let public = reasoning("typed", Vec::new());
|
||||
store.push_notification(start(public.clone()));
|
||||
store.push_notification(ServerNotification::ReasoningSummaryTextDelta(
|
||||
ReasoningSummaryTextDeltaNotification {
|
||||
thread_id: thread_id.to_string(),
|
||||
turn_id: "voice-turn".into(),
|
||||
item_id: "typed".into(),
|
||||
delta: "**Public progress**".into(),
|
||||
summary_index: 0,
|
||||
},
|
||||
));
|
||||
store.push_notification(ServerNotification::ItemCompleted(
|
||||
ItemCompletedNotification::new(
|
||||
thread_id.to_string(),
|
||||
"voice-turn".into(),
|
||||
public,
|
||||
/*started_at_ms*/ None,
|
||||
/*completed_at_ms*/ 0,
|
||||
),
|
||||
));
|
||||
store.push_notification(start(ThreadItemPayload::UserMessage {
|
||||
id: "user".into(),
|
||||
client_id: None,
|
||||
content: vec![AppServerUserInput::Text {
|
||||
text: "<realtime_delegation><input>question</input></realtime_delegation>".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}));
|
||||
store.push_notification(start(reasoning(
|
||||
"private",
|
||||
vec!["private after handoff".into()],
|
||||
)));
|
||||
app.replay_thread_snapshot(store.snapshot(), /*resume_restored_queue*/ false);
|
||||
let activity = render_bottom_popup(&app.chat_widget, /*width*/ 80);
|
||||
assert!(activity.contains("Public progress"), "{activity}");
|
||||
assert!(!activity.contains("private"), "{activity}");
|
||||
let summaries: Vec<_> = std::iter::from_fn(|| events.try_recv().ok())
|
||||
.filter_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell)
|
||||
if cell
|
||||
.as_any()
|
||||
.is::<crate::history_cell::ReasoningSummaryCell>() =>
|
||||
{
|
||||
Some(lines_to_single_string(&cell.transcript_lines(/*width*/ 80)))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
insta::assert_snapshot!(summaries.join("\n"), @"• Public progress");
|
||||
|
||||
// Force the handoff out of the buffer before checking late private updates.
|
||||
store.capacity = 1;
|
||||
store.push_notification(delta(&thread_id.to_string(), "voice-turn", "private"));
|
||||
let snapshot = store.snapshot();
|
||||
assert!(snapshot.events.is_empty());
|
||||
assert!(snapshot.turns.is_empty());
|
||||
assert_eq!(snapshot.delegated_turns, vec!["voice-turn"]);
|
||||
|
||||
let mut app = make_test_app().await;
|
||||
|
||||
@@ -268,23 +268,18 @@ impl ThreadEventStore {
|
||||
started @ codex_app_server_protocol::ItemStartedNotification {
|
||||
item:
|
||||
codex_app_server_protocol::ThreadItem {
|
||||
payload: ThreadItemPayload::Reasoning { id, .. },
|
||||
payload: ThreadItemPayload::Reasoning { .. },
|
||||
..
|
||||
},
|
||||
..
|
||||
},
|
||||
) if !self.is_realtime_delegated_turn(&started.turn_id) => {
|
||||
self.active_reasoning_item =
|
||||
Some(codex_app_server_protocol::ItemStartedNotification::new(
|
||||
started.thread_id.clone(),
|
||||
started.turn_id.clone(),
|
||||
ThreadItemPayload::Reasoning {
|
||||
id: id.clone(),
|
||||
summary: Vec::new(),
|
||||
content: Vec::new(),
|
||||
},
|
||||
started.started_at_ms,
|
||||
));
|
||||
) => {
|
||||
// Only typed reasoning starts belong in the replay buffer. Decide while
|
||||
// the handoff order is known, before bounded eviction can obscure it.
|
||||
if self.is_realtime_delegated_turn(&started.turn_id) {
|
||||
return;
|
||||
}
|
||||
self.active_reasoning_item = Some(started.clone());
|
||||
}
|
||||
ServerNotification::ItemCompleted(completed)
|
||||
if matches!(&completed.item.payload, ThreadItemPayload::Reasoning { .. })
|
||||
@@ -471,6 +466,7 @@ impl ThreadEventStore {
|
||||
let key = (n.turn_id.clone(), id.clone());
|
||||
if voice_started.contains(&n.turn_id) {
|
||||
typed_items.remove(&key);
|
||||
return false;
|
||||
} else {
|
||||
typed_items.insert(key);
|
||||
}
|
||||
|
||||
@@ -582,21 +582,24 @@ impl ChatWidget {
|
||||
ThreadItemPayload::AgentMessage { id, .. } if replay_kind.is_none() => {
|
||||
self.is_realtime_delegated_agent_item(¬ification.turn_id, &id);
|
||||
}
|
||||
ThreadItemPayload::Reasoning { id, .. } => {
|
||||
ThreadItemPayload::Reasoning {
|
||||
id,
|
||||
summary,
|
||||
content,
|
||||
} => {
|
||||
if replay_kind.is_none()
|
||||
&& !self.is_realtime_delegated_reasoning_turn(¬ification.turn_id)
|
||||
{
|
||||
// A later voice handoff can steer this turn without making an
|
||||
// already-started typed reasoning item private.
|
||||
self.realtime_conversation
|
||||
.agent_items
|
||||
.entry((notification.turn_id.clone(), id.clone()))
|
||||
.or_insert(realtime::RealtimeAgentItemOrigin::Typed);
|
||||
self.remember_typed_reasoning_item(notification.turn_id.clone(), &id);
|
||||
}
|
||||
if !matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages))
|
||||
&& !self.is_realtime_delegated_reasoning_item(¬ification.turn_id, &id)
|
||||
&& self.status_state.reasoning_item_id.as_ref() != Some(&id)
|
||||
{
|
||||
self.on_reasoning_item_started(id);
|
||||
self.replace_reasoning_parts(summary, content);
|
||||
}
|
||||
}
|
||||
ThreadItemPayload::ContextCompaction { id }
|
||||
|
||||
@@ -762,6 +762,13 @@ impl ChatWidget {
|
||||
.retain(|saved| saved != turn_id);
|
||||
}
|
||||
|
||||
pub(crate) fn remember_typed_reasoning_item(&mut self, turn_id: String, item_id: &str) {
|
||||
self.realtime_conversation
|
||||
.agent_items
|
||||
.entry((turn_id, item_id.to_owned()))
|
||||
.or_insert(RealtimeAgentItemOrigin::Typed);
|
||||
}
|
||||
|
||||
pub(crate) fn remember_realtime_delegated_reasoning_turn(&mut self, turn_id: &str) {
|
||||
if turn_id.len() > 512
|
||||
|| self
|
||||
|
||||
@@ -15,27 +15,19 @@ impl ChatWidget {
|
||||
parts: Option<(Vec<String>, Vec<String>)>,
|
||||
) {
|
||||
let turn_id = started.turn_id;
|
||||
let id = started.item.id().to_string();
|
||||
self.realtime_conversation
|
||||
.agent_items
|
||||
.entry((turn_id, id.clone()))
|
||||
.or_insert(realtime::RealtimeAgentItemOrigin::Typed);
|
||||
let ThreadItemPayload::Reasoning {
|
||||
id,
|
||||
summary,
|
||||
content,
|
||||
} = started.item.payload
|
||||
else {
|
||||
return;
|
||||
};
|
||||
self.remember_typed_reasoning_item(turn_id, &id);
|
||||
self.on_reasoning_item_started(id);
|
||||
self.status_state.reasoning_recovered_after_refresh = true;
|
||||
self.replace_reasoning_parts(summary, content);
|
||||
if let Some((summary, content)) = parts {
|
||||
let reasoning_parts = summary.into_iter().chain(
|
||||
self.config
|
||||
.show_raw_agent_reasoning
|
||||
.then_some(content)
|
||||
.into_iter()
|
||||
.flatten(),
|
||||
);
|
||||
for (index, delta) in reasoning_parts.enumerate() {
|
||||
if index > 0 {
|
||||
self.on_reasoning_section_break();
|
||||
}
|
||||
self.on_agent_reasoning_delta(delta);
|
||||
}
|
||||
self.replace_reasoning_parts(summary, content);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -360,32 +352,10 @@ impl ChatWidget {
|
||||
summary,
|
||||
content,
|
||||
} => {
|
||||
let recover_completion = self.status_state.reasoning_recovered_after_refresh
|
||||
&& (!summary.is_empty()
|
||||
|| (self.config.show_raw_agent_reasoning && !content.is_empty()));
|
||||
if from_replay || recover_completion {
|
||||
if from_replay {
|
||||
self.on_reasoning_item_started(id);
|
||||
} else {
|
||||
// A refreshed snapshot can omit the active item and its earlier deltas.
|
||||
// Reconcile with the complete item before committing the transcript.
|
||||
self.reasoning_buffer.clear();
|
||||
self.reasoning_summary_parts.clear();
|
||||
}
|
||||
let reasoning_parts = summary.into_iter().chain(
|
||||
self.config
|
||||
.show_raw_agent_reasoning
|
||||
.then_some(content)
|
||||
.into_iter()
|
||||
.flatten(),
|
||||
);
|
||||
for (index, delta) in reasoning_parts.enumerate() {
|
||||
if index > 0 {
|
||||
self.on_reasoning_section_break();
|
||||
}
|
||||
self.on_agent_reasoning_delta(delta);
|
||||
}
|
||||
if from_replay {
|
||||
self.on_reasoning_item_started(id);
|
||||
}
|
||||
self.replace_reasoning_parts(summary, content);
|
||||
self.on_agent_reasoning_final();
|
||||
}
|
||||
item @ ThreadItemPayload::CommandExecution {
|
||||
|
||||
@@ -115,8 +115,6 @@ pub(super) struct StatusState {
|
||||
pub(super) reasoning_item_id: Option<String>,
|
||||
/// An in-progress snapshot can omit the start of its next live reasoning update.
|
||||
pub(super) reasoning_resume_turn_id: Option<String>,
|
||||
/// A restored stream may have missed earlier deltas; use its complete item at finalization.
|
||||
pub(super) reasoning_recovered_after_refresh: bool,
|
||||
pub(super) compaction: Option<super::compaction::ActiveCompaction>,
|
||||
pub(super) current_status: StatusIndicatorState,
|
||||
pub(super) pending_guardian_review_status: PendingGuardianReviewStatus,
|
||||
@@ -133,7 +131,6 @@ impl Default for StatusState {
|
||||
Self {
|
||||
reasoning_item_id: None,
|
||||
reasoning_resume_turn_id: None,
|
||||
reasoning_recovered_after_refresh: false,
|
||||
compaction: None,
|
||||
current_status: StatusIndicatorState::working(),
|
||||
pending_guardian_review_status: PendingGuardianReviewStatus::default(),
|
||||
|
||||
@@ -41,7 +41,6 @@ impl ChatWidget {
|
||||
return;
|
||||
}
|
||||
self.status_state.reasoning_item_id = Some(id);
|
||||
self.status_state.reasoning_recovered_after_refresh = false;
|
||||
self.reasoning_buffer.clear();
|
||||
self.reasoning_summary_parts.clear();
|
||||
self.restore_reasoning_status_header();
|
||||
@@ -408,6 +407,26 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn replace_reasoning_parts(
|
||||
&mut self,
|
||||
mut summary: Vec<String>,
|
||||
content: Vec<String>,
|
||||
) {
|
||||
if self.config.show_raw_agent_reasoning {
|
||||
summary.extend(content);
|
||||
}
|
||||
// Empty completions follow some streamed items. Preserve their deltas.
|
||||
if summary.iter().all(String::is_empty) {
|
||||
return;
|
||||
}
|
||||
self.reasoning_buffer.clear();
|
||||
self.reasoning_summary_parts.clear();
|
||||
for part in summary {
|
||||
self.on_reasoning_section_break();
|
||||
self.on_agent_reasoning_delta(part);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn on_agent_reasoning_delta(&mut self, delta: String) {
|
||||
// Accumulate the current reasoning block for history and activity text.
|
||||
self.reasoning_buffer.push_str(&delta);
|
||||
@@ -486,7 +505,6 @@ impl ChatWidget {
|
||||
// Keep the last useful summary through tools and later empty items.
|
||||
self.status_state.reasoning_item_id = None;
|
||||
self.status_state.reasoning_resume_turn_id = None;
|
||||
self.status_state.reasoning_recovered_after_refresh = false;
|
||||
self.reasoning_summary_parts.clear();
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
@@ -37,9 +37,19 @@ fn complete(chat: &mut ChatWidget, id: &str) {
|
||||
async fn reasoning_status_accepts_bold_text_with_a_plain_suffix() {
|
||||
let (mut chat, _rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.on_task_started();
|
||||
handle_agent_reasoning_started(&mut chat, "reasoning");
|
||||
|
||||
delta(&mut chat, "reasoning", "**Checking tests");
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::ItemStarted(ItemStartedNotification::new(
|
||||
"thread-1".into(),
|
||||
"turn-1".into(),
|
||||
AppServerThreadItemPayload::Reasoning {
|
||||
id: "reasoning".into(),
|
||||
summary: vec!["**Checking tests".into()],
|
||||
content: Vec::new(),
|
||||
},
|
||||
/*started_at_ms*/ 0,
|
||||
)),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
let before_close = chat
|
||||
.bottom_pane
|
||||
.status_widget()
|
||||
@@ -271,20 +281,33 @@ async fn completed_reasoning_stays_in_expanded_transcript_for_live_and_replay()
|
||||
"**Tracing terminal components**\n\nThe playback clock preserves elapsed time.",
|
||||
),
|
||||
] {
|
||||
let item = AppServerThreadItemPayload::Reasoning {
|
||||
id: id.into(),
|
||||
summary: vec![summary.into()],
|
||||
content: vec!["private reasoning".into()],
|
||||
};
|
||||
if replay {
|
||||
chat.handle_thread_item(
|
||||
AppServerThreadItemPayload::Reasoning {
|
||||
id: id.to_string(),
|
||||
summary: vec![summary.to_string()],
|
||||
content: Vec::new(),
|
||||
},
|
||||
item,
|
||||
"turn-1".to_string(),
|
||||
ThreadItemRenderSource::Replay(ReplayKind::ThreadSnapshot),
|
||||
);
|
||||
} else {
|
||||
handle_agent_reasoning_started(&mut chat, id);
|
||||
delta(&mut chat, id, summary);
|
||||
complete(&mut chat, id);
|
||||
// Providers can stream a partial summary or supply it only at completion.
|
||||
if id == "first" {
|
||||
delta(&mut chat, id, "**Inspecting");
|
||||
}
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::ItemCompleted(ItemCompletedNotification::new(
|
||||
"thread-1".into(),
|
||||
"turn-1".into(),
|
||||
item,
|
||||
/*started_at_ms*/ None,
|
||||
/*completed_at_ms*/ 0,
|
||||
)),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
}
|
||||
}
|
||||
let mut compact = Vec::new();
|
||||
|
||||
Reference in New Issue
Block a user