mirror of
https://github.com/openai/codex.git
synced 2026-09-28 08:43:01 +08:00
Filter saved reasoning overrides from requests when disabled (#46291)
## Why Resuming a thread after disabling reasoning effort overrides still sent saved `configuration_update` items until compaction. Disabling the feature should also apply to requests built from existing history. ## What changed Pass the `ReasoningEffortOverride` feature state into `ModelClient` and filter `configuration_update` items from request input when disabled. Preserve persisted history and other input items while using the request-level reasoning effort. ## Testing Add regression coverage for resumed threads, compaction, and WebSocket warmup and turn requests. Verify that saved updates remain in history and `agent_message` items remain in request input. GitOrigin-RevId: aac3257da1ca38a0744807c4f7cb12565bd7fe53
This commit is contained in:
@@ -203,6 +203,7 @@ struct ModelClientState {
|
||||
originator: String,
|
||||
model_verbosity: Option<VerbosityConfig>,
|
||||
content_item_kinds_enabled: bool,
|
||||
reasoning_effort_override_enabled: bool,
|
||||
enable_request_compression: bool,
|
||||
include_timing_metrics: bool,
|
||||
beta_features_header: Option<String>,
|
||||
@@ -476,6 +477,7 @@ impl ModelClient {
|
||||
originator: String,
|
||||
model_verbosity: Option<VerbosityConfig>,
|
||||
content_item_kinds_enabled: bool,
|
||||
reasoning_effort_override_enabled: bool,
|
||||
enable_request_compression: bool,
|
||||
include_timing_metrics: bool,
|
||||
beta_features_header: Option<String>,
|
||||
@@ -502,6 +504,7 @@ impl ModelClient {
|
||||
originator,
|
||||
model_verbosity,
|
||||
content_item_kinds_enabled,
|
||||
reasoning_effort_override_enabled,
|
||||
enable_request_compression,
|
||||
include_timing_metrics,
|
||||
beta_features_header,
|
||||
@@ -856,6 +859,11 @@ impl ModelClient {
|
||||
responses_metadata: &CodexResponsesMetadata,
|
||||
) -> Result<ResponsesApiRequest> {
|
||||
let mut input = prompt.get_formatted_input_for_request(model_info);
|
||||
if !self.state.reasoning_effort_override_enabled {
|
||||
// Disabling overrides must also recover threads with saved updates.
|
||||
// Filter only the request copy; persisted history remains unchanged.
|
||||
input.retain(|item| !matches!(item, ResponseItem::ConfigurationUpdate { .. }));
|
||||
}
|
||||
let is_openai = self.state.provider.info().is_openai();
|
||||
let (instructions, tools) = if model_info.use_responses_lite {
|
||||
// These prompt-only items are rebuilt on every request. Hash their visible payloads
|
||||
|
||||
@@ -115,6 +115,7 @@ fn test_model_client_with_thread_id(
|
||||
"test_originator".to_string(),
|
||||
/*model_verbosity*/ None,
|
||||
/*content_item_kinds_enabled*/ true,
|
||||
/*reasoning_effort_override_enabled*/ false,
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
@@ -1631,6 +1632,7 @@ fn model_client_with_counting_attestation(
|
||||
"test_originator".to_string(),
|
||||
/*model_verbosity*/ None,
|
||||
/*content_item_kinds_enabled*/ true,
|
||||
/*reasoning_effort_override_enabled*/ false,
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
|
||||
@@ -1677,6 +1677,7 @@ impl Session {
|
||||
session_configuration.originator.clone(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Self::build_model_client_beta_features_header(config.as_ref()),
|
||||
|
||||
@@ -794,6 +794,7 @@ fn test_model_client_session() -> crate::client::ModelClientSession {
|
||||
"test_originator".to_string(),
|
||||
/*model_verbosity*/ None,
|
||||
/*content_item_kinds_enabled*/ true,
|
||||
/*reasoning_effort_override_enabled*/ false,
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
@@ -6177,6 +6178,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
session_configuration.originator.clone(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
@@ -8363,6 +8365,7 @@ where
|
||||
session_configuration.originator.clone(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
|
||||
@@ -130,6 +130,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
@@ -268,6 +269,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
@@ -391,6 +393,7 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
|
||||
@@ -1662,6 +1662,7 @@ async fn send_request_with_provider(provider: ModelProviderInfo) {
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
@@ -3157,6 +3158,7 @@ async fn azure_responses_request_does_not_store_and_preserves_prefixed_item_ids(
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
/*include_timing_metrics*/ false,
|
||||
/*beta_features_header*/ None,
|
||||
|
||||
@@ -2873,6 +2873,7 @@ async fn websocket_harness_with_provider_options_and_auth(
|
||||
"test_originator".to_string(),
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
/*enable_request_compression*/ false,
|
||||
runtime_metrics_enabled,
|
||||
/*beta_features_header*/ None,
|
||||
|
||||
@@ -4,10 +4,13 @@ use codex_core::ForkSnapshot;
|
||||
use codex_core::RecoverTurnRequest;
|
||||
use codex_core::StartIfIdleSubmission;
|
||||
use codex_core::SuspendTurnOutcome;
|
||||
use codex_core::TurnInput;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::TurnInputSubmission;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_history::ResponseItemEnvelope;
|
||||
use codex_history::RolloutItem;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
@@ -589,6 +592,72 @@ async fn reasoning_effort_override_websocket_appends_then_replays_after_reconnec
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn reasoning_effort_override_disabled_filters_websocket_history() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = responses::start_mock_server().await;
|
||||
let initial_mock = responses::mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![responses::ev_completed("initial")]),
|
||||
)
|
||||
.await;
|
||||
let initial = override_builder().build_with_auto_env(&server).await?;
|
||||
initial
|
||||
.submit_text_turn("before disabling overrides")
|
||||
.await?;
|
||||
assert_eq!(
|
||||
effort_updates(&initial_mock.single_request()),
|
||||
vec![effort_update(ReasoningEffort::Medium)]
|
||||
);
|
||||
let websocket = responses::start_websocket_server(vec![vec![
|
||||
vec![
|
||||
responses::ev_response_created("warmup"),
|
||||
responses::ev_completed("warmup"),
|
||||
],
|
||||
vec![
|
||||
responses::ev_response_created("resumed"),
|
||||
responses::ev_completed("resumed"),
|
||||
],
|
||||
]])
|
||||
.await;
|
||||
let base_url = format!("{}/v1", websocket.uri());
|
||||
let mut builder = override_builder().with_config(move |config| {
|
||||
config
|
||||
.features
|
||||
.disable(Feature::ReasoningEffortOverride)
|
||||
.expect("disable overrides");
|
||||
config.model_reasoning_effort = Some(ReasoningEffort::High);
|
||||
config.model_provider.base_url = Some(base_url);
|
||||
config.model_provider.supports_websockets = true;
|
||||
});
|
||||
if let Some(url) = initial.executor_environment().exec_server_url() {
|
||||
builder = builder.with_exec_server_url(url);
|
||||
}
|
||||
let test = builder.restart(&server, &initial).await?;
|
||||
let warmup = tokio::time::timeout(
|
||||
Duration::from_secs(/*secs*/ 10),
|
||||
websocket.wait_for_request(/*connection_index*/ 0, /*request_index*/ 0),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(warmup.body_json()["generate"], false);
|
||||
test.submit_text_turn("after disabling overrides").await?;
|
||||
let requests = websocket.single_connection();
|
||||
assert_eq!(requests.len(), 2);
|
||||
for request in requests {
|
||||
let body = request.body_json();
|
||||
assert_eq!(body["reasoning"]["effort"], "high");
|
||||
assert!(
|
||||
body["input"]
|
||||
.as_array()
|
||||
.expect("request input")
|
||||
.iter()
|
||||
.all(|item| item["type"] != "configuration_update")
|
||||
);
|
||||
}
|
||||
websocket.shutdown().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum OverrideUnavailable {
|
||||
FeatureDisabled,
|
||||
@@ -652,8 +721,7 @@ async fn reasoning_effort_override_unavailable_uses_request_effort(
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn reasoning_effort_override_disabled_on_resume_retires_update_at_compaction()
|
||||
-> anyhow::Result<()> {
|
||||
async fn reasoning_effort_override_disabled_recovers_saved_history() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = responses::start_mock_server().await;
|
||||
let reply = |id, text| {
|
||||
@@ -679,9 +747,25 @@ async fn reasoning_effort_override_disabled_on_resume_retires_update_at_compacti
|
||||
)
|
||||
.await;
|
||||
let initial = override_builder().build_with_auto_env(&server).await?;
|
||||
initial.submit_text_turn("before resume").await?;
|
||||
let agent_message = serde_json::from_value(serde_json::json!({
|
||||
"type": "agent_message",
|
||||
"author": "/root/worker",
|
||||
"recipient": "/root",
|
||||
"content": [{"type": "input_text", "text": "The worker has finished."}],
|
||||
}))?;
|
||||
let submission = initial
|
||||
.codex
|
||||
.start_turn_if_idle(TurnInputRequest::new(TurnInput::ResponseItem(
|
||||
agent_message,
|
||||
)))
|
||||
.await?;
|
||||
assert!(matches!(submission, StartIfIdleSubmission::Started { .. }));
|
||||
wait_for_event(&initial.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
let chatgpt_base_url = format!("{}/backend-api", server.uri());
|
||||
let resumed = override_builder()
|
||||
let mut builder = override_builder()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
config
|
||||
@@ -690,10 +774,27 @@ async fn reasoning_effort_override_disabled_on_resume_retires_update_at_compacti
|
||||
.expect("disable overrides");
|
||||
config.model_reasoning_effort = Some(ReasoningEffort::High);
|
||||
config.chatgpt_base_url = chatgpt_base_url;
|
||||
})
|
||||
.restart(&server, &initial)
|
||||
.await?;
|
||||
});
|
||||
if let Some(url) = initial.executor_environment().exec_server_url() {
|
||||
builder = builder.with_exec_server_url(url);
|
||||
}
|
||||
let resumed = builder.restart(&server, &initial).await?;
|
||||
resumed.submit_text_turn("after resume").await?;
|
||||
let saved_updates = resumed
|
||||
.codex
|
||||
.load_history(/*include_archived*/ false)
|
||||
.await?
|
||||
.items
|
||||
.into_iter()
|
||||
.filter_map(|item| match item {
|
||||
RolloutItem::ResponseItem(ResponseItemEnvelope {
|
||||
item: item @ ResponseItem::ConfigurationUpdate { .. },
|
||||
..
|
||||
}) => Some(serde_json::to_value(item).expect("serialize saved update")),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(saved_updates, vec![effort_update(ReasoningEffort::Medium)]);
|
||||
resumed.codex.submit(Op::Compact).await?;
|
||||
wait_for_event(&resumed.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
@@ -702,6 +803,11 @@ async fn reasoning_effort_override_disabled_on_resume_retires_update_at_compacti
|
||||
resumed.submit_text_turn("after compaction").await?;
|
||||
|
||||
let requests = mock.requests();
|
||||
assert_eq!(
|
||||
requests[1].inputs_of_type("agent_message"),
|
||||
requests[0].inputs_of_type("agent_message")
|
||||
);
|
||||
assert_eq!(requests[1].inputs_of_type("agent_message").len(), 1);
|
||||
assert_eq!(
|
||||
requests
|
||||
.iter()
|
||||
@@ -715,14 +821,8 @@ async fn reasoning_effort_override_disabled_on_resume_retires_update_at_compacti
|
||||
Value::from("medium"),
|
||||
vec![effort_update(ReasoningEffort::Medium)]
|
||||
),
|
||||
(
|
||||
Value::from("high"),
|
||||
vec![effort_update(ReasoningEffort::Medium)]
|
||||
),
|
||||
(
|
||||
Value::from("high"),
|
||||
vec![effort_update(ReasoningEffort::Medium)]
|
||||
),
|
||||
(Value::from("high"), Vec::new()),
|
||||
(Value::from("high"), Vec::new()),
|
||||
(Value::from("high"), Vec::new()),
|
||||
],
|
||||
);
|
||||
|
||||
@@ -331,6 +331,7 @@ impl MemoryStartupContext {
|
||||
config_snapshot.originator,
|
||||
config.model_verbosity,
|
||||
config.features.enabled(Feature::ContentItemKinds),
|
||||
config.features.enabled(Feature::ReasoningEffortOverride),
|
||||
config.features.enabled(Feature::EnableRequestCompression),
|
||||
config.features.enabled(Feature::RuntimeMetrics),
|
||||
/*beta_features_header*/ None,
|
||||
|
||||
Reference in New Issue
Block a user