From 5179823d25d7b4e0f45b32ca2ec04ab54975f2b0 Mon Sep 17 00:00:00 2001 From: jh-block Date: Wed, 20 May 2026 12:49:32 +0200 Subject: [PATCH] Omit unset thinking effort Preserve an unset unified thinking effort through provider formatting and the model switcher. Providers now only serialize effort fields when the user or legacy config explicitly sets an effort, allowing upstream API defaults to apply. Signed-off-by: jh-block --- crates/goose/src/config/base.rs | 2 +- crates/goose/src/providers/chatgpt_codex.rs | 48 +++++++++---- crates/goose/src/providers/codex.rs | 7 +- .../goose/src/providers/formats/anthropic.rs | 71 +++++++------------ .../goose/src/providers/formats/databricks.rs | 9 ++- .../models/subcomponents/SwitchModelModal.tsx | 23 +++--- 6 files changed, 81 insertions(+), 79 deletions(-) diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index f26d9bdab0..0f83818737 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -1100,7 +1100,7 @@ config_value!(CURSOR_AGENT_COMMAND, String, "cursor-agent"); config_value!(CODEX_COMMAND, String, "codex"); config_value!(CODEX_ENABLE_SKILLS, String, "true"); config_value!(CODEX_SKIP_GIT_CHECK, String, "false"); -config_value!(CHATGPT_CODEX_REASONING_EFFORT, String, "medium"); +config_value!(CHATGPT_CODEX_REASONING_EFFORT, String); config_value!(GOOSE_SEARCH_PATHS, Vec); config_value!(GOOSE_MODE, GooseMode); diff --git a/crates/goose/src/providers/chatgpt_codex.rs b/crates/goose/src/providers/chatgpt_codex.rs index 1aa29862b0..bd0edb2aa6 100644 --- a/crates/goose/src/providers/chatgpt_codex.rs +++ b/crates/goose/src/providers/chatgpt_codex.rs @@ -209,32 +209,34 @@ fn build_input_items(messages: &[Message]) -> Result> { Ok(items) } -fn get_reasoning_effort(model_name: &str) -> String { - let config = crate::config::Config::global(); - let effort = config - .get_chatgpt_codex_reasoning_effort() - .map(String::from) - .unwrap_or_else(|_| "medium".to_string()); - +fn configured_reasoning_effort(model_name: &str, config: &crate::config::Config) -> Option { + let effort = String::from(config.get_chatgpt_codex_reasoning_effort().ok()?); let valid_levels = reasoning_levels_for_model(model_name); + if valid_levels.contains(&effort.as_str()) { - effort + Some(effort) } else { tracing::warn!( - "Invalid CHATGPT_CODEX_REASONING_EFFORT '{}' for model '{}', using 'medium'", + "Invalid CHATGPT_CODEX_REASONING_EFFORT '{}' for model '{}', omitting reasoning effort", effort, model_name ); - "medium".to_string() + None } } fn reasoning_effort_for_config(model_config: &ModelConfig) -> Option { + reasoning_effort_for_config_with_config(model_config, crate::config::Config::global()) +} + +fn reasoning_effort_for_config_with_config( + model_config: &ModelConfig, + config: &crate::config::Config, +) -> Option { use crate::model::ThinkingEffort; - model_config - .thinking_effort() - .map(|effort| { + match model_config.thinking_effort() { + Some(effort) => { let valid_levels = reasoning_levels_for_model(&model_config.model_name); let preferred_levels: &[&str] = match effort { ThinkingEffort::Off => return None, @@ -248,8 +250,9 @@ fn reasoning_effort_for_config(model_config: &ModelConfig) -> Option { .iter() .find(|level| valid_levels.contains(level)) .map(|level| (*level).to_string()) - }) - .unwrap_or_else(|| Some(get_reasoning_effort(&model_config.model_name))) + } + None => configured_reasoning_effort(&model_config.model_name, config), + } } fn create_codex_request( @@ -1242,6 +1245,21 @@ mod tests { assert!(payload.get("reasoning_effort").is_none()); } + #[test] + fn test_create_codex_request_omits_reasoning_when_effort_unset() { + let _guard = env_lock::lock_env([ + ("GOOSE_THINKING_EFFORT", None::<&str>), + ("CHATGPT_CODEX_REASONING_EFFORT", None::<&str>), + ]); + let config_file = tempfile::NamedTempFile::new().unwrap(); + let secrets_file = tempfile::NamedTempFile::new().unwrap(); + let empty_config = + crate::config::Config::new_with_file_secrets(config_file.path(), secrets_file.path()) + .unwrap(); + + assert!(configured_reasoning_effort("gpt-5.2-codex", &empty_config).is_none()); + } + #[test_case( JwtClaims { chatgpt_account_id: Some("account-1".to_string()), diff --git a/crates/goose/src/providers/codex.rs b/crates/goose/src/providers/codex.rs index 8b420d4146..5811967e7c 100644 --- a/crates/goose/src/providers/codex.rs +++ b/crates/goose/src/providers/codex.rs @@ -79,10 +79,7 @@ impl CodexProvider { effort: Option, ) -> Option { use crate::model::ThinkingEffort; - match effort - .or_else(Self::legacy_reasoning_effort) - .unwrap_or(ThinkingEffort::High) - { + match effort.or_else(Self::legacy_reasoning_effort)? { ThinkingEffort::Off => Some("none".to_string()), ThinkingEffort::Low => Some("low".to_string()), ThinkingEffort::Medium => Some("medium".to_string()), @@ -1259,7 +1256,7 @@ mod tests { ); assert_eq!( CodexProvider::map_thinking_effort("gpt-5.2-codex", None), - Some("high".to_string()) + None ); } diff --git a/crates/goose/src/providers/formats/anthropic.rs b/crates/goose/src/providers/formats/anthropic.rs index af7371ae78..66fedd78db 100644 --- a/crates/goose/src/providers/formats/anthropic.rs +++ b/crates/goose/src/providers/formats/anthropic.rs @@ -503,13 +503,21 @@ pub fn thinking_effort(model_config: &ModelConfig) -> ThinkingEffort { .unwrap_or(ThinkingEffort::High) } -fn adaptive_effort_value(model_config: &ModelConfig) -> &'static str { - match model_config.thinking_effort() { - Some(ThinkingEffort::Low) => "low", - Some(ThinkingEffort::Medium) => "medium", - Some(ThinkingEffort::Max) if is_claude_opus_47(&model_config.model_name) => "xhigh", - Some(ThinkingEffort::Max) => "max", - Some(ThinkingEffort::Off | ThinkingEffort::High) | None => "high", +pub fn adaptive_effort_value(model_config: &ModelConfig) -> Option<&'static str> { + adaptive_effort_value_for_model(&model_config.model_name, model_config.thinking_effort()) +} + +fn adaptive_effort_value_for_model( + model_name: &str, + effort: Option, +) -> Option<&'static str> { + match effort? { + ThinkingEffort::Off => None, + ThinkingEffort::Low => Some("low"), + ThinkingEffort::Medium => Some("medium"), + ThinkingEffort::High => Some("high"), + ThinkingEffort::Max if is_claude_opus_47(model_name) => Some("xhigh"), + ThinkingEffort::Max => Some("max"), } } @@ -562,8 +570,9 @@ fn apply_thinking_config( "thinking".to_string(), json!({"type": "adaptive", "display": "summarized"}), ); - let effort = adaptive_effort_value(model_config); - obj.insert("output_config".to_string(), json!({"effort": effort})); + if let Some(effort) = adaptive_effort_value(model_config) { + obj.insert("output_config".to_string(), json!({"effort": effort})); + } } ThinkingType::Enabled => { let budget_tokens = thinking_budget_tokens(model_config); @@ -587,8 +596,9 @@ fn apply_thinking_config( "thinking".to_string(), json!({"type": "adaptive", "display": "summarized"}), ); - let effort = adaptive_effort_value(model_config); - obj.insert("output_config".to_string(), json!({"effort": effort})); + if let Some(effort) = adaptive_effort_value(model_config) { + obj.insert("output_config".to_string(), json!({"effort": effort})); + } } else { let budget_tokens = thinking_budget_tokens(model_config); obj.insert("max_tokens".to_string(), json!(max_tokens + budget_tokens)); @@ -1245,40 +1255,11 @@ mod tests { } #[test] - fn test_create_request_preserved_opus_47_context_defaults_to_high_effort() -> Result<()> { - let _guard = env_lock::lock_env([ - ("GOOSE_THINKING_EFFORT", None::<&str>), - ("CLAUDE_THINKING_TYPE", None::<&str>), - ("CLAUDE_THINKING_ENABLED", None::<&str>), - ("ANTHROPIC_THINKING_BUDGET", None::<&str>), - ("CLAUDE_THINKING_BUDGET", None::<&str>), - ("ANTHROPIC_PRESERVE_THINKING_CONTEXT", None::<&str>), - ("ANTHROPIC_PRESERVE_UNSIGNED_THINKING", None::<&str>), - ]); - - let mut config = cfg("claude-opus-4-7"); - config.max_tokens = Some(4096); - let messages = vec![ - Message::assistant().with_content(MessageContent::thinking("internal", "")), - Message::user().with_text("Continue"), - ]; - - let payload = create_request_with_options( - &config, - "system", - &messages, - &[], - AnthropicFormatOptions { - preserve_unsigned_thinking: true, - preserve_thinking_context: true, - }, - )?; - - assert_eq!(payload["thinking"]["type"], "adaptive"); - assert_eq!(payload["output_config"]["effort"], "high"); - assert_eq!(payload["max_tokens"], 4096); - - Ok(()) + fn test_adaptive_effort_omits_unset_default() { + assert_eq!( + adaptive_effort_value_for_model("claude-opus-4-7", None), + None + ); } #[test] diff --git a/crates/goose/src/providers/formats/databricks.rs b/crates/goose/src/providers/formats/databricks.rs index 7918c75004..508d6da7ed 100644 --- a/crates/goose/src/providers/formats/databricks.rs +++ b/crates/goose/src/providers/formats/databricks.rs @@ -1,7 +1,7 @@ use crate::conversation::message::{Message, MessageContent}; use crate::model::ModelConfig; use crate::providers::formats::anthropic::{ - thinking_budget_tokens, thinking_effort, thinking_type, ThinkingType, + adaptive_effort_value, thinking_budget_tokens, thinking_type, ThinkingType, }; use crate::providers::utils::{ convert_image, detect_image_path, extract_reasoning_effort, is_openai_responses_model, @@ -237,10 +237,9 @@ fn apply_claude_thinking_config(payload: &mut Value, model_config: &ModelConfig) match thinking_type(model_config) { ThinkingType::Adaptive => { obj.insert("thinking".to_string(), json!({ "type": "adaptive" })); - obj.insert( - "output_config".to_string(), - json!({ "effort": thinking_effort(model_config).to_string() }), - ); + if let Some(effort) = adaptive_effort_value(model_config) { + obj.insert("output_config".to_string(), json!({ "effort": effort })); + } obj.insert( "max_completion_tokens".to_string(), json!(model_config.max_output_tokens()), diff --git a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx index e127fd5e92..5cba3dcc06 100644 --- a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx +++ b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx @@ -413,12 +413,16 @@ export const SwitchModelModal = ({ }; if (showThinkingControl) { - const effort = thinkingEffort ?? modelObj.request_params?.thinking_effort ?? 'off'; - modelObj = { - ...modelObj, - request_params: { ...modelObj.request_params, thinking_effort: effort }, - }; - upsert('GOOSE_THINKING_EFFORT', effort, false).catch(console.warn); + const effort = (thinkingEffort ?? modelObj.request_params?.thinking_effort) as + | ThinkingEffort + | undefined; + if (effort) { + modelObj = { + ...modelObj, + request_params: { ...modelObj.request_params, thinking_effort: effort }, + }; + upsert('GOOSE_THINKING_EFFORT', effort, false).catch(console.warn); + } } const success = await changeModel(sessionId, modelObj); @@ -704,6 +708,9 @@ export const SwitchModelModal = ({ } }; + const selectedThinkingEffort = + thinkingEffort ?? (selectedPredefinedModel?.request_params?.thinking_effort as ThinkingEffort); + const thinkingEffortControl = showThinkingControl && (