fix: gate clear_thinking behind an opt-in flag (Z.AI-only) (#10439)
Signed-off-by: Douwe Osinga <douwe.osinga@gmail.com> Co-authored-by: Jared Manfredi <j.manfredi@draftkings.com> Co-authored-by: Douwe Osinga <douwe.osinga@gmail.com>
This commit is contained in:
@@ -51,6 +51,7 @@ pub struct AnthropicFormatOptions {
|
||||
pub preserve_unsigned_thinking: bool,
|
||||
pub preserve_thinking_context: bool,
|
||||
pub thinking_disabled: bool,
|
||||
pub emit_clear_thinking: bool,
|
||||
pub current_model: Option<String>,
|
||||
pub prompt_cache_disabled: bool,
|
||||
}
|
||||
@@ -66,11 +67,15 @@ impl AnthropicFormatOptions {
|
||||
|| preserve_thinking_context;
|
||||
let thinking_disabled = model_config.reasoning == Some(false)
|
||||
|| model_config.thinking_effort() == Some(ThinkingEffort::Off);
|
||||
let emit_clear_thinking = model_config
|
||||
.request_param::<bool>("emit_clear_thinking")
|
||||
.unwrap_or(self.emit_clear_thinking);
|
||||
|
||||
Self {
|
||||
preserve_unsigned_thinking,
|
||||
preserve_thinking_context,
|
||||
thinking_disabled,
|
||||
emit_clear_thinking,
|
||||
current_model: self
|
||||
.current_model
|
||||
.or_else(|| Some(model_config.model_name.clone())),
|
||||
@@ -751,8 +756,11 @@ fn apply_thinking_config(
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(thinking) = obj.get_mut("thinking").and_then(|t| t.as_object_mut()) {
|
||||
thinking.insert("clear_thinking".to_string(), json!(false));
|
||||
// Z.AI requires this to preserve reasoning; Anthropic rejects it.
|
||||
if options.emit_clear_thinking {
|
||||
if let Some(thinking) = obj.get_mut("thinking").and_then(|t| t.as_object_mut()) {
|
||||
thinking.insert("clear_thinking".to_string(), json!(false));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1667,6 +1675,7 @@ mod tests {
|
||||
AnthropicFormatOptions {
|
||||
preserve_unsigned_thinking: true,
|
||||
preserve_thinking_context: true,
|
||||
emit_clear_thinking: true,
|
||||
..Default::default()
|
||||
},
|
||||
)?;
|
||||
@@ -1700,6 +1709,38 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_enabled_thinking_without_optin_omits_clear_thinking() -> Result<()> {
|
||||
let mut config = cfg("claude-sonnet-4-5-20250929");
|
||||
config.max_tokens = Some(64000);
|
||||
let messages = vec![
|
||||
Message::assistant().with_content(MessageContentBlock::thinking("internal", "")),
|
||||
Message::user().with_text("Continue"),
|
||||
];
|
||||
|
||||
let payload = create_request_with_options_provider(
|
||||
&config,
|
||||
"system",
|
||||
&messages,
|
||||
&[],
|
||||
AnthropicFormatOptions {
|
||||
preserve_unsigned_thinking: true,
|
||||
preserve_thinking_context: true,
|
||||
emit_clear_thinking: false,
|
||||
..Default::default()
|
||||
},
|
||||
)?;
|
||||
|
||||
assert_eq!(payload["thinking"]["type"], "enabled");
|
||||
assert!(
|
||||
payload["thinking"].get("clear_thinking").is_none(),
|
||||
"Anthropic enabled thinking must not carry clear_thinking, got {}",
|
||||
payload["thinking"]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_create_request_model_params_enable_preserved_thinking_context() -> Result<()> {
|
||||
let _guard = env_lock::lock_env([
|
||||
@@ -1710,6 +1751,7 @@ mod tests {
|
||||
|
||||
let mut params = std::collections::HashMap::new();
|
||||
params.insert("preserve_thinking_context".to_string(), json!(true));
|
||||
params.insert("emit_clear_thinking".to_string(), json!(true));
|
||||
|
||||
let mut config = cfg("glm-4.7");
|
||||
config.request_params = Some(params);
|
||||
@@ -1728,6 +1770,39 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_adaptive_model_preserved_thinking_omits_clear_thinking() -> Result<()> {
|
||||
let mut config = cfg_with_effort("claude-opus-4-8", "high");
|
||||
config.max_tokens = Some(64000);
|
||||
let messages = vec![
|
||||
Message::assistant().with_content(MessageContentBlock::thinking("internal", "")),
|
||||
Message::user().with_text("Continue"),
|
||||
];
|
||||
|
||||
let payload = create_request_with_options_provider(
|
||||
&config,
|
||||
"system",
|
||||
&messages,
|
||||
&[],
|
||||
AnthropicFormatOptions {
|
||||
preserve_unsigned_thinking: true,
|
||||
preserve_thinking_context: true,
|
||||
emit_clear_thinking: false,
|
||||
..Default::default()
|
||||
},
|
||||
)?;
|
||||
|
||||
assert_eq!(payload["thinking"]["type"], "adaptive");
|
||||
assert!(
|
||||
payload["thinking"].get("clear_thinking").is_none(),
|
||||
"adaptive thinking must not carry clear_thinking, got {}",
|
||||
payload["thinking"]
|
||||
);
|
||||
assert_eq!(payload["output_config"]["effort"], "high");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_tool_error_handling_maintains_pairing() {
|
||||
use crate::conversation::message::Message;
|
||||
|
||||
@@ -30,6 +30,7 @@ pub fn is_goose_internal_request_param(key: &str) -> bool {
|
||||
key,
|
||||
"thinking_effort"
|
||||
| "disable_prompt_cache"
|
||||
| "emit_clear_thinking"
|
||||
| "preserve_thinking_context"
|
||||
| "preserve_unsigned_thinking"
|
||||
)
|
||||
|
||||
@@ -371,10 +371,14 @@ impl Provider for AnthropicProvider {
|
||||
}
|
||||
}
|
||||
|
||||
fn format_options_for_provider(preserves_thinking: bool) -> AnthropicFormatOptions {
|
||||
fn format_options_for_provider(
|
||||
preserves_thinking: bool,
|
||||
emit_clear_thinking: bool,
|
||||
) -> AnthropicFormatOptions {
|
||||
AnthropicFormatOptions {
|
||||
preserve_unsigned_thinking: preserves_thinking,
|
||||
preserve_thinking_context: preserves_thinking,
|
||||
emit_clear_thinking,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
@@ -426,7 +430,8 @@ pub fn from_declarative_config(
|
||||
_ => AuthMethod::NoAuth,
|
||||
};
|
||||
|
||||
let format_options = format_options_for_provider(config.preserves_thinking);
|
||||
let format_options =
|
||||
format_options_for_provider(config.preserves_thinking, config.emit_clear_thinking);
|
||||
|
||||
let timeout_secs = config
|
||||
.timeout_seconds
|
||||
@@ -476,8 +481,46 @@ pub fn from_declarative_config(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::api_client::AuthMethod;
|
||||
use crate::conversation::message::MessageContent;
|
||||
use serde_json::json;
|
||||
|
||||
struct StubKeyResolver;
|
||||
|
||||
impl crate::declarative::KeyResolver for StubKeyResolver {
|
||||
type Error = std::convert::Infallible;
|
||||
|
||||
fn resolve_key(&self, _key: &str) -> Result<String, Self::Error> {
|
||||
Ok("test-key".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn zai_provider_config_emits_clear_thinking() {
|
||||
let configs = crate::declarative::fixed_provider_configs().unwrap();
|
||||
let zai = configs.iter().find(|c| c.name == "zai").cloned().unwrap();
|
||||
let builder = from_declarative_config(zai, None, StubKeyResolver).unwrap();
|
||||
|
||||
let mut model = ModelConfig::new("glm-4.7");
|
||||
model.max_tokens = Some(64_000);
|
||||
let messages = vec![
|
||||
Message::assistant().with_content(MessageContent::thinking("internal", "")),
|
||||
Message::user().with_text("Continue"),
|
||||
];
|
||||
|
||||
let payload = create_request_for_model(
|
||||
"zai",
|
||||
&model,
|
||||
"glm-4.7",
|
||||
"system",
|
||||
&messages,
|
||||
&[],
|
||||
builder.format_options,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(payload["thinking"]["clear_thinking"], false);
|
||||
}
|
||||
|
||||
fn make_provider_with_custom_models(
|
||||
host: &str,
|
||||
custom_models: Vec<String>,
|
||||
|
||||
@@ -147,6 +147,9 @@ pub struct DeclarativeProviderConfig {
|
||||
pub fast_model: Option<String>,
|
||||
#[serde(default)]
|
||||
pub preserves_thinking: bool,
|
||||
/// Enables Z.AI's `clear_thinking` field, which Anthropic does not support.
|
||||
#[serde(default)]
|
||||
pub emit_clear_thinking: bool,
|
||||
#[serde(default)]
|
||||
pub setup: Option<goose_provider_types::canonical::catalog::ProviderSetupMetadata>,
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
"model_doc_link": "https://docs.z.ai/devpack/tool/goose",
|
||||
"fast_model": "glm-4.5-air",
|
||||
"preserves_thinking": true,
|
||||
"emit_clear_thinking": true,
|
||||
"models": [
|
||||
{"name": "glm-5.2", "context_limit": 1000000},
|
||||
{"name": "glm-5.1", "context_limit": 200000},
|
||||
|
||||
@@ -573,6 +573,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: false,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1352,6 +1352,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: false,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -215,6 +215,7 @@ pub fn create_custom_provider(
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
};
|
||||
|
||||
@@ -296,6 +297,7 @@ pub fn update_custom_provider(params: UpdateCustomProviderParams) -> Result<()>
|
||||
setup_steps: existing_config.setup_steps,
|
||||
fast_model: existing_config.fast_model.clone(),
|
||||
preserves_thinking,
|
||||
emit_clear_thinking: existing_config.emit_clear_thinking,
|
||||
setup: existing_config.setup,
|
||||
};
|
||||
|
||||
@@ -567,6 +569,7 @@ mod tests {
|
||||
setup_steps: Vec::new(),
|
||||
fast_model: None,
|
||||
preserves_thinking: true,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,6 +133,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: false,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -545,6 +545,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: true,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -472,6 +472,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: true,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -400,6 +400,7 @@ mod tests {
|
||||
setup_steps: vec![],
|
||||
fast_model: None,
|
||||
preserves_thinking: false,
|
||||
emit_clear_thinking: false,
|
||||
setup: None,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user