diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 207f75a7..e9453a24 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1606,6 +1606,7 @@ async fn handle_response_item( for item in content { let text = match item { ReasoningItemContent::ReasoningText { text } => text, + ReasoningItemContent::Text { text } => text, }; let event = Event { id: sub_id.to_string(), diff --git a/codex-rs/core/src/models.rs b/codex-rs/core/src/models.rs index e052bc43..da1e31b7 100644 --- a/codex-rs/core/src/models.rs +++ b/codex-rs/core/src/models.rs @@ -45,7 +45,7 @@ pub enum ResponseItem { Reasoning { id: String, summary: Vec, - #[serde(default, skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "should_serialize_reasoning_content")] content: Option>, encrypted_content: Option, }, @@ -81,6 +81,15 @@ pub enum ResponseItem { Other, } +fn should_serialize_reasoning_content(content: &Option>) -> bool { + match content { + Some(content) => !content + .iter() + .any(|c| matches!(c, ReasoningItemContent::ReasoningText { .. })), + None => false, + } +} + impl From for ResponseItem { fn from(item: ResponseInputItem) -> Self { match item { @@ -142,6 +151,7 @@ pub enum ReasoningItemReasoningSummary { #[serde(tag = "type", rename_all = "snake_case")] pub enum ReasoningItemContent { ReasoningText { text: String }, + Text { text: String }, } impl From> for ResponseInputItem { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 517a9a35..9f7f0518 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -133,6 +133,7 @@ impl ChatWidget<'_> { fn on_agent_message(&mut self, message: String) { let sink = AppEventHistorySink(self.app_event_tx.clone()); let finished = self.stream.apply_final_answer(&message, &sink); + self.last_stream_kind = Some(StreamKind::Answer); self.handle_if_stream_finished(finished); self.mark_needs_redraw(); } @@ -145,9 +146,10 @@ impl ChatWidget<'_> { self.handle_streaming_delta(StreamKind::Reasoning, delta); } - fn on_agent_reasoning_final(&mut self) { + fn on_agent_reasoning_final(&mut self, text: String) { let sink = AppEventHistorySink(self.app_event_tx.clone()); - let finished = self.stream.finalize(StreamKind::Reasoning, false, &sink); + let finished = self.stream.apply_final_reasoning(&text, &sink); + self.last_stream_kind = Some(StreamKind::Reasoning); self.handle_if_stream_finished(finished); self.mark_needs_redraw(); } @@ -633,9 +635,9 @@ impl ChatWidget<'_> { | EventMsg::AgentReasoningRawContentDelta(AgentReasoningRawContentDeltaEvent { delta, }) => self.on_agent_reasoning_delta(delta), - EventMsg::AgentReasoning(AgentReasoningEvent { .. }) - | EventMsg::AgentReasoningRawContent(AgentReasoningRawContentEvent { .. }) => { - self.on_agent_reasoning_final() + EventMsg::AgentReasoning(AgentReasoningEvent { text }) + | EventMsg::AgentReasoningRawContent(AgentReasoningRawContentEvent { text }) => { + self.on_agent_reasoning_final(text) } EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(), EventMsg::TaskStarted => self.on_task_started(), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap new file mode 100644 index 00000000..8258ea0b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/chatwidget/tests.rs +assertion_line: 886 +expression: combined +--- +thinking +I will first analyze the request. + +codex +Here is the result. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap new file mode 100644 index 00000000..c90ec273 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: combined +--- +thinking +I will first analyze the request. + +codex +Here is the result. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 0a7dd933..143dd4d0 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -12,6 +12,7 @@ use codex_core::plan_tool::UpdatePlanArgs; use codex_core::protocol::AgentMessageDeltaEvent; use codex_core::protocol::AgentMessageEvent; use codex_core::protocol::AgentReasoningDeltaEvent; +use codex_core::protocol::AgentReasoningEvent; use codex_core::protocol::ApplyPatchApprovalRequestEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; @@ -24,6 +25,7 @@ use codex_core::protocol::TaskCompleteEvent; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; +use insta::assert_snapshot; use pretty_assertions::assert_eq; use std::fs::File; use std::io::BufRead; @@ -413,46 +415,6 @@ async fn binary_size_transcript_matches_ideal_fixture() { assert_eq!(visible_after, ideal); } -#[test] -fn final_longer_answer_after_single_char_delta_is_complete() { - let (mut chat, rx, _op_rx) = make_chatwidget_manual(); - - // Simulate a stray delta without newline (e.g., punctuation). - chat.handle_codex_event(Event { - id: "sub-x".into(), - msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { delta: "?".into() }), - }); - - // Now send the full final answer with no newline. - let full = "Hi! How can I help with codex-rs today? Want me to explore the repo, run tests, or work on a specific change?"; - chat.handle_codex_event(Event { - id: "sub-x".into(), - msg: EventMsg::AgentMessage(AgentMessageEvent { - message: full.into(), - }), - }); - - // Drain and assert the full message appears in history. - let cells = drain_insert_history(&rx); - let mut found = false; - for lines in &cells { - let s = lines - .iter() - .flat_map(|l| l.spans.iter()) - .map(|sp| sp.content.clone()) - .collect::(); - if s.contains(full) { - found = true; - break; - } - } - assert!( - found, - "expected full final message to be flushed to history, cells={:?}", - cells.len() - ); -} - #[test] fn apply_patch_events_emit_history_cells() { let (mut chat, rx, _op_rx) = make_chatwidget_manual(); @@ -923,3 +885,91 @@ fn multiple_agent_messages_in_single_turn_emit_multiple_headers() { let second_idx = combined.find("Second message").unwrap(); assert!(first_idx < second_idx, "messages out of order: {combined}"); } + +#[test] +fn final_reasoning_then_message_without_deltas_are_rendered() { + let (mut chat, rx, _op_rx) = make_chatwidget_manual(); + + // No deltas; only final reasoning followed by final message. + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentReasoning(AgentReasoningEvent { + text: "I will first analyze the request.".into(), + }), + }); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentMessage(AgentMessageEvent { + message: "Here is the result.".into(), + }), + }); + + // Drain history and snapshot the combined visible content. + let cells = drain_insert_history(&rx); + let combined = cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert_snapshot!(combined); +} + +#[test] +fn deltas_then_same_final_message_are_rendered_snapshot() { + let (mut chat, rx, _op_rx) = make_chatwidget_manual(); + + // Stream some reasoning deltas first. + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "I will ".into(), + }), + }); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "first analyze the ".into(), + }), + }); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "request.".into(), + }), + }); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentReasoning(AgentReasoningEvent { + text: "request.".into(), + }), + }); + + // Then stream answer deltas, followed by the exact same final message. + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "Here is the ".into(), + }), + }); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { + delta: "result.".into(), + }), + }); + + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::AgentMessage(AgentMessageEvent { + message: "Here is the result.".into(), + }), + }); + + // Snapshot the combined visible content to ensure we render as expected + // when deltas are followed by the identical final message. + let cells = drain_insert_history(&rx); + let combined = cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect::(); + assert_snapshot!(combined); +} diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 5201d359..161a1111 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -143,6 +143,10 @@ impl StreamController { }; let cfg = self.config.clone(); let state = self.state_mut(kind); + // Record that at least one delta was received for this stream + if !delta.is_empty() { + state.has_seen_delta = true; + } state.collector.push_delta(delta); if delta.contains('\n') { let newly_completed = state.collector.commit_complete_lines(&cfg); @@ -263,19 +267,41 @@ impl StreamController { /// Apply a full final answer: replace queued content with only the remaining tail, /// then finalize immediately and notify completion. pub(crate) fn apply_final_answer(&mut self, message: &str, sink: &impl HistorySink) -> bool { - self.begin(StreamKind::Answer, sink); - if !message.is_empty() { - let mut msg_with_nl = message.to_string(); - if !msg_with_nl.ends_with('\n') { - msg_with_nl.push('\n'); + self.apply_full_final(StreamKind::Answer, message, true, sink) + } + + pub(crate) fn apply_final_reasoning(&mut self, message: &str, sink: &impl HistorySink) -> bool { + self.apply_full_final(StreamKind::Reasoning, message, false, sink) + } + + fn apply_full_final( + &mut self, + kind: StreamKind, + message: &str, + immediate: bool, + sink: &impl HistorySink, + ) -> bool { + self.begin(kind, sink); + + { + let state = self.state_mut(kind); + // Only inject the final full message if we have not seen any deltas for this stream. + // If deltas were received, rely on the collector's existing buffer to avoid duplication. + if !state.has_seen_delta && !message.is_empty() { + // normalize to end with newline + let mut msg = message.to_owned(); + if !msg.ends_with('\n') { + msg.push('\n'); + } + + // replace while preserving already committed count + let committed = state.collector.committed_count(); + state + .collector + .replace_with_and_mark_committed(&msg, committed); } - let state = self.state_mut(StreamKind::Answer); - let already_committed = state.collector.committed_count(); - // Preserve previously committed count so finalize emits only the remaining tail. - state - .collector - .replace_with_and_mark_committed(&msg_with_nl, already_committed); } - self.finalize(StreamKind::Answer, true, sink) + + self.finalize(kind, immediate, sink) } } diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index c6f4ad21..bb4fdcb6 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -11,6 +11,7 @@ pub(crate) enum StreamKind { pub(crate) struct StreamState { pub(crate) collector: MarkdownStreamCollector, pub(crate) streamer: AnimatedLineStreamer, + pub(crate) has_seen_delta: bool, } impl StreamState { @@ -18,11 +19,13 @@ impl StreamState { Self { collector: MarkdownStreamCollector::new(), streamer: AnimatedLineStreamer::new(), + has_seen_delta: false, } } pub(crate) fn clear(&mut self) { self.collector.clear(); self.streamer.clear(); + self.has_seen_delta = false; } pub(crate) fn step(&mut self) -> crate::markdown_stream::StepResult { self.streamer.step()