From 5d2702f6b897cec9d51ceafbd8547564d49d573c Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Fri, 6 Feb 2026 18:39:52 -0800 Subject: [PATCH] fix(tui): conditionally restore status indicator using message phase (#10947) TLDR: use new message phase field emitted by preamble-supported models to determine whether an AgentMessage is mid-turn commentary. if so, restore the status indicator afterwards to indicate the turn has not completed. ### Problem `commit_tick` hides the status indicator while streaming assistant text. For preamble-capable models, that text can be commentary mid-turn, so hiding was correct during streaming but restore timing mattered: - restoring too aggressively caused jitter/flashing - not restoring caused indicator to stay hidden before subsequent work (tool calls, web search, etc.) ### Fix - Add optional `phase` to `AgentMessageItem` and propagate it from `ResponseItem::Message` - Keep indicator hidden during streamed commit ticks, restore only when: - assistant item completes as `phase=commentary`, and - stream queues are idle + task is still running. - Treat `phase=None` as final-answer behavior (no restore) to keep existing behavior for non-preamble models ### Tests Add/update tests for: - no idle-tick restore without commentary completion - commentary completion restoring status before tool begin - snapshot coverage for preamble/status behavior --------- Co-authored-by: Josh McKinney --- .../schema/json/ClientRequest.json | 22 +++- .../schema/json/EventMsg.json | 34 ++++- .../schema/json/ServerNotification.json | 34 ++++- .../codex_app_server_protocol.schemas.json | 56 ++++++-- .../json/v1/ForkConversationResponse.json | 34 ++++- .../json/v1/ResumeConversationParams.json | 22 +++- .../json/v1/ResumeConversationResponse.json | 34 ++++- .../v1/SessionConfiguredNotification.json | 34 ++++- .../RawResponseItemCompletedNotification.json | 22 +++- .../schema/json/v2/ThreadResumeParams.json | 22 +++- .../schema/typescript/AgentMessageItem.ts | 17 ++- .../schema/typescript/MessagePhase.ts | 6 + .../app-server-protocol/src/protocol/v2.rs | 1 + codex-rs/core/src/codex.rs | 1 + codex-rs/core/src/event_mapping.rs | 16 ++- codex-rs/protocol/src/items.rs | 16 +++ codex-rs/protocol/src/models.rs | 12 +- codex-rs/tui/src/chatwidget.rs | 101 +++++++++++--- codex-rs/tui/src/chatwidget/tests.rs | 123 +++++++++++++++++- 19 files changed, 527 insertions(+), 80 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 7ffaadbfc..aa6c86d87 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1064,11 +1064,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index f399912e0..905d6af65 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -3169,11 +3169,23 @@ ] }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -4685,6 +4697,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -4695,6 +4708,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 0e2f94f4d..46a47ac76 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -3948,11 +3948,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -6686,6 +6698,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -6696,6 +6709,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a926db910..ddcf49556 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -6064,11 +6064,23 @@ ] }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -9146,6 +9158,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -9156,6 +9169,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" @@ -12178,11 +12202,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index a5838e89e..3b5ed750b 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -3169,11 +3169,23 @@ ] }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -4685,6 +4697,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -4695,6 +4708,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json index 9ce52963d..5c7f94378 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json @@ -271,11 +271,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "NewConversationParams": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index 718b17aa2..9c5c3653e 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -3169,11 +3169,23 @@ ] }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -4685,6 +4697,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -4695,6 +4708,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index a85b78281..a008e0253 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -3169,11 +3169,23 @@ ] }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", @@ -4685,6 +4697,7 @@ "type": "object" }, { + "description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.", "properties": { "content": { "items": { @@ -4695,6 +4708,17 @@ "id": { "type": "string" }, + "phase": { + "anyOf": [ + { + "$ref": "#/definitions/MessagePhase" + }, + { + "type": "null" + } + ], + "description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter." + }, "type": { "enum": [ "AgentMessage" diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index c1e36ad8e..748eeaab4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -238,11 +238,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "ReasoningItemContent": { "oneOf": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index 58e20e58e..fc6593b18 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -247,11 +247,23 @@ "type": "string" }, "MessagePhase": { - "enum": [ - "commentary", - "final_answer" - ], - "type": "string" + "description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.", + "oneOf": [ + { + "description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.", + "enum": [ + "commentary" + ], + "type": "string" + }, + { + "description": "The assistant's terminal answer text for the current turn.", + "enum": [ + "final_answer" + ], + "type": "string" + } + ] }, "Personality": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/AgentMessageItem.ts b/codex-rs/app-server-protocol/schema/typescript/AgentMessageItem.ts index f88406758..ee67a3e23 100644 --- a/codex-rs/app-server-protocol/schema/typescript/AgentMessageItem.ts +++ b/codex-rs/app-server-protocol/schema/typescript/AgentMessageItem.ts @@ -2,5 +2,20 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AgentMessageContent } from "./AgentMessageContent"; +import type { MessagePhase } from "./MessagePhase"; -export type AgentMessageItem = { id: string, content: Array, }; +/** + * Assistant-authored message payload used in turn-item streams. + * + * `phase` is optional because not all providers/models emit it. Consumers + * should use it when present, but retain legacy completion semantics when it + * is `None`. + */ +export type AgentMessageItem = { id: string, content: Array, +/** + * Optional phase metadata carried through from `ResponseItem::Message`. + * + * This is currently used by TUI rendering to distinguish mid-turn + * commentary from a final answer and avoid status-indicator jitter. + */ +phase?: MessagePhase, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts b/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts index d339c0fa8..9e16021b5 100644 --- a/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts +++ b/codex-rs/app-server-protocol/schema/typescript/MessagePhase.ts @@ -2,4 +2,10 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +/** + * Classifies an assistant message as interim commentary or final answer text. + * + * Providers do not emit this consistently, so callers must treat `None` as + * "phase unknown" and keep compatibility behavior for legacy models. + */ export type MessagePhase = "commentary" | "final_answer"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 67e412ff0..b4a2e8267 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -3249,6 +3249,7 @@ mod tests { text: "world".to_string(), }, ], + phase: None, }); assert_eq!( diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 93f87e5c4..076fb4b1e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4483,6 +4483,7 @@ async fn emit_agent_message_in_plan_mode( TurnItem::AgentMessage(codex_protocol::items::AgentMessageItem { id: agent_message_id.clone(), content: Vec::new(), + phase: None, }) }); sess.emit_turn_item_started(turn_context, &start_item).await; diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 2ad19d3df..16f7e1c47 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -5,6 +5,7 @@ use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::items::WebSearchItem; use codex_protocol::models::ContentItem; +use codex_protocol::models::MessagePhase; use codex_protocol::models::ReasoningItemContent; use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::ResponseItem; @@ -69,7 +70,11 @@ fn parse_user_message(message: &[ContentItem]) -> Option { Some(UserMessageItem::new(&content)) } -fn parse_agent_message(id: Option<&String>, message: &[ContentItem]) -> AgentMessageItem { +fn parse_agent_message( + id: Option<&String>, + message: &[ContentItem], + phase: Option, +) -> AgentMessageItem { let mut content: Vec = Vec::new(); for content_item in message.iter() { match content_item { @@ -85,18 +90,23 @@ fn parse_agent_message(id: Option<&String>, message: &[ContentItem]) -> AgentMes } } let id = id.cloned().unwrap_or_else(|| Uuid::new_v4().to_string()); - AgentMessageItem { id, content } + AgentMessageItem { id, content, phase } } pub fn parse_turn_item(item: &ResponseItem) -> Option { match item { ResponseItem::Message { - role, content, id, .. + role, + content, + id, + phase, + .. } => match role.as_str() { "user" => parse_user_message(content).map(TurnItem::UserMessage), "assistant" => Some(TurnItem::AgentMessage(parse_agent_message( id.as_ref(), content, + phase.clone(), ))), "system" => None, _ => None, diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 9a387a9d2..35bed2ab4 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -1,3 +1,4 @@ +use crate::models::MessagePhase; use crate::models::WebSearchAction; use crate::protocol::AgentMessageEvent; use crate::protocol::AgentReasoningEvent; @@ -40,9 +41,21 @@ pub enum AgentMessageContent { } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +/// Assistant-authored message payload used in turn-item streams. +/// +/// `phase` is optional because not all providers/models emit it. Consumers +/// should use it when present, but retain legacy completion semantics when it +/// is `None`. pub struct AgentMessageItem { pub id: String, pub content: Vec, + /// Optional phase metadata carried through from `ResponseItem::Message`. + /// + /// This is currently used by TUI rendering to distinguish mid-turn + /// commentary from a final answer and avoid status-indicator jitter. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub phase: Option, } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] @@ -172,10 +185,13 @@ impl AgentMessageItem { Self { id: uuid::Uuid::new_v4().to_string(), content: content.to_vec(), + phase: None, } } pub fn as_legacy_events(&self) -> Vec { + // Legacy events only preserve visible assistant text; `phase` has no + // representation in the v1 event stream. self.content .iter() .map(|c| match c { diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index ae38a7890..7f3319970 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -74,8 +74,17 @@ pub enum ContentItem { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] +/// Classifies an assistant message as interim commentary or final answer text. +/// +/// Providers do not emit this consistently, so callers must treat `None` as +/// "phase unknown" and keep compatibility behavior for legacy models. pub enum MessagePhase { + /// Mid-turn assistant text (for example preamble/progress narration). + /// + /// Additional tool calls or assistant output may follow before turn + /// completion. Commentary, + /// The assistant's terminal answer text for the current turn. FinalAnswer, } @@ -93,7 +102,8 @@ pub enum ResponseItem { #[ts(optional)] end_turn: Option, // Optional output-message phase (for example: "commentary", "final_answer"). - // Do not use directly; availability can vary by provider and model. + // Availability varies by provider/model, so downstream consumers must + // preserve fallback behavior when this is absent. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] phase: Option, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 19e7ff791..94fece362 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -20,6 +20,11 @@ //! is in progress and while MCP server startup is in progress. Those lifecycles are tracked //! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via //! `update_task_running_state`. +//! +//! For preamble-capable models, assistant output may include commentary before +//! the final answer. During streaming we hide the status row to avoid duplicate +//! progress indicators; once commentary completes and stream queues drain, we +//! re-show it so users still see turn-in-progress state between output bursts. use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -116,6 +121,8 @@ use codex_protocol::config_types::Personality; use codex_protocol::config_types::Settings; #[cfg(target_os = "windows")] use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::items::AgentMessageItem; +use codex_protocol::models::MessagePhase; use codex_protocol::models::local_image_label_text; use codex_protocol::parse_command::ParsedCommand; use codex_protocol::request_user_input::RequestUserInputEvent; @@ -542,6 +549,8 @@ pub(crate) struct ChatWidget { current_status_header: String, // Previous status header to restore after a transient stream retry. retry_status_header: Option, + // Set when commentary output completes; once stream queues go idle we restore the status row. + pending_status_indicator_restore: bool, thread_id: Option, thread_name: Option, forked_from: Option, @@ -808,6 +817,37 @@ impl ChatWidget { self.adaptive_chunking.reset(); } + fn stream_controllers_idle(&self) -> bool { + self.stream_controller + .as_ref() + .map(|controller| controller.queued_lines() == 0) + .unwrap_or(true) + && self + .plan_stream_controller + .as_ref() + .map(|controller| controller.queued_lines() == 0) + .unwrap_or(true) + } + + /// Restore the status indicator only after commentary completion is pending, + /// the turn is still running, and all stream queues have drained. + /// + /// This gate prevents flicker while normal output is still actively + /// streaming, but still restores a visible "working" affordance when a + /// commentary block ends before the turn itself has completed. + fn maybe_restore_status_indicator_after_stream_idle(&mut self) { + if !self.pending_status_indicator_restore + || !self.bottom_pane.is_task_running() + || !self.stream_controllers_idle() + { + return; + } + + self.bottom_pane.ensure_status_indicator(); + self.set_status_header(self.current_status_header.clone()); + self.pending_status_indicator_restore = false; + } + /// Update the status indicator header and details. /// /// Passing `None` clears any existing details. @@ -1181,21 +1221,29 @@ impl ChatWidget { } else { text }; + // Plan commit ticks can hide the status row; remember whether we streamed plan output so + // completion can restore it once stream queues are idle. + let should_restore_after_stream = self.plan_stream_controller.is_some(); self.plan_delta_buffer.clear(); self.plan_item_active = false; self.saw_plan_item_this_turn = true; - if let Some(mut controller) = self.plan_stream_controller.take() - && let Some(cell) = controller.finalize() - { + let finalized_streamed_cell = + if let Some(mut controller) = self.plan_stream_controller.take() { + controller.finalize() + } else { + None + }; + if let Some(cell) = finalized_streamed_cell { self.add_boxed_history(cell); // TODO: Replace streamed output with the final plan item text if plan streaming is // removed or if we need to reconcile mismatches between streamed and final content. - return; + } else if !plan_text.is_empty() { + self.add_to_history(history_cell::new_proposed_plan(plan_text)); } - if plan_text.is_empty() { - return; + if should_restore_after_stream { + self.pending_status_indicator_restore = true; + self.maybe_restore_status_indicator_after_stream_idle(); } - self.add_to_history(history_cell::new_proposed_plan(plan_text)); } fn on_agent_reasoning_delta(&mut self, delta: String) { @@ -1256,6 +1304,7 @@ impl ChatWidget { self.quit_shortcut_key = None; self.update_task_running_state(); self.retry_status_header = None; + self.pending_status_indicator_restore = false; self.bottom_pane.set_interrupt_hint_visible(true); self.set_status_header(String::from("Working")); self.full_reasoning_buffer.clear(); @@ -1297,6 +1346,7 @@ impl ChatWidget { self.request_status_line_branch_refresh(); } // Mark task stopped and request redraw now that all content is in history. + self.pending_status_indicator_restore = false; self.agent_turn_running = false; self.update_task_running_state(); self.running_commands.clear(); @@ -1528,6 +1578,7 @@ impl ChatWidget { self.adaptive_chunking.reset(); self.stream_controller = None; self.plan_stream_controller = None; + self.pending_status_indicator_restore = false; self.request_status_line_branch_refresh(); self.maybe_show_pending_rate_limit_prompt(); } @@ -2087,9 +2138,24 @@ impl ChatWidget { if self.retry_status_header.is_none() { self.retry_status_header = Some(self.current_status_header.clone()); } + self.bottom_pane.ensure_status_indicator(); self.set_status(message, additional_details); } + /// Handle completion of an `AgentMessage` turn item. + /// + /// Commentary completion sets a deferred restore flag so the status row + /// returns once stream queues are idle. Final-answer completion (or absent + /// phase for legacy models) clears the flag to preserve historical behavior. + fn on_agent_message_item_completed(&mut self, item: AgentMessageItem) { + self.pending_status_indicator_restore = match item.phase { + // Models that don't support preambles only output AgentMessageItems on turn completion. + Some(MessagePhase::FinalAnswer) | None => false, + Some(MessagePhase::Commentary) => true, + }; + self.maybe_restore_status_indicator_after_stream_idle(); + } + /// Periodic tick for stream commits. In smooth mode this preserves one-line pacing, while /// catch-up mode drains larger batches to reduce queue lag. pub(crate) fn on_commit_tick(&mut self) { @@ -2110,9 +2176,8 @@ impl ChatWidget { /// /// `scope` controls whether this call may commit in smooth mode or only when catch-up /// is currently active. While lines are actively streaming we hide the status row to avoid - /// duplicate "in progress" affordances, but once all stream controllers go idle for this - /// turn we restore the status row if the task is still running so users keep a live - /// spinner/shimmer signal between preamble output and subsequent tool activity. + /// duplicate "in progress" affordances. Restoration is gated separately so we only re-show + /// the row after commentary completion once stream queues are idle. fn run_commit_tick_with_scope(&mut self, scope: CommitTickScope) { let now = Instant::now(); let outcome = run_commit_tick( @@ -2128,10 +2193,7 @@ impl ChatWidget { } if outcome.has_controller && outcome.all_idle { - if self.bottom_pane.is_task_running() { - self.bottom_pane.ensure_status_indicator(); - self.set_status_header(self.current_status_header.clone()); - } + self.maybe_restore_status_indicator_after_stream_idle(); self.app_event_tx.send(AppEvent::StopCommitAnimation); } @@ -2562,6 +2624,7 @@ impl ChatWidget { full_reasoning_buffer: String::new(), current_status_header: String::from("Working"), retry_status_header: None, + pending_status_indicator_restore: false, thread_id: None, thread_name: None, forked_from: None, @@ -2724,6 +2787,7 @@ impl ChatWidget { full_reasoning_buffer: String::new(), current_status_header: String::from("Working"), retry_status_header: None, + pending_status_indicator_restore: false, thread_id: None, thread_name: None, forked_from: None, @@ -2875,6 +2939,7 @@ impl ChatWidget { full_reasoning_buffer: String::new(), current_status_header: String::from("Working"), retry_status_header: None, + pending_status_indicator_restore: false, thread_id: None, thread_name: None, forked_from: None, @@ -3967,8 +4032,12 @@ impl ChatWidget { | EventMsg::ReasoningRawContentDelta(_) | EventMsg::DynamicToolCallRequest(_) => {} EventMsg::ItemCompleted(event) => { - if let codex_protocol::items::TurnItem::Plan(plan_item) = event.item { - self.on_plan_item_completed(plan_item.text); + let item = event.item; + if let codex_protocol::items::TurnItem::Plan(plan_item) = &item { + self.on_plan_item_completed(plan_item.text.clone()); + } + if let codex_protocol::items::TurnItem::AgentMessage(item) = item { + self.on_agent_message_item_completed(item); } } } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index f338157ee..fcd2b8461 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -41,6 +41,7 @@ use codex_core::protocol::ExecCommandSource; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::ExitedReviewModeEvent; use codex_core::protocol::FileChange; +use codex_core::protocol::ItemCompletedEvent; use codex_core::protocol::McpStartupCompleteEvent; use codex_core::protocol::McpStartupStatus; use codex_core::protocol::McpStartupUpdateEvent; @@ -71,6 +72,10 @@ use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; use codex_protocol::config_types::Settings; +use codex_protocol::items::AgentMessageContent; +use codex_protocol::items::AgentMessageItem; +use codex_protocol::items::TurnItem; +use codex_protocol::models::MessagePhase; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::default_input_modalities; @@ -1074,6 +1079,7 @@ async fn make_chatwidget_manual( full_reasoning_buffer: String::new(), current_status_header: String::from("Working"), retry_status_header: None, + pending_status_indicator_restore: false, thread_id: None, thread_name: None, forked_from: None, @@ -1958,6 +1964,28 @@ fn terminal_interaction(chat: &mut ChatWidget, call_id: &str, process_id: &str, }); } +fn complete_assistant_message( + chat: &mut ChatWidget, + item_id: &str, + text: &str, + phase: Option, +) { + chat.handle_codex_event(Event { + id: format!("raw-{item_id}"), + msg: EventMsg::ItemCompleted(ItemCompletedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + item: TurnItem::AgentMessage(AgentMessageItem { + id: item_id.to_string(), + content: vec![AgentMessageContent::Text { + text: text.to_string(), + }], + phase, + }), + }), + }); +} + fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) -> ExecCommandBeginEvent { begin_exec_with_source(chat, call_id, raw_cmd, ExecCommandSource::Agent) } @@ -2103,15 +2131,16 @@ async fn enqueueing_history_prompt_multiple_times_is_stable() { #[tokio::test] async fn streaming_final_answer_keeps_task_running_state() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; chat.thread_id = Some(ThreadId::new()); chat.on_task_started(); chat.on_agent_message_delta("Final answer line\n".to_string()); chat.on_commit_tick(); + drain_insert_history(&mut rx); assert!(chat.bottom_pane.is_task_running()); - assert!(chat.bottom_pane.status_widget().is_some()); + assert!(!chat.bottom_pane.status_indicator_visible()); chat.bottom_pane .set_composer_text("queued submission".to_string(), Vec::new(), Vec::new()); @@ -2133,7 +2162,26 @@ async fn streaming_final_answer_keeps_task_running_state() { } #[tokio::test] -async fn preamble_keeps_status_indicator_visible_until_exec_begin() { +async fn idle_commit_ticks_do_not_restore_status_without_commentary_completion() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.on_task_started(); + assert_eq!(chat.bottom_pane.status_indicator_visible(), true); + + chat.on_agent_message_delta("Final answer line\n".to_string()); + chat.on_commit_tick(); + drain_insert_history(&mut rx); + + assert_eq!(chat.bottom_pane.status_indicator_visible(), false); + assert_eq!(chat.bottom_pane.is_task_running(), true); + + // A second idle tick should not toggle the row back on and cause jitter. + chat.on_commit_tick(); + assert_eq!(chat.bottom_pane.status_indicator_visible(), false); +} + +#[tokio::test] +async fn commentary_completion_restores_status_indicator_before_exec_begin() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; chat.on_task_started(); @@ -2143,12 +2191,45 @@ async fn preamble_keeps_status_indicator_visible_until_exec_begin() { chat.on_commit_tick(); drain_insert_history(&mut rx); + assert_eq!(chat.bottom_pane.status_indicator_visible(), false); + + complete_assistant_message( + &mut chat, + "msg-commentary", + "Preamble line\n", + Some(MessagePhase::Commentary), + ); + assert_eq!(chat.bottom_pane.status_indicator_visible(), true); assert_eq!(chat.bottom_pane.is_task_running(), true); begin_exec(&mut chat, "call-1", "echo hi"); + assert_eq!(chat.bottom_pane.status_indicator_visible(), true); +} + +#[tokio::test] +async fn plan_completion_restores_status_indicator_after_streaming_plan_output() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + chat.set_feature_enabled(Feature::CollaborationModes, true); + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); + + chat.on_task_started(); + assert_eq!(chat.bottom_pane.status_indicator_visible(), true); + + chat.on_plan_delta("- Step 1\n".to_string()); + chat.on_commit_tick(); + drain_insert_history(&mut rx); + + assert_eq!(chat.bottom_pane.status_indicator_visible(), false); + assert_eq!(chat.bottom_pane.is_task_running(), true); + + chat.on_plan_item_completed("- Step 1\n".to_string()); assert_eq!(chat.bottom_pane.status_indicator_visible(), true); + assert_eq!(chat.bottom_pane.is_task_running(), true); } #[tokio::test] @@ -2157,11 +2238,17 @@ async fn preamble_keeps_working_status_snapshot() { chat.thread_id = Some(ThreadId::new()); // Regression sequence: a preamble line is committed to history before any exec/tool event. - // The status row must remain visible so the spinner/shimmer still communicates "working". + // After commentary completes, the status row should be restored before subsequent work. chat.on_task_started(); chat.on_agent_message_delta("Preamble line\n".to_string()); chat.on_commit_tick(); drain_insert_history(&mut rx); + complete_assistant_message( + &mut chat, + "msg-commentary-snapshot", + "Preamble line\n", + Some(MessagePhase::Commentary), + ); let height = chat.desired_height(80); let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) @@ -5176,6 +5263,34 @@ async fn stream_error_updates_status_indicator() { assert_eq!(status.details(), Some(details)); } +#[tokio::test] +async fn stream_error_restores_hidden_status_indicator() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + chat.on_task_started(); + chat.on_agent_message_delta("Preamble line\n".to_string()); + chat.on_commit_tick(); + drain_insert_history(&mut rx); + assert!(!chat.bottom_pane.status_indicator_visible()); + + let msg = "Reconnecting... 2/5"; + let details = "Idle timeout waiting for SSE"; + chat.handle_codex_event(Event { + id: "sub-1".into(), + msg: EventMsg::StreamError(StreamErrorEvent { + message: msg.to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + additional_details: Some(details.to_string()), + }), + }); + + let status = chat + .bottom_pane + .status_widget() + .expect("status indicator should be visible"); + assert_eq!(status.header(), msg); + assert_eq!(status.details(), Some(details)); +} + #[tokio::test] async fn warning_event_adds_warning_history_cell() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;