From 77ee4e9f9da6598eef533e6728d8e47d1772bb99 Mon Sep 17 00:00:00 2001 From: Amit Singh Date: Sat, 28 Mar 2026 14:27:00 +0530 Subject: [PATCH 1/2] feat(domain): add message phase support across messages --- Cargo.lock | 4 +- Cargo.toml | 2 +- crates/forge_app/src/hooks/doom_loop.rs | 4 + crates/forge_app/src/hooks/tracing.rs | 1 + crates/forge_app/src/orch.rs | 1 + crates/forge_app/src/user_prompt.rs | 3 + crates/forge_domain/src/context.rs | 18 +- crates/forge_domain/src/hook.rs | 1 + crates/forge_domain/src/message.rs | 19 ++ crates/forge_domain/src/result_stream_ext.rs | 23 +++ .../src/transformer/transform_tool_calls.rs | 1 + .../src/conversation/conversation_record.rs | 1 + .../src/conversation/conversation_repo.rs | 1 + .../provider/openai_responses/repository.rs | 104 ++++++++++- .../src/provider/openai_responses/request.rs | 172 +++++++++++++++++- .../src/provider/openai_responses/response.rs | 136 +++++++++++++- 16 files changed, 479 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e935d9dc52..528134059b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -138,9 +138,9 @@ dependencies = [ [[package]] name = "async-openai" -version = "0.33.1" +version = "0.34.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cc48c3deb4ad9a2ee8c8e364c79eb0f74e69e17ed7e883d55988b90ea44fe986" +checksum = "ec08254d61379df136135d3d1ac04301be7699fd7d9e57655c63ac7d650a6922" dependencies = [ "derive_builder 0.20.2", "getrandom 0.3.4", diff --git a/Cargo.toml b/Cargo.toml index 9a9f43786a..e28b023abc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -131,7 +131,7 @@ gray_matter = "0.3.2" num-format = "0.4" humantime = "2.1.0" dashmap = "7.0.0-rc2" -async-openai = { version = "0.33.1", default-features = false, features = ["response-types"] } # Using only types, not the API client - reduces dependencies +async-openai = { version = "0.34.0", default-features = false, features = ["response-types"] } # Using only types, not the API client - reduces dependencies google-cloud-auth = "1.7.0" # Google Cloud authentication with automatic token refresh # Internal crates diff --git a/crates/forge_app/src/hooks/doom_loop.rs b/crates/forge_app/src/hooks/doom_loop.rs index 4e96982915..3515b74e7b 100644 --- a/crates/forge_app/src/hooks/doom_loop.rs +++ b/crates/forge_app/src/hooks/doom_loop.rs @@ -268,6 +268,7 @@ mod tests { model: None, reasoning_details: None, droppable: false, + phase: None, } } @@ -403,6 +404,7 @@ mod tests { model: None, reasoning_details: None, droppable: false, + phase: None, }; let user_msg = TextMessage { @@ -414,6 +416,7 @@ mod tests { model: None, reasoning_details: None, droppable: false, + phase: None, }; let assistant_msg_2 = TextMessage { @@ -425,6 +428,7 @@ mod tests { model: None, reasoning_details: None, droppable: false, + phase: None, }; let messages = [ diff --git a/crates/forge_app/src/hooks/tracing.rs b/crates/forge_app/src/hooks/tracing.rs index dfb9045d69..94755f2b2c 100644 --- a/crates/forge_app/src/hooks/tracing.rs +++ b/crates/forge_app/src/hooks/tracing.rs @@ -204,6 +204,7 @@ mod tests { tool_calls: vec![], usage: Default::default(), finish_reason: None, + phase: None, }; let event = EventData::new(test_agent(), test_model_id(), ResponsePayload::new(message)); diff --git a/crates/forge_app/src/orch.rs b/crates/forge_app/src/orch.rs index a377814c33..853118dd66 100644 --- a/crates/forge_app/src/orch.rs +++ b/crates/forge_app/src/orch.rs @@ -311,6 +311,7 @@ impl Orchestrator { message.reasoning_details.clone(), message.usage, tool_call_records, + message.phase, ); if self.error_tracker.limit_reached() { diff --git a/crates/forge_app/src/user_prompt.rs b/crates/forge_app/src/user_prompt.rs index 22f542876e..382ba8e765 100644 --- a/crates/forge_app/src/user_prompt.rs +++ b/crates/forge_app/src/user_prompt.rs @@ -76,6 +76,7 @@ impl UserPromptGenerator { reasoning_details: None, model: Some(self.agent.model.clone()), droppable: true, // Droppable so it can be removed during context compression + phase: None, }; context = context.add_message(ContextMessage::Text(todo_message)); } @@ -121,6 +122,7 @@ impl UserPromptGenerator { reasoning_details: None, model: Some(self.agent.model.clone()), droppable: true, // Piped input is droppable + phase: None, }; context = context.add_message(ContextMessage::Text(piped_message)); } @@ -197,6 +199,7 @@ impl UserPromptGenerator { reasoning_details: None, model: Some(self.agent.model.clone()), droppable: false, + phase: None, }; context = context.add_message(ContextMessage::Text(message)); } diff --git a/crates/forge_domain/src/context.rs b/crates/forge_domain/src/context.rs index 8aca162601..4f41f2adf5 100644 --- a/crates/forge_domain/src/context.rs +++ b/crates/forge_domain/src/context.rs @@ -14,6 +14,7 @@ fn is_false(value: &bool) -> bool { !value } +use crate::MessagePhase; use crate::temperature::Temperature; use crate::top_k::TopK; use crate::top_p::TopP; @@ -169,6 +170,7 @@ impl ContextMessage { reasoning_details: None, model, droppable: false, + phase: None, } .into() } @@ -183,6 +185,7 @@ impl ContextMessage { model: None, reasoning_details: None, droppable: false, + phase: None, } .into() } @@ -204,6 +207,7 @@ impl ContextMessage { reasoning_details, model: None, droppable: false, + phase: None, } .into() } @@ -311,6 +315,10 @@ pub struct TextMessage { /// Indicates whether this message can be dropped during context compaction #[serde(default, skip_serializing_if = "is_false")] pub droppable: bool, + /// Phase label for assistant messages (`Commentary` or `FinalAnswer`). + /// Preserved from OpenAI Responses API and replayed back on subsequent requests. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub phase: Option, } impl TextMessage { @@ -325,6 +333,7 @@ impl TextMessage { model: None, reasoning_details: None, droppable: false, + phase: None, } } @@ -346,6 +355,7 @@ impl TextMessage { reasoning_details, model, droppable: false, + phase: None, } } } @@ -554,6 +564,7 @@ impl Context { reasoning_details: Option>, usage: Usage, tool_records: Vec<(ToolCallFull, ToolResult)>, + phase: Option, ) -> Self { // Convert flat reasoning string to reasoning_details if present let merged_reasoning_details = if let Some(reasoning_text) = reasoning { @@ -573,7 +584,7 @@ impl Context { }; // Adding tool calls - let message: MessageEntry = ContextMessage::assistant( + let mut message: MessageEntry = ContextMessage::assistant( content, thought_signature, merged_reasoning_details, @@ -586,6 +597,11 @@ impl Context { ) .into(); + // Set phase on the assistant TextMessage if provided + if let ContextMessage::Text(ref mut text_msg) = message.message { + text_msg.phase = phase; + } + let tool_results = tool_records .iter() .map(|record| record.1.clone()) diff --git a/crates/forge_domain/src/hook.rs b/crates/forge_domain/src/hook.rs index 8b16007484..47579d7a43 100644 --- a/crates/forge_domain/src/hook.rs +++ b/crates/forge_domain/src/hook.rs @@ -640,6 +640,7 @@ mod tests { reasoning_details: None, usage: crate::Usage::default(), finish_reason: None, + phase: None, }), )), LifecycleEvent::ToolcallStart(EventData::new( diff --git a/crates/forge_domain/src/message.rs b/crates/forge_domain/src/message.rs index e775bb3a97..969cfba07d 100644 --- a/crates/forge_domain/src/message.rs +++ b/crates/forge_domain/src/message.rs @@ -7,6 +7,20 @@ use super::{ToolCall, ToolCallFull}; use crate::TokenCount; use crate::reasoning::{Reasoning, ReasoningFull}; +/// Labels an assistant message as intermediate commentary or the final answer. +/// +/// For models like `gpt-5.3-codex` and beyond, when sending follow-up requests, +/// preserve and resend phase on all assistant messages -- dropping it can degrade +/// performance. +#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum MessagePhase { + /// Intermediate commentary produced while the model is reasoning. + Commentary, + /// The final answer from the model. + FinalAnswer, +} + #[derive(Default, Clone, Copy, Debug, Serialize, Deserialize, PartialEq)] pub struct Usage { pub prompt_tokens: TokenCount, @@ -47,6 +61,9 @@ pub struct ChatCompletionMessage { pub tool_calls: Vec, pub finish_reason: Option, pub usage: Option, + /// Phase label for assistant messages (e.g. `Commentary` or `FinalAnswer`). + /// Preserved from the response and replayed back on subsequent requests. + pub phase: Option, } impl From for ChatCompletionMessage { @@ -176,6 +193,8 @@ pub struct ChatCompletionMessageFull { pub reasoning_details: Option>, pub usage: Usage, pub finish_reason: Option, + /// Phase label for the assistant message (e.g. `Commentary` or `FinalAnswer`). + pub phase: Option, } #[cfg(test)] diff --git a/crates/forge_domain/src/result_stream_ext.rs b/crates/forge_domain/src/result_stream_ext.rs index 028e7ec1c2..4da05195ac 100644 --- a/crates/forge_domain/src/result_stream_ext.rs +++ b/crates/forge_domain/src/result_stream_ext.rs @@ -245,6 +245,12 @@ impl ResultStreamExt for crate::BoxStream for crate::BoxStream for forge_domain::TextMessage { .reasoning_details .map(|details| details.into_iter().map(Into::into).collect()), droppable: record.droppable, + phase: None, }) } } diff --git a/crates/forge_repo/src/conversation/conversation_repo.rs b/crates/forge_repo/src/conversation/conversation_repo.rs index 33be8e75cb..bf0d152fe7 100644 --- a/crates/forge_repo/src/conversation/conversation_repo.rs +++ b/crates/forge_repo/src/conversation/conversation_repo.rs @@ -703,6 +703,7 @@ mod tests { thought_signature: None, reasoning_details: None, droppable: false, + phase: None, }), usage: Some(Usage { prompt_tokens: forge_domain::TokenCount::Actual(100), diff --git a/crates/forge_repo/src/provider/openai_responses/repository.rs b/crates/forge_repo/src/provider/openai_responses/repository.rs index 54100589d9..1aaa935b3f 100644 --- a/crates/forge_repo/src/provider/openai_responses/repository.rs +++ b/crates/forge_repo/src/provider/openai_responses/repository.rs @@ -67,6 +67,13 @@ impl OpenAIResponsesProvider { } fn get_headers(&self) -> Vec<(String, String)> { + self.get_headers_for_conversation(None) + } + + fn get_headers_for_conversation( + &self, + conversation_id: Option<&str>, + ) -> Vec<(String, String)> { let mut headers = Vec::new(); if let Some(api_key) = self .provider @@ -111,9 +118,21 @@ impl OpenAIResponsesProvider { }); // Codex provider requires the ChatGPT-Account-Id header extracted - // from the JWT at login + // from the JWT at login. + // + // Mirror codex-rs conversation continuity headers by sending: + // - x-client-request-id: conversation id + // - session_id: conversation id if self.provider.id == forge_domain::ProviderId::CODEX { - // Add ChatGPT-Account-Id from credential's stored url_params + if let Some(conversation_id) = conversation_id { + headers.push(( + "x-client-request-id".to_string(), + conversation_id.to_string(), + )); + headers.push(("session_id".to_string(), conversation_id.to_string())); + } + + // Add ChatGPT-Account-Id from credential's stored url_params. if let Some(account_id) = self.provider.credential.as_ref().and_then(|c| { let key: forge_domain::URLParam = "chatgpt_account_id".to_string().into(); c.url_params.get(&key) @@ -124,6 +143,7 @@ impl OpenAIResponsesProvider { headers } + } impl OpenAIResponsesProvider { @@ -132,7 +152,8 @@ impl OpenAIResponsesProvider { model: &ModelId, context: ChatContext, ) -> ResultStream { - let headers = create_headers(self.get_headers()); + let conversation_id = context.conversation_id.as_ref().map(ToString::to_string); + let headers = create_headers(self.get_headers_for_conversation(conversation_id.as_deref())); let mut request = oai::CreateResponse::from_domain(context)?; request.model = Some(model.as_str().to_string()); @@ -603,6 +624,7 @@ mod tests { call_id: "call_id_1".to_string(), name: "tool1".to_string(), arguments: "args1".to_string(), + namespace: None, status: None, })), oai::InputItem::Item(oai::Item::FunctionCall(oai::FunctionToolCall { @@ -610,6 +632,7 @@ mod tests { call_id: "call_id_2".to_string(), name: "tool2".to_string(), arguments: "args2".to_string(), + namespace: None, status: None, })), ]), @@ -1128,6 +1151,81 @@ mod tests { assert!(account_header.is_none()); } + #[test] + fn test_get_headers_codex_with_conversation_id_includes_conversation_headers() { + let provider = Provider { + id: ProviderId::CODEX, + provider_type: forge_domain::ProviderType::Llm, + response: Some(ProviderResponse::OpenAI), + url: Url::parse("https://chatgpt.com/backend-api/codex/responses").unwrap(), + credential: make_credential(ProviderId::CODEX, "test-token"), + custom_headers: None, + auth_methods: vec![], + url_params: vec![], + models: None, + }; + + let infra = Arc::new(MockHttpClient { client: reqwest::Client::new() }); + let provider_impl = OpenAIResponsesProvider::::new(provider, infra); + let fixture = "conversation_test_123"; + + let actual = provider_impl.get_headers_for_conversation(Some(fixture)); + + let x_client_request_id = actual + .iter() + .find(|(k, _)| k == "x-client-request-id") + .map(|(_, v)| v.as_str()); + let session_id = actual + .iter() + .find(|(k, _)| k == "session_id") + .map(|(_, v)| v.as_str()); + + let expected = Some(fixture); + assert_eq!(x_client_request_id, expected); + assert_eq!(session_id, expected); + } + + #[test] + fn test_get_headers_non_codex_with_conversation_id_omits_conversation_headers() { + let provider = openai_responses("test-key", "https://api.openai.com/v1"); + let infra = Arc::new(MockHttpClient { client: reqwest::Client::new() }); + let provider_impl = OpenAIResponsesProvider::::new(provider, infra); + + let actual = provider_impl.get_headers_for_conversation(Some("conversation_test_123")); + + let x_client_request_id = actual.iter().find(|(k, _)| k == "x-client-request-id"); + let session_id = actual.iter().find(|(k, _)| k == "session_id"); + + assert!(x_client_request_id.is_none()); + assert!(session_id.is_none()); + } + + #[test] + fn test_get_headers_codex_without_conversation_id_omits_conversation_headers() { + let provider = Provider { + id: ProviderId::CODEX, + provider_type: forge_domain::ProviderType::Llm, + response: Some(ProviderResponse::OpenAI), + url: Url::parse("https://chatgpt.com/backend-api/codex/responses").unwrap(), + credential: make_credential(ProviderId::CODEX, "test-token"), + custom_headers: None, + auth_methods: vec![], + url_params: vec![], + models: None, + }; + + let infra = Arc::new(MockHttpClient { client: reqwest::Client::new() }); + let provider_impl = OpenAIResponsesProvider::::new(provider, infra); + + let actual = provider_impl.get_headers_for_conversation(None); + + let x_client_request_id = actual.iter().find(|(k, _)| k == "x-client-request-id"); + let session_id = actual.iter().find(|(k, _)| k == "session_id"); + + assert!(x_client_request_id.is_none()); + assert!(session_id.is_none()); + } + #[test] fn test_openai_responses_repository_new() { let infra = Arc::new(MockHttpClient { client: reqwest::Client::new() }); diff --git a/crates/forge_repo/src/provider/openai_responses/request.rs b/crates/forge_repo/src/provider/openai_responses/request.rs index 77ceabd2cc..975954d2c4 100644 --- a/crates/forge_repo/src/provider/openai_responses/request.rs +++ b/crates/forge_repo/src/provider/openai_responses/request.rs @@ -2,12 +2,20 @@ use std::collections::HashMap; use anyhow::Context as _; use async_openai::types::responses as oai; -use forge_app::domain::{Context as ChatContext, ContextMessage, Role, ToolChoice}; +use forge_app::domain::{Context as ChatContext, ContextMessage, MessagePhase, Role, ToolChoice}; use forge_app::utils::enforce_strict_schema; use forge_domain::{Effort, ReasoningConfig, ReasoningFull}; use crate::provider::FromDomain; +/// Converts domain MessagePhase to OpenAI MessagePhase +fn to_oai_phase(phase: MessagePhase) -> oai::MessagePhase { + match phase { + MessagePhase::Commentary => oai::MessagePhase::Commentary, + MessagePhase::FinalAnswer => oai::MessagePhase::FinalAnswer, + } +} + /// Groups reasoning details by their ID and builds OpenAI `ReasoningItem` /// input items. /// @@ -185,6 +193,7 @@ impl FromDomain for oai::CreateResponse { r#type: oai::MessageType::Message, role: oai::Role::Developer, content: oai::EasyInputContent::Text(message.content), + phase: None, })); } } @@ -193,6 +202,7 @@ impl FromDomain for oai::CreateResponse { r#type: oai::MessageType::Message, role: oai::Role::User, content: oai::EasyInputContent::Text(message.content), + phase: None, })); } Role::Assistant => { @@ -201,6 +211,7 @@ impl FromDomain for oai::CreateResponse { r#type: oai::MessageType::Message, role: oai::Role::Assistant, content: oai::EasyInputContent::Text(message.content), + phase: message.phase.map(to_oai_phase), })); } @@ -224,6 +235,7 @@ impl FromDomain for oai::CreateResponse { arguments: call.arguments.into_string(), call_id, name: call.name.to_string(), + namespace: None, id: None, status: None, }, @@ -268,6 +280,7 @@ impl FromDomain for oai::CreateResponse { image_url: Some(img.url().clone()), }), ]), + phase: None, })); } } @@ -289,6 +302,7 @@ impl FromDomain for oai::CreateResponse { parameters: Some(codex_tool_parameters(&tool.input_schema)?), strict: Some(true), description: Some(tool.description), + defer_loading: None, })) }) .collect::>>() @@ -341,6 +355,15 @@ impl FromDomain for oai::CreateResponse { response.stream = Some(true); + // When reasoning is configured, request encrypted content so it can be + // replayed in subsequent turns for stateless reasoning continuity. + if response.reasoning.is_some() { + let includes = response.include.get_or_insert_with(Vec::new); + if !includes.contains(&oai::IncludeEnum::ReasoningEncryptedContent) { + includes.push(oai::IncludeEnum::ReasoningEncryptedContent); + } + } + Ok(response) } } @@ -440,6 +463,40 @@ mod tests { Ok(()) } + #[test] + fn test_codex_request_with_reasoning_includes_encrypted_content() -> anyhow::Result<()> { + use forge_domain::{Effort, ReasoningConfig}; + + let reasoning = ReasoningConfig { + effort: Some(Effort::High), + max_tokens: None, + exclude: None, + enabled: Some(true), + }; + + let context = ChatContext::default() + .add_message(ContextMessage::user("Test", None)) + .reasoning(reasoning); + + let actual = oai::CreateResponse::from_domain(context)?; + + let expected = Some(vec![oai::IncludeEnum::ReasoningEncryptedContent]); + assert_eq!(actual.include, expected); + + Ok(()) + } + + #[test] + fn test_codex_request_without_reasoning_has_no_include() -> anyhow::Result<()> { + let context = ChatContext::default().add_message(ContextMessage::user("Test", None)); + + let actual = oai::CreateResponse::from_domain(context)?; + + assert_eq!(actual.include, None); + + Ok(()) + } + #[test] fn test_codex_request_from_context_converts_messages_tools_and_results() -> anyhow::Result<()> { let model = ModelId::from("codex-mini-latest"); @@ -1229,4 +1286,117 @@ mod tests { .contains("max_tokens must fit into u32") ); } + + #[test] + fn test_codex_request_preserves_phase_on_assistant_message() -> anyhow::Result<()> { + use forge_app::domain::{MessagePhase, TextMessage}; + use forge_domain::Role; + + let mut assistant_msg = TextMessage::new(Role::Assistant, "Thinking about this..."); + assistant_msg.phase = Some(MessagePhase::Commentary); + + let context = ChatContext::default() + .add_message(ContextMessage::user("Hello", None)) + .add_entry(forge_app::domain::MessageEntry::from( + ContextMessage::Text(assistant_msg), + )) + .add_message(ContextMessage::user("Continue", None)); + + let actual = oai::CreateResponse::from_domain(context)?; + + let oai::InputParam::Items(items) = actual.input else { + anyhow::bail!("Expected items input"); + }; + + // Find the assistant EasyMessage + let assistant_item = items + .iter() + .find(|item| { + matches!( + item, + oai::InputItem::EasyMessage(msg) if msg.role == oai::Role::Assistant + ) + }) + .expect("Should have an assistant message"); + + let oai::InputItem::EasyMessage(msg) = assistant_item else { + anyhow::bail!("Expected EasyMessage"); + }; + + assert_eq!(msg.phase, Some(oai::MessagePhase::Commentary)); + + Ok(()) + } + + #[test] + fn test_codex_request_preserves_final_answer_phase() -> anyhow::Result<()> { + use forge_app::domain::{MessagePhase, TextMessage}; + use forge_domain::Role; + + let mut assistant_msg = TextMessage::new(Role::Assistant, "The answer is 42."); + assistant_msg.phase = Some(MessagePhase::FinalAnswer); + + let context = ChatContext::default() + .add_message(ContextMessage::user("What is the answer?", None)) + .add_entry(forge_app::domain::MessageEntry::from( + ContextMessage::Text(assistant_msg), + )) + .add_message(ContextMessage::user("Thanks", None)); + + let actual = oai::CreateResponse::from_domain(context)?; + + let oai::InputParam::Items(items) = actual.input else { + anyhow::bail!("Expected items input"); + }; + + let assistant_item = items + .iter() + .find(|item| { + matches!( + item, + oai::InputItem::EasyMessage(msg) if msg.role == oai::Role::Assistant + ) + }) + .expect("Should have an assistant message"); + + let oai::InputItem::EasyMessage(msg) = assistant_item else { + anyhow::bail!("Expected EasyMessage"); + }; + + assert_eq!(msg.phase, Some(oai::MessagePhase::FinalAnswer)); + + Ok(()) + } + + #[test] + fn test_codex_request_no_phase_when_none() -> anyhow::Result<()> { + let context = ChatContext::default() + .add_message(ContextMessage::user("Hello", None)) + .add_message(ContextMessage::assistant("Response", None, None, None)) + .add_message(ContextMessage::user("Continue", None)); + + let actual = oai::CreateResponse::from_domain(context)?; + + let oai::InputParam::Items(items) = actual.input else { + anyhow::bail!("Expected items input"); + }; + + let assistant_item = items + .iter() + .find(|item| { + matches!( + item, + oai::InputItem::EasyMessage(msg) if msg.role == oai::Role::Assistant + ) + }) + .expect("Should have an assistant message"); + + let oai::InputItem::EasyMessage(msg) = assistant_item else { + anyhow::bail!("Expected EasyMessage"); + }; + + assert_eq!(msg.phase, None); + + Ok(()) + } } diff --git a/crates/forge_repo/src/provider/openai_responses/response.rs b/crates/forge_repo/src/provider/openai_responses/response.rs index 5fa4ca286e..33b6dd53c5 100644 --- a/crates/forge_repo/src/provider/openai_responses/response.rs +++ b/crates/forge_repo/src/provider/openai_responses/response.rs @@ -2,8 +2,8 @@ use std::collections::{HashMap, HashSet}; use async_openai::types::responses as oai; use forge_app::domain::{ - ChatCompletionMessage, Content, FinishReason, TokenCount, ToolCall, ToolCallArguments, - ToolCallFull, ToolCallId, ToolCallPart, ToolName, Usage, + ChatCompletionMessage, Content, FinishReason, MessagePhase, TokenCount, ToolCall, + ToolCallArguments, ToolCallFull, ToolCallId, ToolCallPart, ToolName, Usage, }; use forge_domain::{BoxStream, ResultStream}; use futures::StreamExt; @@ -97,6 +97,17 @@ impl IntoDomain for oai::ResponseUsage { } } +impl IntoDomain for oai::MessagePhase { + type Domain = MessagePhase; + + fn into_domain(self) -> Self::Domain { + match self { + oai::MessagePhase::Commentary => MessagePhase::Commentary, + oai::MessagePhase::FinalAnswer => MessagePhase::FinalAnswer, + } + } +} + impl IntoDomain for oai::Response { type Domain = ChatCompletionMessage; @@ -110,6 +121,12 @@ impl IntoDomain for oai::Response { let mut saw_tool_call = false; for item in &self.output { match item { + oai::OutputItem::Message(output_msg) => { + // Preserve phase from the assistant output message + if let Some(phase) = output_msg.phase { + message.phase = Some(phase.into_domain()); + } + } oai::OutputItem::FunctionCall(call) => { saw_tool_call = true; message = message.add_tool_call(ToolCall::Full(ToolCallFull { @@ -216,6 +233,28 @@ struct CodexStreamState { received_toolcall_deltas: HashSet, } +/// Retains only reasoning details that carry `encrypted_content` data. +/// +/// During streaming, reasoning text and summary parts are already emitted +/// via delta events. However, `encrypted_content` (type `reasoning.encrypted`) +/// is only available in the final `ResponseCompleted`/`ResponseIncomplete` +/// event. This function filters out text/summary reasoning details (which would +/// be duplicated) and keeps only the encrypted content entries that are required +/// for stateless multi-turn reasoning replay. +fn retain_encrypted_reasoning_details( + details: Option>, +) -> Option> { + let details = details?; + let encrypted: Vec = details + .into_iter() + .filter(|r| { + r.as_full() + .is_some_and(|fulls| fulls.iter().any(|f| f.type_of.as_deref() == Some("reasoning.encrypted"))) + }) + .collect(); + if encrypted.is_empty() { None } else { Some(encrypted) } +} + impl IntoDomain for BoxStream { type Domain = ResultStream; @@ -235,6 +274,7 @@ impl IntoDomain for BoxStream { .add_reasoning_detail(forge_domain::Reasoning::Part(vec![ forge_domain::ReasoningPart { text: Some(delta.delta), + id: Some(delta.item_id), type_of: Some("reasoning.text".to_string()), ..Default::default() }, @@ -246,6 +286,7 @@ impl IntoDomain for BoxStream { .add_reasoning_detail(forge_domain::Reasoning::Part(vec![ forge_domain::ReasoningPart { text: Some(delta.delta), + id: Some(delta.item_id), type_of: Some("reasoning.summary".to_string()), ..Default::default() }, @@ -361,7 +402,12 @@ impl IntoDomain for BoxStream { done.response.into_domain(); message.content = None; // Clear content to avoid duplication message.reasoning = None; // Clear reasoning to avoid duplication - message.reasoning_details = None; // Clear reasoning details to avoid duplication + // Keep only encrypted-content reasoning details — text and + // summary were already streamed via deltas but + // encrypted_content is never streamed and must be preserved + // for multi-turn reasoning replay. + message.reasoning_details = + retain_encrypted_reasoning_details(message.reasoning_details); message.tool_calls.clear(); // Clear tool calls to avoid duplication Some(Ok(message)) } @@ -372,7 +418,9 @@ impl IntoDomain for BoxStream { done.response.into_domain(); message.content = None; // Clear content to avoid duplication message.reasoning = None; // Clear reasoning to avoid duplication - message.reasoning_details = None; // Clear reasoning details to avoid duplication + // Keep only encrypted-content reasoning details (see above). + message.reasoning_details = + retain_encrypted_reasoning_details(message.reasoning_details); message.tool_calls.clear(); // Clear tool calls to avoid duplication message = message.finish_reason_opt(Some(FinishReason::Length)); Some(Ok(message)) @@ -784,6 +832,84 @@ mod tests { assert!(actual.tool_calls.is_empty()); } + #[test] + fn test_response_into_domain_preserves_commentary_phase() { + let fixture: oai::Response = serde_json::from_value(serde_json::json!({ + "id": "resp_1", + "created_at": 0, + "model": "codex-mini-latest", + "object": "response", + "status": "completed", + "output": [ + { + "id": "msg_1", + "type": "message", + "role": "assistant", + "phase": "commentary", + "content": [ + { + "type": "output_text", + "text": "Thinking...", + "annotations": [] + } + ], + "status": "completed" + } + ] + })) + .unwrap(); + let actual = fixture.into_domain(); + + assert_eq!( + actual.phase, + Some(forge_app::domain::MessagePhase::Commentary) + ); + assert_eq!(actual.content, Some(Content::full("Thinking..."))); + } + + #[test] + fn test_response_into_domain_preserves_final_answer_phase() { + let fixture: oai::Response = serde_json::from_value(serde_json::json!({ + "id": "resp_1", + "created_at": 0, + "model": "codex-mini-latest", + "object": "response", + "status": "completed", + "output": [ + { + "id": "msg_1", + "type": "message", + "role": "assistant", + "phase": "final_answer", + "content": [ + { + "type": "output_text", + "text": "The answer is 42.", + "annotations": [] + } + ], + "status": "completed" + } + ] + })) + .unwrap(); + let actual = fixture.into_domain(); + + assert_eq!( + actual.phase, + Some(forge_app::domain::MessagePhase::FinalAnswer) + ); + assert_eq!(actual.content, Some(Content::full("The answer is 42."))); + } + + #[test] + fn test_response_into_domain_no_phase_when_absent() { + let fixture = fixture_response_with_text("Hello"); + let actual = fixture.into_domain(); + + assert_eq!(actual.phase, None); + } + #[test] fn test_response_into_domain_with_function_call() { let fixture = @@ -925,6 +1051,7 @@ mod tests { actual.reasoning_details, Some(vec![Reasoning::Part(vec![forge_domain::ReasoningPart { text: Some("thinking...".to_string()), + id: Some("item_1".to_string()), type_of: Some("reasoning.text".to_string()), ..Default::default() }])]) @@ -949,6 +1076,7 @@ mod tests { actual.reasoning_details, Some(vec![Reasoning::Part(vec![forge_domain::ReasoningPart { text: Some("summary...".to_string()), + id: Some("item_1".to_string()), type_of: Some("reasoning.summary".to_string()), ..Default::default() }])]) From 0eb67f823894a930f5cbe80cb5bb62838dd55b59 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 28 Mar 2026 09:01:15 +0000 Subject: [PATCH 2/2] [autofix.ci] apply automated fixes --- crates/forge_domain/src/context.rs | 8 ++++---- crates/forge_domain/src/message.rs | 7 ++++--- crates/forge_domain/src/result_stream_ext.rs | 5 +---- .../src/provider/openai_responses/repository.rs | 6 +----- .../src/provider/openai_responses/request.rs | 12 ++++++------ .../src/provider/openai_responses/response.rs | 17 ++++++++++++----- 6 files changed, 28 insertions(+), 27 deletions(-) diff --git a/crates/forge_domain/src/context.rs b/crates/forge_domain/src/context.rs index 4f41f2adf5..d3018a56ce 100644 --- a/crates/forge_domain/src/context.rs +++ b/crates/forge_domain/src/context.rs @@ -14,13 +14,12 @@ fn is_false(value: &bool) -> bool { !value } -use crate::MessagePhase; use crate::temperature::Temperature; use crate::top_k::TopK; use crate::top_p::TopP; use crate::{ - Attachment, AttachmentContent, ConversationId, EventValue, Image, ModelId, ReasoningFull, - ToolChoice, ToolDefinition, ToolOutput, ToolValue, Usage, + Attachment, AttachmentContent, ConversationId, EventValue, Image, MessagePhase, ModelId, + ReasoningFull, ToolChoice, ToolDefinition, ToolOutput, ToolValue, Usage, }; /// Response format for structured output @@ -316,7 +315,8 @@ pub struct TextMessage { #[serde(default, skip_serializing_if = "is_false")] pub droppable: bool, /// Phase label for assistant messages (`Commentary` or `FinalAnswer`). - /// Preserved from OpenAI Responses API and replayed back on subsequent requests. + /// Preserved from OpenAI Responses API and replayed back on subsequent + /// requests. #[serde(default, skip_serializing_if = "Option::is_none")] pub phase: Option, } diff --git a/crates/forge_domain/src/message.rs b/crates/forge_domain/src/message.rs index 969cfba07d..fd5cecdbb8 100644 --- a/crates/forge_domain/src/message.rs +++ b/crates/forge_domain/src/message.rs @@ -10,8 +10,8 @@ use crate::reasoning::{Reasoning, ReasoningFull}; /// Labels an assistant message as intermediate commentary or the final answer. /// /// For models like `gpt-5.3-codex` and beyond, when sending follow-up requests, -/// preserve and resend phase on all assistant messages -- dropping it can degrade -/// performance. +/// preserve and resend phase on all assistant messages -- dropping it can +/// degrade performance. #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum MessagePhase { @@ -193,7 +193,8 @@ pub struct ChatCompletionMessageFull { pub reasoning_details: Option>, pub usage: Usage, pub finish_reason: Option, - /// Phase label for the assistant message (e.g. `Commentary` or `FinalAnswer`). + /// Phase label for the assistant message (e.g. `Commentary` or + /// `FinalAnswer`). pub phase: Option, } diff --git a/crates/forge_domain/src/result_stream_ext.rs b/crates/forge_domain/src/result_stream_ext.rs index 4da05195ac..f2597e878a 100644 --- a/crates/forge_domain/src/result_stream_ext.rs +++ b/crates/forge_domain/src/result_stream_ext.rs @@ -246,10 +246,7 @@ impl ResultStreamExt for crate::BoxStream OpenAIResponsesProvider { self.get_headers_for_conversation(None) } - fn get_headers_for_conversation( - &self, - conversation_id: Option<&str>, - ) -> Vec<(String, String)> { + fn get_headers_for_conversation(&self, conversation_id: Option<&str>) -> Vec<(String, String)> { let mut headers = Vec::new(); if let Some(api_key) = self .provider @@ -143,7 +140,6 @@ impl OpenAIResponsesProvider { headers } - } impl OpenAIResponsesProvider { diff --git a/crates/forge_repo/src/provider/openai_responses/request.rs b/crates/forge_repo/src/provider/openai_responses/request.rs index 975954d2c4..07546c5933 100644 --- a/crates/forge_repo/src/provider/openai_responses/request.rs +++ b/crates/forge_repo/src/provider/openai_responses/request.rs @@ -1297,9 +1297,9 @@ mod tests { let context = ChatContext::default() .add_message(ContextMessage::user("Hello", None)) - .add_entry(forge_app::domain::MessageEntry::from( - ContextMessage::Text(assistant_msg), - )) + .add_entry(forge_app::domain::MessageEntry::from(ContextMessage::Text( + assistant_msg, + ))) .add_message(ContextMessage::user("Continue", None)); let actual = oai::CreateResponse::from_domain(context)?; @@ -1338,9 +1338,9 @@ mod tests { let context = ChatContext::default() .add_message(ContextMessage::user("What is the answer?", None)) - .add_entry(forge_app::domain::MessageEntry::from( - ContextMessage::Text(assistant_msg), - )) + .add_entry(forge_app::domain::MessageEntry::from(ContextMessage::Text( + assistant_msg, + ))) .add_message(ContextMessage::user("Thanks", None)); let actual = oai::CreateResponse::from_domain(context)?; diff --git a/crates/forge_repo/src/provider/openai_responses/response.rs b/crates/forge_repo/src/provider/openai_responses/response.rs index 33b6dd53c5..900ffd0a5d 100644 --- a/crates/forge_repo/src/provider/openai_responses/response.rs +++ b/crates/forge_repo/src/provider/openai_responses/response.rs @@ -239,8 +239,8 @@ struct CodexStreamState { /// via delta events. However, `encrypted_content` (type `reasoning.encrypted`) /// is only available in the final `ResponseCompleted`/`ResponseIncomplete` /// event. This function filters out text/summary reasoning details (which would -/// be duplicated) and keeps only the encrypted content entries that are required -/// for stateless multi-turn reasoning replay. +/// be duplicated) and keeps only the encrypted content entries that are +/// required for stateless multi-turn reasoning replay. fn retain_encrypted_reasoning_details( details: Option>, ) -> Option> { @@ -248,11 +248,18 @@ fn retain_encrypted_reasoning_details( let encrypted: Vec = details .into_iter() .filter(|r| { - r.as_full() - .is_some_and(|fulls| fulls.iter().any(|f| f.type_of.as_deref() == Some("reasoning.encrypted"))) + r.as_full().is_some_and(|fulls| { + fulls + .iter() + .any(|f| f.type_of.as_deref() == Some("reasoning.encrypted")) + }) }) .collect(); - if encrypted.is_empty() { None } else { Some(encrypted) } + if encrypted.is_empty() { + None + } else { + Some(encrypted) + } } impl IntoDomain for BoxStream {