fix(client): replay reasoning_content for DeepSeek models on openai provider (#1739)
should_replay_reasoning_content_for_provider() returned false whenever
provider_accepts_reasoning_content(provider) was false (true for
ApiProvider::Openai) without checking the model. This single gate feeds
both build_for_provider (include_reasoning) and
sanitize_thinking_mode_messages, so a DeepSeek reasoning model on the
generic openai provider (DeepSeek-compatible endpoint) had all
reasoning_content stripped -> the DeepSeek thinking-mode API 400s
('reasoning_content in the thinking mode must be passed back'). This is
the over-aggressive half of ac01b225 (fix #1542).
Gate the early return on the model too:
!provider_accepts_reasoning_content(provider) && !requires_reasoning_content(model).
Known DeepSeek reasoning models replay regardless of provider; genuine
non-DeepSeek models on openai still strip (effort=off still wins). #1542
not regressed (provider_accepts_reasoning_content untouched).
Two pre-existing client.rs tests asserted the buggy case (deepseek-v4-pro
on Openai -> dropped); retargeted to gpt-4o to preserve their #1542
intent without encoding the bug. New positive/negative coverage in
chat.rs.
Refs #1739, #1694, #1542, #1736.
This commit is contained in:
committed by
Hunter Bown
parent
3adc05d627
commit
1a5ee2f67d
+13
-16
@@ -1265,8 +1265,14 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn generic_openai_provider_drops_deepseek_reasoning_content() {
|
||||
// #1542 intent (narrowed by #1739/#1694): a *genuine non-DeepSeek*
|
||||
// model on the generic openai provider must not carry DeepSeek-only
|
||||
// `reasoning_content`. A DeepSeek reasoning model on the openai
|
||||
// provider (DeepSeek-compatible endpoint) is now covered separately
|
||||
// and DOES replay reasoning_content — see
|
||||
// `deepseek_model_on_openai_provider_still_replays_reasoning_content`.
|
||||
let request = MessageRequest {
|
||||
model: "deepseek-v4-pro".to_string(),
|
||||
model: "gpt-4o".to_string(),
|
||||
messages: vec![Message {
|
||||
role: "assistant".to_string(),
|
||||
content: vec![
|
||||
@@ -1291,19 +1297,6 @@ mod tests {
|
||||
top_p: None,
|
||||
};
|
||||
|
||||
let deepseek =
|
||||
build_chat_messages_for_request_and_provider(&request, ApiProvider::Deepseek);
|
||||
let native_assistant = deepseek
|
||||
.iter()
|
||||
.find(|value| value.get("role").and_then(Value::as_str) == Some("assistant"))
|
||||
.expect("assistant message");
|
||||
assert_eq!(
|
||||
native_assistant
|
||||
.get("reasoning_content")
|
||||
.and_then(Value::as_str),
|
||||
Some("plan")
|
||||
);
|
||||
|
||||
let openai = build_chat_messages_for_request_and_provider(&request, ApiProvider::Openai);
|
||||
let generic_assistant = openai
|
||||
.iter()
|
||||
@@ -2707,8 +2700,12 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn sanitize_thinking_mode_skips_generic_openai_provider() {
|
||||
// #1542 intent (narrowed by #1739/#1694): the sanitizer only skips for
|
||||
// a *genuine non-DeepSeek* model on the generic openai provider. A
|
||||
// DeepSeek reasoning model on the openai provider still gets sanitized
|
||||
// (see chat.rs `deepseek_model_on_openai_provider_still_replays_*`).
|
||||
let mut body = json!({
|
||||
"model": "deepseek-v4-pro",
|
||||
"model": "gpt-4o",
|
||||
"messages": [
|
||||
{ "role": "user", "content": "hi" },
|
||||
{
|
||||
@@ -2721,7 +2718,7 @@ mod tests {
|
||||
|
||||
let result = sanitize_thinking_mode_messages(
|
||||
&mut body,
|
||||
"deepseek-v4-pro",
|
||||
"gpt-4o",
|
||||
Some("max"),
|
||||
ApiProvider::Openai,
|
||||
);
|
||||
|
||||
@@ -1628,7 +1628,12 @@ fn should_replay_reasoning_content_for_provider(
|
||||
model: &str,
|
||||
effort: Option<&str>,
|
||||
) -> bool {
|
||||
if !provider_accepts_reasoning_content(provider) {
|
||||
if !provider_accepts_reasoning_content(provider) && !requires_reasoning_content(model) {
|
||||
// Generic non-DeepSeek model on a provider that rejects the field:
|
||||
// keep stripping it (preserves the #1542 fix). But a known DeepSeek
|
||||
// reasoning model pointed at a DeepSeek-compatible endpoint via the
|
||||
// generic `openai` provider still requires reasoning_content replay,
|
||||
// or the thinking-mode API returns 400 (#1739 / #1694).
|
||||
return false;
|
||||
}
|
||||
should_replay_reasoning_content(model, effort)
|
||||
@@ -2828,7 +2833,7 @@ mod alias_thinking_detection_tests {
|
||||
//! https://api-docs.deepseek.com/guides/thinking_mode
|
||||
use super::{
|
||||
provider_accepts_reasoning_content, requires_reasoning_content,
|
||||
should_replay_reasoning_content,
|
||||
should_replay_reasoning_content, should_replay_reasoning_content_for_provider,
|
||||
};
|
||||
use crate::config::ApiProvider;
|
||||
|
||||
@@ -2897,4 +2902,49 @@ mod alias_thinking_detection_tests {
|
||||
assert!(provider_accepts_reasoning_content(ApiProvider::Deepseek));
|
||||
assert!(provider_accepts_reasoning_content(ApiProvider::NvidiaNim));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deepseek_model_on_openai_provider_still_replays_reasoning_content() {
|
||||
// #1739 / #1694: a DeepSeek thinking model pointed at a
|
||||
// DeepSeek-compatible endpoint via the generic `openai` provider must
|
||||
// still replay reasoning_content, even though the provider itself does
|
||||
// not accept the field. Otherwise the thinking-mode API returns 400.
|
||||
assert!(should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"deepseek-v4-flash",
|
||||
None,
|
||||
));
|
||||
assert!(should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"deepseek-v4-pro",
|
||||
None,
|
||||
));
|
||||
assert!(should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"deepseek-reasoner",
|
||||
Some("medium"),
|
||||
));
|
||||
// The documented escape hatch still wins over model detection.
|
||||
assert!(!should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"deepseek-v4-flash",
|
||||
Some("off"),
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generic_model_on_openai_provider_still_strips_reasoning_content() {
|
||||
// #1542 no-regression guard: a genuine non-DeepSeek model on the
|
||||
// openai provider must continue to have reasoning_content stripped.
|
||||
assert!(!should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"gpt-4o",
|
||||
None,
|
||||
));
|
||||
assert!(!should_replay_reasoning_content_for_provider(
|
||||
ApiProvider::Openai,
|
||||
"claude-sonnet-4-6",
|
||||
None,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user