mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core: share responses request builder with compact requests (#20989)
## Why `ModelClientSession` and `compact_conversation_history()` were still rebuilding the same `ResponsesApiRequest` fields separately. That duplication makes it easy for normal `/responses` turns and compact requests to drift when request-shape changes land later, which is exactly the kind of cache-affecting divergence we want to avoid. This follow-up keeps the scope small by extracting the shared request-construction logic into one helper and using it from both paths. ## What changed - move `ResponsesApiRequest` construction into a shared `ModelClient::build_responses_request(...)` helper in `core/src/client.rs` - update the normal `/responses` streaming path to call that helper instead of the old `ModelClientSession`-local implementation - update `compact_conversation_history()` to derive its compact payload from the same helper so `model`, `instructions`, `input`, `tools`, `parallel_tool_calls`, `reasoning`, and `text` stay aligned with normal request building - add a unit test covering the shared helper's prompt cache key, installation metadata, and `service_tier` behavior ## Verification - `cargo test -p codex-core build_responses_request_sets_shared_cache_and_metadata_fields` - `cargo test -p codex-core --test all remote_compact_v2_reuses_context_compaction_for_followups` ## Docs No docs update needed.
This commit is contained in:
jif-oai
committed by
GitHub
Unverified
GitHub
parent
4fd7dfe223
commit
e3451ce6be
+83
-101
@@ -433,36 +433,33 @@ impl ModelClient {
|
||||
RequestRouteTelemetry::for_endpoint(RESPONSES_COMPACT_ENDPOINT),
|
||||
self.state.auth_env_telemetry.clone(),
|
||||
);
|
||||
let request = self.build_responses_request(
|
||||
&client_setup.api_provider,
|
||||
prompt,
|
||||
model_info,
|
||||
effort,
|
||||
summary,
|
||||
/*service_tier*/ None,
|
||||
)?;
|
||||
let ResponsesApiRequest {
|
||||
model,
|
||||
instructions,
|
||||
input,
|
||||
tools,
|
||||
parallel_tool_calls,
|
||||
reasoning,
|
||||
text,
|
||||
..
|
||||
} = request;
|
||||
let client =
|
||||
ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth)
|
||||
.with_telemetry(Some(request_telemetry));
|
||||
|
||||
let instructions = prompt.base_instructions.text.clone();
|
||||
let input = prompt.get_formatted_input();
|
||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
||||
let verbosity = if model_info.support_verbosity {
|
||||
self.state.model_verbosity.or(model_info.default_verbosity)
|
||||
} else {
|
||||
if self.state.model_verbosity.is_some() {
|
||||
warn!(
|
||||
"model_verbosity is set but ignored as the model does not support verbosity: {}",
|
||||
model_info.slug
|
||||
);
|
||||
}
|
||||
None
|
||||
};
|
||||
let text = create_text_param_for_request(
|
||||
verbosity,
|
||||
&prompt.output_schema,
|
||||
prompt.output_schema_strict,
|
||||
);
|
||||
let payload = ApiCompactionInput {
|
||||
model: &model_info.slug,
|
||||
model: &model,
|
||||
input: &input,
|
||||
instructions: &instructions,
|
||||
tools,
|
||||
parallel_tool_calls: prompt.parallel_tool_calls,
|
||||
parallel_tool_calls,
|
||||
reasoning,
|
||||
text,
|
||||
};
|
||||
@@ -664,6 +661,67 @@ impl ModelClient {
|
||||
}
|
||||
}
|
||||
|
||||
fn build_responses_request(
|
||||
&self,
|
||||
provider: &codex_api::Provider,
|
||||
prompt: &Prompt,
|
||||
model_info: &ModelInfo,
|
||||
effort: Option<ReasoningEffortConfig>,
|
||||
summary: ReasoningSummaryConfig,
|
||||
service_tier: Option<ServiceTier>,
|
||||
) -> Result<ResponsesApiRequest> {
|
||||
let instructions = &prompt.base_instructions.text;
|
||||
let input = prompt.get_formatted_input();
|
||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
||||
let include = if reasoning.is_some() {
|
||||
vec!["reasoning.encrypted_content".to_string()]
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let verbosity = if model_info.support_verbosity {
|
||||
self.state.model_verbosity.or(model_info.default_verbosity)
|
||||
} else {
|
||||
if self.state.model_verbosity.is_some() {
|
||||
warn!(
|
||||
"model_verbosity is set but ignored as the model does not support verbosity: {}",
|
||||
model_info.slug
|
||||
);
|
||||
}
|
||||
None
|
||||
};
|
||||
let text = create_text_param_for_request(
|
||||
verbosity,
|
||||
&prompt.output_schema,
|
||||
prompt.output_schema_strict,
|
||||
);
|
||||
let prompt_cache_key = Some(self.state.conversation_id.to_string());
|
||||
let request = ResponsesApiRequest {
|
||||
model: model_info.slug.clone(),
|
||||
instructions: instructions.clone(),
|
||||
input,
|
||||
tools,
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: prompt.parallel_tool_calls,
|
||||
reasoning,
|
||||
store: provider.is_azure_responses_endpoint(),
|
||||
stream: true,
|
||||
include,
|
||||
service_tier: match service_tier {
|
||||
Some(ServiceTier::Fast) => Some("priority".to_string()),
|
||||
Some(service_tier) => Some(service_tier.to_string()),
|
||||
None => None,
|
||||
},
|
||||
prompt_cache_key,
|
||||
text,
|
||||
client_metadata: Some(HashMap::from([(
|
||||
X_CODEX_INSTALLATION_ID_HEADER.to_string(),
|
||||
self.state.installation_id.clone(),
|
||||
)])),
|
||||
};
|
||||
Ok(request)
|
||||
}
|
||||
|
||||
/// Returns whether the Responses-over-WebSocket transport is active for this session.
|
||||
///
|
||||
/// WebSocket use is controlled by provider capability and session-scoped fallback state.
|
||||
@@ -833,82 +891,6 @@ impl ModelClientSession {
|
||||
.set_connection_reused(/*connection_reused*/ false);
|
||||
}
|
||||
|
||||
fn build_responses_request(
|
||||
&self,
|
||||
provider: &codex_api::Provider,
|
||||
prompt: &Prompt,
|
||||
model_info: &ModelInfo,
|
||||
effort: Option<ReasoningEffortConfig>,
|
||||
summary: ReasoningSummaryConfig,
|
||||
service_tier: Option<ServiceTier>,
|
||||
) -> Result<ResponsesApiRequest> {
|
||||
let instructions = &prompt.base_instructions.text;
|
||||
let input = prompt.get_formatted_input();
|
||||
let tools = create_tools_json_for_responses_api(&prompt.tools)?;
|
||||
let default_reasoning_effort = model_info.default_reasoning_level;
|
||||
let reasoning = if model_info.supports_reasoning_summaries {
|
||||
Some(Reasoning {
|
||||
effort: effort.or(default_reasoning_effort),
|
||||
summary: if summary == ReasoningSummaryConfig::None {
|
||||
None
|
||||
} else {
|
||||
Some(summary)
|
||||
},
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let include = if reasoning.is_some() {
|
||||
vec!["reasoning.encrypted_content".to_string()]
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
let verbosity = if model_info.support_verbosity {
|
||||
self.client
|
||||
.state
|
||||
.model_verbosity
|
||||
.or(model_info.default_verbosity)
|
||||
} else {
|
||||
if self.client.state.model_verbosity.is_some() {
|
||||
warn!(
|
||||
"model_verbosity is set but ignored as the model does not support verbosity: {}",
|
||||
model_info.slug
|
||||
);
|
||||
}
|
||||
None
|
||||
};
|
||||
let text = create_text_param_for_request(
|
||||
verbosity,
|
||||
&prompt.output_schema,
|
||||
prompt.output_schema_strict,
|
||||
);
|
||||
let prompt_cache_key = Some(self.client.state.conversation_id.to_string());
|
||||
let request = ResponsesApiRequest {
|
||||
model: model_info.slug.clone(),
|
||||
instructions: instructions.clone(),
|
||||
input,
|
||||
tools,
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: prompt.parallel_tool_calls,
|
||||
reasoning,
|
||||
store: provider.is_azure_responses_endpoint(),
|
||||
stream: true,
|
||||
include,
|
||||
service_tier: match service_tier {
|
||||
Some(ServiceTier::Fast) => Some("priority".to_string()),
|
||||
Some(service_tier) => Some(service_tier.to_string()),
|
||||
None => None,
|
||||
},
|
||||
prompt_cache_key,
|
||||
text,
|
||||
client_metadata: Some(HashMap::from([(
|
||||
X_CODEX_INSTALLATION_ID_HEADER.to_string(),
|
||||
self.client.state.installation_id.clone(),
|
||||
)])),
|
||||
};
|
||||
Ok(request)
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
/// Builds shared Responses API transport options and request-body options.
|
||||
///
|
||||
@@ -1209,7 +1191,7 @@ impl ModelClientSession {
|
||||
let compression = self.responses_request_compression(client_setup.auth.as_ref());
|
||||
let options = self.build_responses_options(turn_metadata_header, compression);
|
||||
|
||||
let request = self.build_responses_request(
|
||||
let request = self.client.build_responses_request(
|
||||
&client_setup.api_provider,
|
||||
prompt,
|
||||
model_info,
|
||||
@@ -1315,7 +1297,7 @@ impl ModelClientSession {
|
||||
let compression = self.responses_request_compression(client_setup.auth.as_ref());
|
||||
|
||||
let options = self.build_responses_options(turn_metadata_header, compression);
|
||||
let request = self.build_responses_request(
|
||||
let request = self.client.build_responses_request(
|
||||
&client_setup.api_provider,
|
||||
prompt,
|
||||
model_info,
|
||||
|
||||
@@ -770,10 +770,15 @@ async fn includes_conversation_id_and_model_headers_in_request() {
|
||||
let installation_id =
|
||||
std::fs::read_to_string(test.codex_home_path().join(INSTALLATION_ID_FILENAME))
|
||||
.expect("read installation id");
|
||||
let session_id_string = session_id.to_string();
|
||||
|
||||
assert_eq!(request_session_id, session_id.to_string());
|
||||
assert_eq!(request_session_id, session_id_string);
|
||||
assert_eq!(request_originator, originator().value);
|
||||
assert_eq!(request_authorization, "Bearer Test API Key");
|
||||
assert_eq!(
|
||||
request_body["prompt_cache_key"].as_str(),
|
||||
Some(session_id_string.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
request_body["client_metadata"]["x-codex-installation-id"].as_str(),
|
||||
Some(installation_id.as_str())
|
||||
|
||||
Reference in New Issue
Block a user