From 02d2c8231c753e928ea41441e5b53b7145b5a01b Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 4 Apr 2026 11:52:19 -0700 Subject: [PATCH] codex: address PR review feedback (#16803) Backfill a missing reasoning-summary prefix from the completed item snapshot when late suffix deltas streamed normally after earlier orphan deltas were dropped. --- codex-rs/tui/src/chatwidget.rs | 22 ++++++-- ...missing_prefix_before_streamed_suffix.snap | 5 ++ .../src/chatwidget/tests/history_replay.rs | 56 +++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix.snap diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e533d47671..32b6ae2fdb 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6016,10 +6016,13 @@ impl ChatWidget { ThreadItem::Reasoning { summary, content, .. } => { - let should_backfill_reasoning = from_replay - || (self.reasoning_buffer.trim().is_empty() - && self.full_reasoning_buffer.trim().is_empty()); - if should_backfill_reasoning { + let mut completed_reasoning = summary.join(""); + if self.config.show_raw_agent_reasoning { + completed_reasoning.push_str(&content.join("")); + } + let streamed_reasoning = + format!("{}{}", self.full_reasoning_buffer, self.reasoning_buffer); + if from_replay || streamed_reasoning.trim().is_empty() { for delta in summary { self.on_agent_reasoning_delta(delta); } @@ -6028,6 +6031,17 @@ impl ChatWidget { self.on_agent_reasoning_delta(delta); } } + } else if !completed_reasoning.is_empty() + && completed_reasoning != streamed_reasoning + && let Some(missing_prefix) = completed_reasoning + .strip_suffix(&streamed_reasoning) + .filter(|missing_prefix| !missing_prefix.is_empty()) + { + // If early summary deltas were dropped before the item became active, prepend + // the missing prefix from the completed item snapshot and keep the streamed + // suffix so we still avoid double-rendering. + self.full_reasoning_buffer = + format!("{missing_prefix}{}", self.full_reasoning_buffer); } self.on_agent_reasoning_final(); } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix.snap new file mode 100644 index 0000000000..dce699b7bd --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/history_replay.rs +expression: rendered +--- +• prefix suffix diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index f5df21f82f..f7fb4e219a 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -770,6 +770,62 @@ async fn live_reasoning_item_completed_renders_summary_without_prior_delta() { ); } +#[tokio::test] +async fn live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.show_welcome_banner = false; + + chat.handle_server_notification( + ServerNotification::TurnStarted(TurnStartedNotification { + thread_id: "thread-1".to_string(), + turn: AppServerTurn { + id: "turn-1".to_string(), + items: Vec::new(), + status: AppServerTurnStatus::InProgress, + error: None, + }, + }), + /*replay_kind*/ None, + ); + let _ = drain_insert_history(&mut rx); + + chat.handle_server_notification( + ServerNotification::ReasoningSummaryTextDelta(ReasoningSummaryTextDeltaNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + item_id: "reasoning-1".to_string(), + delta: "suffix".to_string(), + summary_index: 0, + }), + /*replay_kind*/ None, + ); + + chat.handle_server_notification( + ServerNotification::ItemCompleted(ItemCompletedNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + item: AppServerThreadItem::Reasoning { + id: "reasoning-1".to_string(), + summary: vec!["prefix suffix".to_string()], + content: Vec::new(), + }, + }), + /*replay_kind*/ None, + ); + + let rendered = match rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => { + lines_to_single_string(&cell.transcript_lines(/*width*/ 80)) + } + other => panic!("expected InsertHistoryCell, got {other:?}"), + }; + assert_eq!(rendered.matches("prefix suffix").count(), 1); + assert_chatwidget_snapshot!( + "live_reasoning_item_completed_backfills_missing_prefix_before_streamed_suffix", + rendered + ); +} + #[tokio::test] async fn replayed_turn_started_does_not_mark_task_running() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;