mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Reduce cloning when building Responses requests (#34825)
## Why Preparing Responses API requests rebuilt tool definitions as a generic JSON tree, and incremental WebSocket requests cloned their full item prefix for comparison. ## What changed - Serialize tool definitions into shared raw JSON that can be embedded directly in HTTP and WebSocket requests. - Compare incremental request prefixes in place while still ignoring internal message metadata. ## Testing - Verify raw tool JSON matches the existing value encoding. - Preserve the serialized WebSocket request payload. GitOrigin-RevId: 66e2921792c332e8904954dafa597f6d39abcf29
This commit is contained in:
@@ -19,7 +19,7 @@ http = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
schemars = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_json = { workspace = true, features = ["raw_value"] }
|
||||
thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["fs", "macros", "net", "rt", "sync", "time"] }
|
||||
tokio-tungstenite = { workspace = true }
|
||||
|
||||
@@ -12,8 +12,10 @@ use futures::Stream;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use serde_json::value::RawValue;
|
||||
use std::collections::HashMap;
|
||||
use std::pin::Pin;
|
||||
use std::sync::Arc;
|
||||
use std::task::Context;
|
||||
use std::task::Poll;
|
||||
use tokio::sync::mpsc;
|
||||
@@ -29,7 +31,7 @@ pub struct CompactionInput<'a> {
|
||||
#[serde(skip_serializing_if = "str::is_empty")]
|
||||
pub instructions: &'a str,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub tools: Option<Vec<Value>>,
|
||||
pub tools: Option<ResponsesApiTools>,
|
||||
pub parallel_tool_calls: bool,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reasoning: Option<Reasoning>,
|
||||
@@ -212,6 +214,40 @@ impl From<VerbosityConfig> for OpenAiVerbosity {
|
||||
}
|
||||
}
|
||||
|
||||
/// Serialized tool definitions for Responses API requests.
|
||||
///
|
||||
/// Keeping the tool list as raw JSON avoids rebuilding a generic JSON value
|
||||
/// tree, while the shared allocation keeps request clones cheap.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ResponsesApiTools(Arc<RawValue>);
|
||||
|
||||
impl ResponsesApiTools {
|
||||
pub(crate) fn as_raw_value(&self) -> &RawValue {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Arc<RawValue>> for ResponsesApiTools {
|
||||
fn from(value: Arc<RawValue>) -> Self {
|
||||
Self(value)
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for ResponsesApiTools {
|
||||
fn eq(&self, other: &Self) -> bool {
|
||||
Arc::ptr_eq(&self.0, &other.0) || self.0.get() == other.0.get()
|
||||
}
|
||||
}
|
||||
|
||||
impl Serialize for ResponsesApiTools {
|
||||
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: serde::Serializer,
|
||||
{
|
||||
self.0.serialize(serializer)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Clone, PartialEq)]
|
||||
pub struct ResponsesApiRequest {
|
||||
pub model: String,
|
||||
@@ -219,7 +255,7 @@ pub struct ResponsesApiRequest {
|
||||
pub instructions: String,
|
||||
pub input: Vec<ResponseItem>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub tools: Option<Vec<serde_json::Value>>,
|
||||
pub tools: Option<ResponsesApiTools>,
|
||||
pub tool_choice: String,
|
||||
pub parallel_tool_calls: bool,
|
||||
pub reasoning: Option<Reasoning>,
|
||||
@@ -245,7 +281,7 @@ impl<'a> From<&'a ResponsesApiRequest> for ResponseCreateWsRequest<'a> {
|
||||
instructions: &request.instructions,
|
||||
previous_response_id: None,
|
||||
input: &request.input,
|
||||
tools: request.tools.as_deref(),
|
||||
tools: request.tools.as_ref().map(ResponsesApiTools::as_raw_value),
|
||||
tool_choice: &request.tool_choice,
|
||||
parallel_tool_calls: request.parallel_tool_calls,
|
||||
reasoning: request.reasoning.as_ref(),
|
||||
@@ -271,7 +307,7 @@ pub struct ResponseCreateWsRequest<'a> {
|
||||
pub previous_response_id: Option<String>,
|
||||
pub input: &'a [ResponseItem],
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub tools: Option<&'a [Value]>,
|
||||
pub tools: Option<&'a RawValue>,
|
||||
pub tool_choice: &'a str,
|
||||
pub parallel_tool_calls: bool,
|
||||
pub reasoning: Option<&'a Reasoning>,
|
||||
|
||||
@@ -907,7 +907,10 @@ mod tests {
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use serde_json::value::RawValue;
|
||||
use serde_json::value::to_raw_value;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[test]
|
||||
fn direct_serialization_preserves_websocket_request_payload() {
|
||||
@@ -923,11 +926,17 @@ mod tests {
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}],
|
||||
tools: Some(vec![json!({
|
||||
"type": "function",
|
||||
"name": "lookup",
|
||||
"parameters": {"type": "object"}
|
||||
})]),
|
||||
tools: Some(
|
||||
Arc::<RawValue>::from(
|
||||
to_raw_value(&vec![json!({
|
||||
"type": "function",
|
||||
"name": "lookup",
|
||||
"parameters": {"type": "object"}
|
||||
})])
|
||||
.expect("serialize tools"),
|
||||
)
|
||||
.into(),
|
||||
),
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: true,
|
||||
reasoning: None,
|
||||
|
||||
@@ -39,6 +39,7 @@ pub use crate::common::ResponseCreateWsRequest;
|
||||
pub use crate::common::ResponseEvent;
|
||||
pub use crate::common::ResponseStream;
|
||||
pub use crate::common::ResponsesApiRequest;
|
||||
pub use crate::common::ResponsesApiTools;
|
||||
pub use crate::common::ResponsesWsRequest;
|
||||
pub use crate::common::StreamOptions;
|
||||
pub use crate::common::TextControls;
|
||||
|
||||
@@ -28,6 +28,7 @@ use http::HeaderMap;
|
||||
use http::HeaderValue;
|
||||
use http::StatusCode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::value::RawValue;
|
||||
|
||||
fn assert_path_ends_with(requests: &[Request], suffix: &str) {
|
||||
assert_eq!(requests.len(), 1);
|
||||
@@ -38,6 +39,10 @@ fn assert_path_ends_with(requests: &[Request], suffix: &str) {
|
||||
);
|
||||
}
|
||||
|
||||
fn empty_tools() -> Arc<RawValue> {
|
||||
Arc::from(RawValue::from_string("[]".to_string()).expect("valid tool JSON"))
|
||||
}
|
||||
|
||||
fn request_body_bytes(request: &Request) -> &[u8] {
|
||||
let Some(RequestBody::EncodedJson(body)) = request.body.as_ref() else {
|
||||
panic!("expected a prepared request body");
|
||||
@@ -316,7 +321,7 @@ async fn responses_client_stream_request_preserves_item_ids() -> Result<()> {
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}],
|
||||
tools: Some(Vec::new()),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".into(),
|
||||
parallel_tool_calls: false,
|
||||
reasoning: None,
|
||||
@@ -403,7 +408,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> {
|
||||
model: "gpt-test".into(),
|
||||
instructions: "Say hi".into(),
|
||||
input: Vec::new(),
|
||||
tools: Some(Vec::new()),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".into(),
|
||||
parallel_tool_calls: false,
|
||||
reasoning: None,
|
||||
@@ -523,7 +528,7 @@ async fn azure_store_sends_ids_and_headers() -> Result<()> {
|
||||
phase: None,
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}],
|
||||
tools: Some(Vec::new()),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".into(),
|
||||
parallel_tool_calls: false,
|
||||
reasoning: None,
|
||||
|
||||
+35
-20
@@ -89,6 +89,7 @@ use codex_rollout_trace::CompactionTraceContext;
|
||||
use codex_rollout_trace::InferenceTraceAttempt;
|
||||
use codex_rollout_trace::InferenceTraceContext;
|
||||
use codex_tools::create_tools_json_for_responses_api;
|
||||
use codex_tools::create_tools_raw_json_for_responses_api;
|
||||
use eventsource_stream::Event;
|
||||
use eventsource_stream::EventStreamError;
|
||||
use futures::StreamExt;
|
||||
@@ -358,6 +359,21 @@ fn responses_request_properties_match(
|
||||
&& previous_text == current_text
|
||||
}
|
||||
|
||||
fn response_items_equal_ignoring_internal_metadata(
|
||||
previous: &ResponseItem,
|
||||
current: &ResponseItem,
|
||||
) -> bool {
|
||||
if previous == current {
|
||||
return true;
|
||||
}
|
||||
|
||||
let mut previous = previous.clone();
|
||||
previous.clear_internal_chat_message_metadata_passthrough();
|
||||
let mut current = current.clone();
|
||||
current.clear_internal_chat_message_metadata_passthrough();
|
||||
previous == current
|
||||
}
|
||||
|
||||
impl WebsocketSession {
|
||||
fn set_connection_reused(&self, connection_reused: bool) {
|
||||
*self
|
||||
@@ -836,8 +852,8 @@ impl ModelClient {
|
||||
.iter_mut()
|
||||
.for_each(ResponseItem::clear_internal_chat_message_metadata_passthrough);
|
||||
}
|
||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||
let (instructions, tools) = if model_info.use_responses_lite {
|
||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||
let mut prefix = vec![ResponseItem::AdditionalTools {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
@@ -857,7 +873,10 @@ impl ModelClient {
|
||||
input.splice(0..0, prefix);
|
||||
(String::new(), None)
|
||||
} else {
|
||||
(prompt.base_instructions.text.clone(), Some(tools))
|
||||
(
|
||||
prompt.base_instructions.text.clone(),
|
||||
Some(create_tools_raw_json_for_responses_api(&prompt.tools)?.into()),
|
||||
)
|
||||
};
|
||||
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
||||
let stream_options = (self.state.concurrent_reasoning_summaries_enabled
|
||||
@@ -1167,29 +1186,25 @@ impl ModelClientSession {
|
||||
return None;
|
||||
}
|
||||
|
||||
// To compare the inputs, we concatenate the previous request items with the response items,
|
||||
// then compare that against the equivalent slice of request items, ignoring metadata. If
|
||||
// they match, we can consider the remaining items the incremental request.
|
||||
let mut previous_items = previous_request.input.clone();
|
||||
if let Some(response) = last_response {
|
||||
previous_items.extend_from_slice(&response.items_added);
|
||||
}
|
||||
previous_items
|
||||
.iter_mut()
|
||||
.for_each(ResponseItem::clear_internal_chat_message_metadata_passthrough);
|
||||
|
||||
let response_items =
|
||||
last_response.map_or(&[][..], |response| response.items_added.as_slice());
|
||||
let previous_items_len = previous_request
|
||||
.input
|
||||
.len()
|
||||
.checked_add(response_items.len())?;
|
||||
let Some((request_items_to_compare, incremental_items)) =
|
||||
request.input.split_at_checked(previous_items.len())
|
||||
request.input.split_at_checked(previous_items_len)
|
||||
else {
|
||||
trace!("incremental request failed, incompatible request length");
|
||||
return None;
|
||||
};
|
||||
let mut request_prefix = request_items_to_compare.to_vec();
|
||||
request_prefix
|
||||
.iter_mut()
|
||||
.for_each(ResponseItem::clear_internal_chat_message_metadata_passthrough);
|
||||
|
||||
if previous_items != request_prefix {
|
||||
let previous_items = previous_request.input.iter().chain(response_items);
|
||||
if !previous_items
|
||||
.zip(request_items_to_compare)
|
||||
.all(|(previous, current)| {
|
||||
response_items_equal_ignoring_internal_metadata(previous, current)
|
||||
})
|
||||
{
|
||||
trace!("incremental request failed, items didn't match");
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -6,9 +6,15 @@ use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ImageDetail;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::value::RawValue;
|
||||
use std::sync::Arc;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn empty_tools() -> Arc<RawValue> {
|
||||
Arc::from(RawValue::from_string("[]".to_string()).expect("valid tool JSON"))
|
||||
}
|
||||
|
||||
fn prompt_with_image_outputs() -> Prompt {
|
||||
Prompt {
|
||||
input: vec![
|
||||
@@ -105,12 +111,11 @@ fn responses_lite_request_copies_strip_image_details() {
|
||||
#[test]
|
||||
fn serializes_text_verbosity_when_set() {
|
||||
let input: Vec<ResponseItem> = vec![];
|
||||
let tools: Vec<serde_json::Value> = vec![];
|
||||
let req = ResponsesApiRequest {
|
||||
model: "gpt-5.4".to_string(),
|
||||
instructions: "i".to_string(),
|
||||
input,
|
||||
tools: Some(tools),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: true,
|
||||
reasoning: None,
|
||||
@@ -139,7 +144,6 @@ fn serializes_text_verbosity_when_set() {
|
||||
#[test]
|
||||
fn serializes_text_schema_with_strict_format() {
|
||||
let input: Vec<ResponseItem> = vec![];
|
||||
let tools: Vec<serde_json::Value> = vec![];
|
||||
let schema = serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -158,7 +162,7 @@ fn serializes_text_schema_with_strict_format() {
|
||||
model: "gpt-5.4".to_string(),
|
||||
instructions: "i".to_string(),
|
||||
input,
|
||||
tools: Some(tools),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: true,
|
||||
reasoning: None,
|
||||
@@ -215,12 +219,11 @@ fn serializes_text_schema_with_non_strict_format() {
|
||||
#[test]
|
||||
fn omits_text_when_not_set() {
|
||||
let input: Vec<ResponseItem> = vec![];
|
||||
let tools: Vec<serde_json::Value> = vec![];
|
||||
let req = ResponsesApiRequest {
|
||||
model: "gpt-5.4".to_string(),
|
||||
instructions: "i".to_string(),
|
||||
input,
|
||||
tools: Some(tools),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: true,
|
||||
reasoning: None,
|
||||
@@ -244,7 +247,7 @@ fn serializes_flex_service_tier_when_set() {
|
||||
model: "gpt-5.4".to_string(),
|
||||
instructions: "i".to_string(),
|
||||
input: vec![],
|
||||
tools: Some(vec![]),
|
||||
tools: Some(empty_tools().into()),
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: true,
|
||||
reasoning: None,
|
||||
|
||||
@@ -72,7 +72,7 @@ impl ToolRouter {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn model_visible_specs(&self) -> Vec<ToolSpec> {
|
||||
pub(crate) fn model_visible_specs(&self) -> Vec<ToolSpec> {
|
||||
self.model_visible_specs.clone()
|
||||
}
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ rmcp = { workspace = true, default-features = false, features = [
|
||||
"server",
|
||||
] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_json = { workspace = true, features = ["raw_value"] }
|
||||
thiserror = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
urlencoding = { workspace = true }
|
||||
|
||||
@@ -105,3 +105,4 @@ pub use tool_spec::ResponsesApiWebSearchFilters;
|
||||
pub use tool_spec::ResponsesApiWebSearchUserLocation;
|
||||
pub use tool_spec::ToolSpec;
|
||||
pub use tool_spec::create_tools_json_for_responses_api;
|
||||
pub use tool_spec::create_tools_raw_json_for_responses_api;
|
||||
|
||||
@@ -9,6 +9,8 @@ use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLo
|
||||
use codex_protocol::config_types::WebSearchUserLocationType;
|
||||
use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use serde_json::value::RawValue;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// When serialized as JSON, this produces a valid "Tool" in the OpenAI
|
||||
/// Responses API.
|
||||
@@ -87,6 +89,13 @@ pub fn create_tools_json_for_responses_api(
|
||||
Ok(tools_json)
|
||||
}
|
||||
|
||||
/// Returns raw JSON that can be embedded directly in a Responses API request.
|
||||
pub fn create_tools_raw_json_for_responses_api(
|
||||
tools: &[ToolSpec],
|
||||
) -> Result<Arc<RawValue>, serde_json::Error> {
|
||||
serde_json::value::to_raw_value(tools).map(Arc::from)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub struct ResponsesApiWebSearchFilters {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::JsonSchema;
|
||||
use crate::ResponsesApiNamespaceTool;
|
||||
use crate::ResponsesApiTool;
|
||||
use crate::create_tools_json_for_responses_api;
|
||||
use crate::create_tools_raw_json_for_responses_api;
|
||||
use codex_protocol::config_types::WebSearchContextSize;
|
||||
use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters;
|
||||
use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation;
|
||||
@@ -143,6 +144,29 @@ fn create_tools_json_for_responses_api_includes_top_level_name() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn raw_tool_json_matches_value_encoding() {
|
||||
let specs = vec![ToolSpec::Function(ResponsesApiTool {
|
||||
name: "demo".to_string(),
|
||||
description: "A demo tool".to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(
|
||||
BTreeMap::new(),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None,
|
||||
),
|
||||
output_schema: None,
|
||||
})];
|
||||
let expected = create_tools_json_for_responses_api(&specs).expect("serialize tools");
|
||||
let raw = create_tools_raw_json_for_responses_api(&specs).expect("serialize raw tools");
|
||||
|
||||
assert_eq!(
|
||||
serde_json::from_str::<Vec<serde_json::Value>>(raw.get()).expect("parse raw tools"),
|
||||
expected,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn namespace_tool_spec_serializes_expected_wire_shape() {
|
||||
assert_eq!(
|
||||
|
||||
Reference in New Issue
Block a user