mirror of
https://github.com/openai/codex.git
synced 2026-11-03 16:23:51 +00:00
Avoid cloning Responses WebSocket payloads (#34381)
## Why Building a Responses WebSocket request copied the model, instructions, input history, tools, and other request settings into a second payload before serialization. ## What changed - Make `ResponseCreateWsRequest` borrow shared fields from `ResponsesApiRequest`. - Keep only WebSocket-specific fields owned, and allocate a separate input vector only for incremental continuations. - Restore response item IDs after preparing a full request so the request retained for continuation matching keeps its original state. ## Testing Update the serialization test to verify that the borrowed WebSocket request produces the same wire payload as the Responses API request plus its WebSocket-specific fields. GitOrigin-RevId: b63d746b7f5164b5204ab95b057c914a4aeb71e1
This commit is contained in:
1 parent
8431dc590a
commit
6b9a5592a6
3 files changed
+105
-80
No files matched your search
+61
-44
@@ -1222,11 +1222,10 @@ impl ModelClientSession {
|
||||
|
||||
fn prepare_websocket_request(
|
||||
&mut self,
|
||||
payload: ResponseCreateWsRequest,
|
||||
request: &ResponsesApiRequest,
|
||||
) -> (ResponsesWsRequest, bool) {
|
||||
) -> (Option<(String, Vec<ResponseItem>)>, bool) {
|
||||
let Some(last_response) = self.get_last_response() else {
|
||||
return (ResponsesWsRequest::ResponseCreate(payload), false);
|
||||
return (None, false);
|
||||
};
|
||||
let previous_response_id_from_untraced_warmup =
|
||||
self.websocket_session.last_response_from_untraced_warmup;
|
||||
@@ -1235,20 +1234,16 @@ impl ModelClientSession {
|
||||
Some(&last_response),
|
||||
/*allow_empty_delta*/ true,
|
||||
) else {
|
||||
return (ResponsesWsRequest::ResponseCreate(payload), false);
|
||||
return (None, false);
|
||||
};
|
||||
|
||||
if last_response.response_id.is_empty() {
|
||||
trace!("incremental request failed, no previous response id");
|
||||
return (ResponsesWsRequest::ResponseCreate(payload), false);
|
||||
return (None, false);
|
||||
}
|
||||
|
||||
(
|
||||
ResponsesWsRequest::ResponseCreate(ResponseCreateWsRequest {
|
||||
previous_response_id: Some(last_response.response_id),
|
||||
input: incremental_items,
|
||||
..payload
|
||||
}),
|
||||
Some((last_response.response_id, incremental_items)),
|
||||
previous_response_id_from_untraced_warmup,
|
||||
)
|
||||
}
|
||||
@@ -1548,7 +1543,7 @@ impl ModelClientSession {
|
||||
client_setup.agent_identity_telemetry.clone(),
|
||||
pending_retry,
|
||||
);
|
||||
let request = self.client.build_responses_request(
|
||||
let mut request = self.client.build_responses_request(
|
||||
&client_setup.api_provider,
|
||||
prompt,
|
||||
model_info,
|
||||
@@ -1569,17 +1564,6 @@ impl ModelClientSession {
|
||||
if let Some(turn_state) = self.turn_state.get() {
|
||||
client_metadata.insert(X_CODEX_TURN_STATE_HEADER.to_string(), turn_state.clone());
|
||||
}
|
||||
let mut ws_payload = ResponseCreateWsRequest {
|
||||
client_metadata: response_create_client_metadata(
|
||||
Some(client_metadata),
|
||||
request_trace.as_ref(),
|
||||
),
|
||||
..ResponseCreateWsRequest::from(&request)
|
||||
};
|
||||
if warmup {
|
||||
ws_payload.generate = Some(false);
|
||||
}
|
||||
|
||||
match self
|
||||
.websocket_connection(WebsocketConnectParams {
|
||||
session_telemetry,
|
||||
@@ -1616,8 +1600,8 @@ impl ModelClientSession {
|
||||
Err(err) => return Err(self.client.state.provider.map_api_error(err)),
|
||||
}
|
||||
|
||||
let (mut ws_request, previous_response_id_from_untraced_warmup) =
|
||||
self.prepare_websocket_request(ws_payload, &request);
|
||||
let (incremental_request, previous_response_id_from_untraced_warmup) =
|
||||
self.prepare_websocket_request(&request);
|
||||
let inference_trace_attempt = if warmup {
|
||||
// Prewarm sends `generate=false`; it is connection setup, not a
|
||||
// model inference attempt that should appear in rollout traces.
|
||||
@@ -1625,21 +1609,48 @@ impl ModelClientSession {
|
||||
} else {
|
||||
inference_trace.start_attempt()
|
||||
};
|
||||
stamp_ws_stream_request_start_ms(&mut ws_request);
|
||||
let ResponsesWsRequest::ResponseCreate(ws_payload) = &mut ws_request;
|
||||
let store = ws_payload.store;
|
||||
self.client
|
||||
.prepare_response_items_for_request(&mut ws_payload.input, store);
|
||||
if previous_response_id_from_untraced_warmup {
|
||||
// The transport can reuse an untraced warmup response id and omit the
|
||||
// already-sent input, but rollout replay needs the logical model-visible
|
||||
// request rather than the compressed websocket delta.
|
||||
inference_trace_attempt.record_started(&request);
|
||||
}
|
||||
|
||||
let (previous_response_id, mut incremental_items) = match incremental_request {
|
||||
Some((response_id, items)) => (Some(response_id), Some(items)),
|
||||
None => (None, None),
|
||||
};
|
||||
let store = request.store;
|
||||
let original_item_ids = if let Some(incremental_items) = &mut incremental_items {
|
||||
self.client
|
||||
.prepare_response_items_for_request(incremental_items, store);
|
||||
None
|
||||
} else {
|
||||
let original_item_ids = request
|
||||
.input
|
||||
.iter()
|
||||
.map(|item| item.id().cloned())
|
||||
.collect::<Vec<_>>();
|
||||
self.client
|
||||
.prepare_response_items_for_request(&mut request.input, store);
|
||||
Some(original_item_ids)
|
||||
};
|
||||
let ws_payload = ResponseCreateWsRequest {
|
||||
previous_response_id,
|
||||
input: incremental_items.as_deref().unwrap_or(&request.input),
|
||||
generate: if warmup { Some(false) } else { None },
|
||||
client_metadata: response_create_client_metadata(
|
||||
Some(client_metadata),
|
||||
request_trace.as_ref(),
|
||||
),
|
||||
..ResponseCreateWsRequest::from(&request)
|
||||
};
|
||||
let mut ws_request = ResponsesWsRequest::ResponseCreate(ws_payload);
|
||||
stamp_ws_stream_request_start_ms(&mut ws_request);
|
||||
if !previous_response_id_from_untraced_warmup {
|
||||
inference_trace_attempt.record_started(&ws_request);
|
||||
}
|
||||
self.websocket_session.last_request = Some(request);
|
||||
self.websocket_session.last_response_from_untraced_warmup = warmup;
|
||||
|
||||
let websocket_connection =
|
||||
self.websocket_session.connection.as_ref().ok_or_else(|| {
|
||||
self.client.state.provider.map_api_error(ApiError::Stream(
|
||||
@@ -1652,18 +1663,24 @@ impl ModelClientSession {
|
||||
self.websocket_session.connection_reused(),
|
||||
Some(Arc::clone(&self.turn_state)),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
let response_debug_context =
|
||||
extract_response_debug_context_from_api_error(&err);
|
||||
let err = self.client.state.provider.map_api_error(err);
|
||||
inference_trace_attempt.record_failed(
|
||||
&err,
|
||||
response_debug_context.request_id.as_deref(),
|
||||
/*output_items*/ &[],
|
||||
);
|
||||
err
|
||||
})?;
|
||||
.await;
|
||||
if let Some(original_item_ids) = original_item_ids {
|
||||
for (item, original_item_id) in request.input.iter_mut().zip(original_item_ids) {
|
||||
item.set_id(original_item_id);
|
||||
}
|
||||
}
|
||||
self.websocket_session.last_request = Some(request);
|
||||
self.websocket_session.last_response_from_untraced_warmup = warmup;
|
||||
let stream_result = stream_result.map_err(|err| {
|
||||
let response_debug_context = extract_response_debug_context_from_api_error(&err);
|
||||
let err = self.client.state.provider.map_api_error(err);
|
||||
inference_trace_attempt.record_failed(
|
||||
&err,
|
||||
response_debug_context.request_id.as_deref(),
|
||||
/*output_items*/ &[],
|
||||
);
|
||||
err
|
||||
})?;
|
||||
let (stream, last_request_rx) = map_response_stream(
|
||||
stream_result,
|
||||
request_session_telemetry,
|
||||
@@ -1848,7 +1865,7 @@ impl ModelClientSession {
|
||||
///
|
||||
/// Meant to be called just before sending the request over the socket, to capture realistic
|
||||
/// transport timing.
|
||||
fn stamp_ws_stream_request_start_ms(request: &mut ResponsesWsRequest) {
|
||||
fn stamp_ws_stream_request_start_ms(request: &mut ResponsesWsRequest<'_>) {
|
||||
let ResponsesWsRequest::ResponseCreate(payload) = request;
|
||||
payload
|
||||
.client_metadata
|
||||
|
||||
Reference in new issue
Block a user