mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Respect model support for reasoning summaries (#32290)
## What changed - Add `supports_reasoning_summary_parameter` to model metadata, defaulting to `true` for backward compatibility. - Omit `reasoning.summary` and its summary-delivery stream option when the selected model does not support the parameter. - Apply the capability of the final selected model when a spawned agent uses a different model. ## Testing - Cover unsupported models in regular requests and spawned-agent model overrides. GitOrigin-RevId: 72b783799fc0685cef1501ef2dbf62d1308ceead
This commit is contained in:
@@ -35,6 +35,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -81,6 +81,7 @@ async fn models_client_hits_models_endpoint() {
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
+11
-11
@@ -809,11 +809,9 @@ impl ModelClient {
|
||||
effort: effort
|
||||
.or_else(|| model_info.default_reasoning_level.clone())
|
||||
.map(reasoning_effort_for_request),
|
||||
summary: if summary == ReasoningSummaryConfig::None {
|
||||
None
|
||||
} else {
|
||||
Some(summary)
|
||||
},
|
||||
summary: (model_info.supports_reasoning_summary_parameter
|
||||
&& summary != ReasoningSummaryConfig::None)
|
||||
.then_some(summary),
|
||||
// When Responses Lite is disabled, omit context so Responses uses the default,
|
||||
// which is currently `current_turn`.
|
||||
context: model_info
|
||||
@@ -863,11 +861,13 @@ impl ModelClient {
|
||||
} else {
|
||||
(prompt.base_instructions.text.clone(), Some(tools))
|
||||
};
|
||||
let stream_options = (self.state.concurrent_reasoning_summaries_enabled && is_openai)
|
||||
.then_some(StreamOptions {
|
||||
reasoning_summary_delivery: codex_api::ReasoningSummaryDelivery::SequentialCutoff,
|
||||
});
|
||||
let reasoning = Some(Self::build_reasoning(model_info, effort, summary));
|
||||
let reasoning = Self::build_reasoning(model_info, effort, summary);
|
||||
let stream_options = (self.state.concurrent_reasoning_summaries_enabled
|
||||
&& is_openai
|
||||
&& reasoning.summary.is_some())
|
||||
.then_some(StreamOptions {
|
||||
reasoning_summary_delivery: codex_api::ReasoningSummaryDelivery::SequentialCutoff,
|
||||
});
|
||||
let include = vec!["reasoning.encrypted_content".to_string()];
|
||||
let verbosity = if model_info.support_verbosity {
|
||||
self.state.model_verbosity.or(model_info.default_verbosity)
|
||||
@@ -894,7 +894,7 @@ impl ModelClient {
|
||||
tools,
|
||||
tool_choice: "auto".to_string(),
|
||||
parallel_tool_calls: prompt.parallel_tool_calls && !model_info.use_responses_lite,
|
||||
reasoning,
|
||||
reasoning: Some(reasoning),
|
||||
store: provider.is_azure_responses_endpoint(),
|
||||
stream: true,
|
||||
stream_options,
|
||||
|
||||
@@ -245,6 +245,7 @@ fn remote_model_with_auto_review_override(slug: &str, review_model: &str) -> Mod
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -2431,6 +2431,64 @@ async fn configured_reasoning_summary_is_sent() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn model_without_summary_parameter_support_omits_configured_summary() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let resp_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
|
||||
)
|
||||
.await;
|
||||
let mut model_catalog = bundled_models_response().expect("bundled models.json should parse");
|
||||
let model = model_catalog
|
||||
.models
|
||||
.iter_mut()
|
||||
.find(|model| model.slug == "gpt-5.4")
|
||||
.expect("gpt-5.4 exists in bundled models.json");
|
||||
model.supports_reasoning_summary_parameter = false;
|
||||
|
||||
let TestCodex { codex, .. } = test_codex()
|
||||
.with_model("gpt-5.4")
|
||||
.with_config(move |config| {
|
||||
config.model_catalog = Some(model_catalog);
|
||||
config.model_reasoning_effort = Some(ReasoningEffort::High);
|
||||
config.model_reasoning_summary = Some(ReasoningSummary::Detailed);
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ConcurrentReasoningSummaries)
|
||||
.expect("test config should allow feature update");
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: Default::default(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request_body = resp_mock.single_request().body_json();
|
||||
pretty_assertions::assert_eq!(request_body["reasoning"], json!({"effort": "high"}));
|
||||
pretty_assertions::assert_eq!(
|
||||
request_body["include"],
|
||||
json!(["reasoning.encrypted_content"])
|
||||
);
|
||||
pretty_assertions::assert_eq!(request_body.get("stream_options"), None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn sequential_cutoff_is_omitted_for_non_openai_provider() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -98,6 +98,7 @@ fn test_model_info(
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
@@ -925,6 +926,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result<
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -368,6 +368,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -642,6 +642,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
approvals: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
@@ -758,6 +759,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
approvals: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -490,6 +490,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
@@ -743,6 +744,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
base_instructions: remote_base.to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
@@ -1269,6 +1271,7 @@ fn test_remote_model_with_policy(
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -1653,6 +1653,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -72,6 +72,7 @@ fn test_model_info(
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -3,7 +3,9 @@ use codex_core::StartThreadOptions;
|
||||
use codex_core::ThreadConfigSnapshot;
|
||||
use codex_core::config::AgentRoleConfig;
|
||||
use codex_features::Feature;
|
||||
use codex_models_manager::bundled_models_response;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -979,6 +981,80 @@ async fn spawn_agent_requested_model_and_reasoning_override_inherited_settings_w
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(true, false; "unsupported child")]
|
||||
#[test_case(false, true; "supported child")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawned_agent_uses_summary_support_for_final_model(
|
||||
parent_supports_summary: bool,
|
||||
child_supports_summary: bool,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut model_catalog = bundled_models_response().expect("bundled models.json should parse");
|
||||
for (slug, supports_summary) in [
|
||||
(INHERITED_MODEL, parent_supports_summary),
|
||||
(REQUESTED_MODEL, child_supports_summary),
|
||||
] {
|
||||
let model = model_catalog
|
||||
.models
|
||||
.iter_mut()
|
||||
.find(|model| model.slug == slug)
|
||||
.unwrap_or_else(|| panic!("{slug} should exist in bundled models.json"));
|
||||
model.supports_reasoning_summary_parameter = supports_summary;
|
||||
}
|
||||
|
||||
let (_test, _spawned_id, child_request_log) = setup_turn_one_with_custom_spawned_child(
|
||||
&server,
|
||||
json!({
|
||||
"message": CHILD_PROMPT,
|
||||
"model": REQUESTED_MODEL,
|
||||
}),
|
||||
/*child_response_delay*/ Some(Duration::from_secs(1)),
|
||||
/*wait_for_parent_notification*/ false,
|
||||
move |builder| {
|
||||
builder.with_config(move |config| {
|
||||
config.model_catalog = Some(model_catalog);
|
||||
config.model_reasoning_summary = Some(ReasoningSummary::Detailed);
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ConcurrentReasoningSummaries)
|
||||
.expect("test config should allow feature update");
|
||||
})
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
let child_body = loop {
|
||||
if let Some(body) = child_request_log
|
||||
.requests()
|
||||
.iter()
|
||||
.map(ResponsesRequest::body_json)
|
||||
.find(|body| body["model"] == REQUESTED_MODEL)
|
||||
{
|
||||
break body;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
anyhow::bail!("timed out waiting for the child request");
|
||||
}
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
};
|
||||
assert_eq!(child_body["model"], json!(REQUESTED_MODEL));
|
||||
let expected_reasoning = if child_supports_summary {
|
||||
json!({"effort": "medium", "summary": "detailed"})
|
||||
} else {
|
||||
json!({"effort": "medium"})
|
||||
};
|
||||
assert_eq!(child_body["reasoning"], expected_reasoning);
|
||||
assert_eq!(
|
||||
child_body.get("stream_options").is_some(),
|
||||
child_supports_summary
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn spawned_multi_agent_v2_child_inherits_parent_developer_context() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -1375,6 +1375,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -139,6 +139,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo {
|
||||
base_instructions: BASE_INSTRUCTIONS.to_string(),
|
||||
model_messages: local_personality_messages_for_slug(slug),
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
@@ -349,6 +349,15 @@ const fn default_effective_context_window_percent() -> i64 {
|
||||
95
|
||||
}
|
||||
|
||||
const fn default_true() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
#[allow(clippy::trivially_copy_pass_by_ref)]
|
||||
const fn is_true(value: &bool) -> bool {
|
||||
*value
|
||||
}
|
||||
|
||||
/// Model metadata returned by the Codex backend `/models` endpoint.
|
||||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)]
|
||||
pub struct ModelInfo {
|
||||
@@ -375,6 +384,9 @@ pub struct ModelInfo {
|
||||
pub model_messages: Option<ModelMessages>,
|
||||
#[serde(default)]
|
||||
pub include_skills_usage_instructions: bool,
|
||||
/// Whether the model accepts the Responses API `reasoning.summary` parameter.
|
||||
#[serde(default = "default_true", skip_serializing_if = "is_true")]
|
||||
pub supports_reasoning_summary_parameter: bool,
|
||||
#[serde(default)]
|
||||
pub default_reasoning_summary: ReasoningSummary,
|
||||
pub support_verbosity: bool,
|
||||
@@ -680,6 +692,7 @@ mod tests {
|
||||
base_instructions: "base".to_string(),
|
||||
model_messages: spec,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: ReasoningSummary::Auto,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
@@ -1002,6 +1015,7 @@ mod tests {
|
||||
|
||||
assert_eq!(model.availability_nux, None);
|
||||
assert!(!model.include_skills_usage_instructions);
|
||||
assert!(model.supports_reasoning_summary_parameter);
|
||||
assert!(!model.supports_image_detail_original);
|
||||
assert_eq!(model.web_search_tool_type, WebSearchToolType::Text);
|
||||
assert!(!model.supports_search_tool);
|
||||
|
||||
@@ -28,6 +28,7 @@ fn model_with_shell_type(shell_type: ConfigShellToolType) -> ModelInfo {
|
||||
base_instructions: String::new(),
|
||||
model_messages: None,
|
||||
include_skills_usage_instructions: false,
|
||||
supports_reasoning_summary_parameter: true,
|
||||
default_reasoning_summary: Default::default(),
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
|
||||
Reference in New Issue
Block a user