From c6fd056aa6e2eb7f98cc02b1eb85c3cbced54da3 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 12 Sep 2025 12:06:33 -0700 Subject: [PATCH] feat: reasoning effort as optional (#3527) Allow the reasoning effort to be optional --- codex-rs/common/src/config_summary.rs | 5 ++- codex-rs/common/src/model_presets.rs | 12 +++---- codex-rs/core/src/client.rs | 6 ++-- codex-rs/core/src/client_common.rs | 10 +++--- codex-rs/core/src/codex.rs | 2 +- codex-rs/core/src/config.rs | 31 ++++++++++++------- codex-rs/core/tests/suite/client.rs | 17 +++++++--- codex-rs/core/tests/suite/model_overrides.rs | 4 +-- codex-rs/core/tests/suite/prompt_caching.rs | 4 +-- codex-rs/mcp-server/src/outgoing_message.rs | 4 +-- .../suite/codex_message_processor_flow.rs | 2 +- codex-rs/protocol/src/mcp_protocol.rs | 6 ++-- codex-rs/protocol/src/protocol.rs | 16 +++++++--- codex-rs/tui/src/app.rs | 25 ++++++++------- codex-rs/tui/src/app_event.rs | 2 +- codex-rs/tui/src/chatwidget.rs | 8 +++-- codex-rs/tui/src/chatwidget/tests.rs | 2 +- 17 files changed, 95 insertions(+), 61 deletions(-) diff --git a/codex-rs/common/src/config_summary.rs b/codex-rs/common/src/config_summary.rs index 39d52473..dabc606c 100644 --- a/codex-rs/common/src/config_summary.rs +++ b/codex-rs/common/src/config_summary.rs @@ -17,7 +17,10 @@ pub fn create_config_summary_entries(config: &Config) -> Vec<(&'static str, Stri { entries.push(( "reasoning effort", - config.model_reasoning_effort.to_string(), + config + .model_reasoning_effort + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "none".to_string()), )); entries.push(( "reasoning summaries", diff --git a/codex-rs/common/src/model_presets.rs b/codex-rs/common/src/model_presets.rs index 2b6e5dfb..cf536119 100644 --- a/codex-rs/common/src/model_presets.rs +++ b/codex-rs/common/src/model_presets.rs @@ -12,7 +12,7 @@ pub struct ModelPreset { /// Model slug (e.g., "gpt-5"). pub model: &'static str, /// Reasoning effort to apply for this preset. - pub effort: ReasoningEffort, + pub effort: Option, } /// Built-in list of model presets that pair a model with a reasoning effort. @@ -26,35 +26,35 @@ pub fn builtin_model_presets() -> &'static [ModelPreset] { label: "gpt-5 minimal", description: "— fastest responses with limited reasoning; ideal for coding, instructions, or lightweight tasks", model: "gpt-5", - effort: ReasoningEffort::Minimal, + effort: Some(ReasoningEffort::Minimal), }, ModelPreset { id: "gpt-5-low", label: "gpt-5 low", description: "— balances speed with some reasoning; useful for straightforward queries and short explanations", model: "gpt-5", - effort: ReasoningEffort::Low, + effort: Some(ReasoningEffort::Low), }, ModelPreset { id: "gpt-5-medium", label: "gpt-5 medium", description: "— default setting; provides a solid balance of reasoning depth and latency for general-purpose tasks", model: "gpt-5", - effort: ReasoningEffort::Medium, + effort: Some(ReasoningEffort::Medium), }, ModelPreset { id: "gpt-5-high", label: "gpt-5 high", description: "— maximizes reasoning depth for complex or ambiguous problems", model: "gpt-5", - effort: ReasoningEffort::High, + effort: Some(ReasoningEffort::High), }, ModelPreset { id: "gpt-5-high-new", label: "gpt-5 high new", description: "— our latest release tuned to rely on the model's built-in reasoning defaults", model: "gpt-5-high-new", - effort: ReasoningEffort::Medium, + effort: None, }, ]; PRESETS diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index e6e21df3..8af61d0c 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -72,7 +72,7 @@ pub struct ModelClient { client: reqwest::Client, provider: ModelProviderInfo, conversation_id: ConversationId, - effort: ReasoningEffortConfig, + effort: Option, summary: ReasoningSummaryConfig, } @@ -81,7 +81,7 @@ impl ModelClient { config: Arc, auth_manager: Option>, provider: ModelProviderInfo, - effort: ReasoningEffortConfig, + effort: Option, summary: ReasoningSummaryConfig, conversation_id: ConversationId, ) -> Self { @@ -356,7 +356,7 @@ impl ModelClient { } /// Returns the current reasoning effort setting. - pub fn get_reasoning_effort(&self) -> ReasoningEffortConfig { + pub fn get_reasoning_effort(&self) -> Option { self.effort } diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 2f849239..ffcc63f5 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -136,14 +136,14 @@ pub(crate) struct ResponsesApiRequest<'a> { pub(crate) fn create_reasoning_param_for_request( model_family: &ModelFamily, - effort: ReasoningEffortConfig, + effort: Option, summary: ReasoningSummaryConfig, ) -> Option { - if model_family.supports_reasoning_summaries { - Some(Reasoning { effort, summary }) - } else { - None + if !model_family.supports_reasoning_summaries { + return None; } + + effort.map(|effort| Reasoning { effort, summary }) } pub(crate) fn create_text_param_for_request( diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 79bf72ec..6d6eb913 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -323,7 +323,7 @@ struct ConfigureSession { /// If not specified, server will use its default model. model: String, - model_reasoning_effort: ReasoningEffortConfig, + model_reasoning_effort: Option, model_reasoning_summary: ReasoningSummaryConfig, /// Model instructions that are appended to the base instructions. diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 2cfd15fc..443c92a8 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -140,7 +140,7 @@ pub struct Config { /// Value to use for `reasoning.effort` when making a request using the /// Responses API. - pub model_reasoning_effort: ReasoningEffort, + pub model_reasoning_effort: Option, /// If not "none", the value to use for `reasoning.summary` when making a /// request using the Responses API. @@ -423,14 +423,24 @@ pub async fn persist_model_selection( if let Some(profile_name) = active_profile { let profile_table = ensure_profile_table(&mut doc, profile_name)?; profile_table["model"] = toml_edit::value(model); - if let Some(effort) = effort { - profile_table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); + match effort { + Some(effort) => { + profile_table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); + } + None => { + profile_table.remove("model_reasoning_effort"); + } } } else { let table = doc.as_table_mut(); table["model"] = toml_edit::value(model); - if let Some(effort) = effort { - table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); + match effort { + Some(effort) => { + table["model_reasoning_effort"] = toml_edit::value(effort.to_string()); + } + None => { + table.remove("model_reasoning_effort"); + } } } @@ -913,8 +923,7 @@ impl Config { .unwrap_or(false), model_reasoning_effort: config_profile .model_reasoning_effort - .or(cfg.model_reasoning_effort) - .unwrap_or_default(), + .or(cfg.model_reasoning_effort), model_reasoning_summary: config_profile .model_reasoning_summary .or(cfg.model_reasoning_summary) @@ -1438,7 +1447,7 @@ model_verbosity = "high" codex_linux_sandbox_exe: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, - model_reasoning_effort: ReasoningEffort::High, + model_reasoning_effort: Some(ReasoningEffort::High), model_reasoning_summary: ReasoningSummary::Detailed, model_verbosity: None, chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(), @@ -1494,7 +1503,7 @@ model_verbosity = "high" codex_linux_sandbox_exe: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, - model_reasoning_effort: ReasoningEffort::default(), + model_reasoning_effort: None, model_reasoning_summary: ReasoningSummary::default(), model_verbosity: None, chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(), @@ -1565,7 +1574,7 @@ model_verbosity = "high" codex_linux_sandbox_exe: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, - model_reasoning_effort: ReasoningEffort::default(), + model_reasoning_effort: None, model_reasoning_summary: ReasoningSummary::default(), model_verbosity: None, chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(), @@ -1622,7 +1631,7 @@ model_verbosity = "high" codex_linux_sandbox_exe: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, - model_reasoning_effort: ReasoningEffort::High, + model_reasoning_effort: Some(ReasoningEffort::High), model_reasoning_summary: ReasoningSummary::Detailed, model_verbosity: Some(Verbosity::High), chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 992679df..579f37cd 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -441,6 +441,7 @@ async fn chatgpt_auth_sends_correct_request() { // Init session let codex_home = TempDir::new().unwrap(); let mut config = load_default_config_for_test(&codex_home); + let include_reasoning = config.model_reasoning_effort.is_some(); config.model_provider = model_provider; let conversation_manager = ConversationManager::with_auth(create_dummy_codex_auth()); let NewConversation { @@ -482,10 +483,18 @@ async fn chatgpt_auth_sends_correct_request() { ); assert_eq!(request_chatgpt_account_id.to_str().unwrap(), "account_id"); assert!(request_body["stream"].as_bool().unwrap()); - assert_eq!( - request_body["include"][0].as_str().unwrap(), - "reasoning.encrypted_content" - ); + if include_reasoning { + assert_eq!( + request_body["include"][0].as_str().unwrap(), + "reasoning.encrypted_content" + ); + } else { + assert!( + request_body["include"] + .as_array() + .is_none_or(|items| items.is_empty()) + ); + } } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/core/tests/suite/model_overrides.rs b/codex-rs/core/tests/suite/model_overrides.rs index 8bd3a856..a186c13e 100644 --- a/codex-rs/core/tests/suite/model_overrides.rs +++ b/codex-rs/core/tests/suite/model_overrides.rs @@ -36,7 +36,7 @@ async fn override_turn_context_does_not_persist_when_config_exists() { approval_policy: None, sandbox_policy: None, model: Some("o3".to_string()), - effort: Some(ReasoningEffort::High), + effort: Some(Some(ReasoningEffort::High)), summary: None, }) .await @@ -76,7 +76,7 @@ async fn override_turn_context_does_not_create_config_file() { approval_policy: None, sandbox_policy: None, model: Some("o3".to_string()), - effort: Some(ReasoningEffort::Medium), + effort: Some(Some(ReasoningEffort::Medium)), summary: None, }) .await diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 5ac3da60..c6731fee 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -387,7 +387,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() { exclude_slash_tmp: true, }), model: Some("o3".to_string()), - effort: Some(ReasoningEffort::High), + effort: Some(Some(ReasoningEffort::High)), summary: Some(ReasoningSummary::Detailed), }) .await @@ -519,7 +519,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() { exclude_slash_tmp: true, }, model: "o3".to_string(), - effort: ReasoningEffort::High, + effort: Some(ReasoningEffort::High), summary: ReasoningSummary::Detailed, }) .await diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 04e99516..90e7f9d2 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -280,7 +280,7 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), - reasoning_effort: ReasoningEffort::default(), + reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, history_entry_count: 1000, initial_messages: None, @@ -314,7 +314,7 @@ mod tests { let session_configured_event = SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), - reasoning_effort: ReasoningEffort::default(), + reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, history_entry_count: 1000, initial_messages: None, diff --git a/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs index 49d9ab18..50a480b4 100644 --- a/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/mcp-server/tests/suite/codex_message_processor_flow.rs @@ -320,7 +320,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::new_read_only_policy(), model: "mock-model".to_string(), - effort: ReasoningEffort::Medium, + effort: Some(ReasoningEffort::Medium), summary: ReasoningSummary::Auto, }) .await diff --git a/codex-rs/protocol/src/mcp_protocol.rs b/codex-rs/protocol/src/mcp_protocol.rs index 24357ce1..fe77d20b 100644 --- a/codex-rs/protocol/src/mcp_protocol.rs +++ b/codex-rs/protocol/src/mcp_protocol.rs @@ -223,7 +223,8 @@ pub struct NewConversationResponse { pub conversation_id: ConversationId, pub model: String, /// Note this could be ignored by the model. - pub reasoning_effort: ReasoningEffort, + #[serde(skip_serializing_if = "Option::is_none")] + pub reasoning_effort: Option, pub rollout_path: PathBuf, } @@ -526,7 +527,8 @@ pub struct SendUserTurnParams { pub approval_policy: AskForApproval, pub sandbox_policy: SandboxPolicy, pub model: String, - pub effort: ReasoningEffort, + #[serde(skip_serializing_if = "Option::is_none")] + pub effort: Option, pub summary: ReasoningSummary, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 134059e9..3ae8ffef 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -81,7 +81,8 @@ pub enum Op { model: String, /// Will only be honored if the model is configured to use reasoning. - effort: ReasoningEffortConfig, + #[serde(skip_serializing_if = "Option::is_none")] + effort: Option, /// Will only be honored if the model is configured to use reasoning. summary: ReasoningSummaryConfig, @@ -111,8 +112,11 @@ pub enum Op { model: Option, /// Updated reasoning effort (honored only for reasoning-capable models). + /// + /// Use `Some(Some(_))` to set a specific effort, `Some(None)` to clear + /// the effort, or `None` to leave the existing value unchanged. #[serde(skip_serializing_if = "Option::is_none")] - effort: Option, + effort: Option>, /// Updated reasoning summary preference (honored only for reasoning-capable models). #[serde(skip_serializing_if = "Option::is_none")] @@ -913,7 +917,8 @@ pub struct TurnContextItem { pub approval_policy: AskForApproval, pub sandbox_policy: SandboxPolicy, pub model: String, - pub effort: ReasoningEffortConfig, + #[serde(skip_serializing_if = "Option::is_none")] + pub effort: Option, pub summary: ReasoningSummaryConfig, } @@ -1082,7 +1087,8 @@ pub struct SessionConfiguredEvent { pub model: String, /// The effort the model is putting into reasoning about the user's request. - pub reasoning_effort: ReasoningEffortConfig, + #[serde(skip_serializing_if = "Option::is_none")] + pub reasoning_effort: Option, /// Identifier of the history log file (inode on Unix, 0 otherwise). pub history_log_id: u64, @@ -1172,7 +1178,7 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, model: "codex-mini-latest".to_string(), - reasoning_effort: ReasoningEffortConfig::default(), + reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, history_entry_count: 0, initial_messages: None, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d8819276..2b22396f 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -337,7 +337,7 @@ impl App { } } - fn on_update_reasoning_effort(&mut self, effort: ReasoningEffortConfig) { + fn on_update_reasoning_effort(&mut self, effort: Option) { let changed = self.config.model_reasoning_effort != effort; self.chat_widget.set_reasoning_effort(effort); self.config.model_reasoning_effort = effort; @@ -372,17 +372,18 @@ impl App { let model = self.config.model.clone(); let effort = self.config.model_reasoning_effort; + let effort_label = effort + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "none".to_string()); let codex_home = self.config.codex_home.clone(); match scope { SaveScope::Profile(profile) => { - match persist_model_selection(&codex_home, Some(profile), &model, Some(effort)) - .await - { + match persist_model_selection(&codex_home, Some(profile), &model, effort).await { Ok(()) => { self.model_saved_to_profile = true; self.chat_widget.add_info_message(format!( - "Saved model {model} ({effort}) for profile `{profile}`. Press Ctrl+S again to make this your global default." + "Saved model {model} ({effort_label}) for profile `{profile}`. Press Ctrl+S again to make this your global default." )); } Err(err) => { @@ -397,11 +398,11 @@ impl App { } } SaveScope::Global => { - match persist_model_selection(&codex_home, None, &model, Some(effort)).await { + match persist_model_selection(&codex_home, None, &model, effort).await { Ok(()) => { self.model_saved_to_global = true; self.chat_widget.add_info_message(format!( - "Saved model {model} ({effort}) as your global default." + "Saved model {model} ({effort_label}) as your global default." )); } Err(err) => { @@ -537,19 +538,19 @@ mod tests { let mut app = make_test_app(); app.model_saved_to_profile = true; app.model_saved_to_global = true; - app.config.model_reasoning_effort = ReasoningEffortConfig::Medium; + app.config.model_reasoning_effort = Some(ReasoningEffortConfig::Medium); app.chat_widget - .set_reasoning_effort(ReasoningEffortConfig::Medium); + .set_reasoning_effort(Some(ReasoningEffortConfig::Medium)); - app.on_update_reasoning_effort(ReasoningEffortConfig::High); + app.on_update_reasoning_effort(Some(ReasoningEffortConfig::High)); assert_eq!( app.config.model_reasoning_effort, - ReasoningEffortConfig::High + Some(ReasoningEffortConfig::High) ); assert_eq!( app.chat_widget.config_ref().model_reasoning_effort, - ReasoningEffortConfig::High + Some(ReasoningEffortConfig::High) ); assert!(!app.model_saved_to_profile); assert!(!app.model_saved_to_global); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index cbd6f3d8..ec067792 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -46,7 +46,7 @@ pub(crate) enum AppEvent { CommitTick, /// Update the current reasoning effort in the running app and widget. - UpdateReasoningEffort(ReasoningEffort), + UpdateReasoningEffort(Option), /// Update the current model slug in the running app and widget. UpdateModel(String), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 1f33facd..f47467e4 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1191,9 +1191,13 @@ impl ChatWidget { tracing::info!( "New model: {}, New effort: {}, Current model: {}, Current effort: {}", model_slug.clone(), - effort, + effort + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "none".to_string()), current_model, current_effort + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "none".to_string()) ); })]; items.push(SelectionItem { @@ -1264,7 +1268,7 @@ impl ChatWidget { } /// Set the reasoning effort in the widget's config copy. - pub(crate) fn set_reasoning_effort(&mut self, effort: ReasoningEffortConfig) { + pub(crate) fn set_reasoning_effort(&mut self, effort: Option) { self.config.model_reasoning_effort = effort; } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 298fc849..2cb0685f 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -138,7 +138,7 @@ fn resumed_initial_messages_render_history() { let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, model: "test-model".to_string(), - reasoning_effort: ReasoningEffortConfig::default(), + reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, history_entry_count: 0, initial_messages: Some(vec![