mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-28 13:32:23 +00:00
fix(inference): replay deepseek reasoning_content on tool-call turns (Sentry TAURI-RUST-4KB) (#2876)
Co-authored-by: Ghost Scripter <ghostscripter@zerolend.xyz>
This commit is contained in:
co-authored by
Ghost Scripter
parent
ad6d600469
commit
fa50ceb222
@@ -597,7 +597,17 @@ pub(crate) fn parse_structured_tool_calls(tool_calls: &[ToolCall]) -> Vec<Parsed
|
||||
/// Build assistant history entry in JSON format for native tool-call APIs.
|
||||
/// `convert_messages` in the OpenRouter provider parses this JSON to reconstruct
|
||||
/// the proper `NativeMessage` with structured `tool_calls`.
|
||||
pub(crate) fn build_native_assistant_history(text: &str, tool_calls: &[ToolCall]) -> String {
|
||||
///
|
||||
/// `reasoning_content` carries the model's thinking output (when the provider
|
||||
/// surfaced it). It is persisted so the next request can replay it: DeepSeek's
|
||||
/// thinking mode rejects an `assistant` turn that carries `tool_calls` if its
|
||||
/// `reasoning_content` is not passed back (Sentry TAURI-RUST-4KB). Omitted from
|
||||
/// the JSON when empty, so non-reasoning models are unaffected.
|
||||
pub(crate) fn build_native_assistant_history(
|
||||
text: &str,
|
||||
reasoning_content: Option<&str>,
|
||||
tool_calls: &[ToolCall],
|
||||
) -> String {
|
||||
let calls_json: Vec<serde_json::Value> = tool_calls
|
||||
.iter()
|
||||
.map(|tc| {
|
||||
@@ -615,11 +625,16 @@ pub(crate) fn build_native_assistant_history(text: &str, tool_calls: &[ToolCall]
|
||||
serde_json::Value::String(text.trim().to_string())
|
||||
};
|
||||
|
||||
serde_json::json!({
|
||||
let mut entry = serde_json::json!({
|
||||
"content": content,
|
||||
"tool_calls": calls_json,
|
||||
})
|
||||
.to_string()
|
||||
});
|
||||
|
||||
if let Some(reasoning) = reasoning_content.map(str::trim).filter(|r| !r.is_empty()) {
|
||||
entry["reasoning_content"] = serde_json::Value::String(reasoning.to_string());
|
||||
}
|
||||
|
||||
entry.to_string()
|
||||
}
|
||||
|
||||
pub(crate) fn build_assistant_history_with_tool_calls(
|
||||
|
||||
@@ -267,10 +267,25 @@ fn structured_tool_call_and_history_helpers_round_trip_expected_shapes() {
|
||||
assert_eq!(parsed.len(), 1);
|
||||
assert_eq!(parsed[0].arguments, serde_json::json!({ "value": "hello" }));
|
||||
|
||||
let native = build_native_assistant_history("done", &tool_calls);
|
||||
let native = build_native_assistant_history("done", None, &tool_calls);
|
||||
let native_json: serde_json::Value = serde_json::from_str(&native).expect("valid json");
|
||||
assert_eq!(native_json["content"], "done");
|
||||
assert_eq!(native_json["tool_calls"][0]["id"], "call-1");
|
||||
// No reasoning supplied -> field omitted entirely (non-reasoning models
|
||||
// must not gain a spurious `reasoning_content` key).
|
||||
assert!(native_json.get("reasoning_content").is_none());
|
||||
|
||||
// DeepSeek thinking mode: reasoning must round-trip onto the tool-call
|
||||
// turn (Sentry TAURI-RUST-4KB).
|
||||
let native_reasoning =
|
||||
build_native_assistant_history("done", Some(" step-by-step thoughts "), &tool_calls);
|
||||
let reasoning_json: serde_json::Value =
|
||||
serde_json::from_str(&native_reasoning).expect("valid json");
|
||||
assert_eq!(reasoning_json["reasoning_content"], "step-by-step thoughts");
|
||||
// Whitespace-only reasoning is treated as absent.
|
||||
let native_blank = build_native_assistant_history("done", Some(" "), &tool_calls);
|
||||
let blank_json: serde_json::Value = serde_json::from_str(&native_blank).expect("valid json");
|
||||
assert!(blank_json.get("reasoning_content").is_none());
|
||||
|
||||
let xml_history = build_assistant_history_with_tool_calls("", &tool_calls);
|
||||
assert!(xml_history.contains("<tool_call>"));
|
||||
|
||||
@@ -1526,8 +1526,11 @@ async fn run_inner_loop(
|
||||
if force_text_mode {
|
||||
history.push(ChatMessage::assistant(response_text.clone()));
|
||||
} else {
|
||||
let assistant_history_content =
|
||||
super::super::parse::build_native_assistant_history(&response_text, &native_calls);
|
||||
let assistant_history_content = super::super::parse::build_native_assistant_history(
|
||||
&response_text,
|
||||
resp.reasoning_content.as_deref(),
|
||||
&native_calls,
|
||||
);
|
||||
history.push(ChatMessage::assistant(assistant_history_content));
|
||||
}
|
||||
|
||||
|
||||
@@ -583,7 +583,11 @@ pub(crate) async fn run_tool_call_loop(
|
||||
let assistant_history_content = if resp.tool_calls.is_empty() {
|
||||
response_text.clone()
|
||||
} else {
|
||||
build_native_assistant_history(&response_text, &resp.tool_calls)
|
||||
build_native_assistant_history(
|
||||
&response_text,
|
||||
resp.reasoning_content.as_deref(),
|
||||
&resp.tool_calls,
|
||||
)
|
||||
};
|
||||
|
||||
let native_calls = resp.tool_calls;
|
||||
|
||||
@@ -593,6 +593,22 @@ impl OpenAiCompatibleProvider {
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string);
|
||||
|
||||
// Replay the assistant's reasoning so
|
||||
// DeepSeek thinking mode accepts the
|
||||
// tool-call turn on the follow-up request
|
||||
// (Sentry TAURI-RUST-4KB). Prefer the value
|
||||
// embedded in the JSON content (written by
|
||||
// `build_native_assistant_history` in the
|
||||
// tool-loop path); fall back to the value
|
||||
// stored in `extra_metadata` (written by the
|
||||
// main session-turn path).
|
||||
let reasoning_content = value
|
||||
.get("reasoning_content")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.filter(|s| !s.trim().is_empty())
|
||||
.map(ToString::to_string)
|
||||
.or_else(|| reasoning_content.clone());
|
||||
|
||||
return NativeMessage {
|
||||
role: "assistant".to_string(),
|
||||
content,
|
||||
@@ -792,7 +808,12 @@ impl OpenAiCompatibleProvider {
|
||||
// the next turn for thinking models (e.g. DeepSeek-R1, Qwen3) whose APIs
|
||||
// return HTTP 400 ("reasoning_content in thinking mode must be passed back")
|
||||
// when the field is omitted from subsequent assistant messages.
|
||||
let reasoning_content = message.reasoning_content.clone();
|
||||
let reasoning_content = message
|
||||
.reasoning_content
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(str::to_owned);
|
||||
let mut tool_calls = message
|
||||
.tool_calls
|
||||
.unwrap_or_default()
|
||||
@@ -1739,6 +1760,15 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
.ok_or_else(|| anyhow::anyhow!("No response from {}", self.name))?;
|
||||
|
||||
let text = choice.message.effective_content_optional();
|
||||
// See `parse_native_response`: replay reasoning on the follow-up
|
||||
// request so DeepSeek thinking mode accepts the tool-call turn.
|
||||
let reasoning_content = choice
|
||||
.message
|
||||
.reasoning_content
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(ToString::to_string);
|
||||
let tool_calls = choice
|
||||
.message
|
||||
.tool_calls
|
||||
@@ -1760,7 +1790,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
text,
|
||||
tool_calls,
|
||||
usage,
|
||||
reasoning_content: None,
|
||||
reasoning_content,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -658,6 +658,49 @@ fn parse_native_response_preserves_tool_call_id() {
|
||||
assert_eq!(parsed.tool_calls[0].name, "shell");
|
||||
}
|
||||
|
||||
/// DeepSeek thinking mode emits the chain-of-thought in `reasoning_content`
|
||||
/// alongside the tool call. `parse_native_response` must surface it so the
|
||||
/// agent loop can replay it on the follow-up request (Sentry TAURI-RUST-4KB).
|
||||
#[test]
|
||||
fn parse_native_response_captures_reasoning_content() {
|
||||
let message = ResponseMessage {
|
||||
content: None,
|
||||
tool_calls: Some(vec![ToolCall {
|
||||
id: Some("call_r".to_string()),
|
||||
kind: Some("function".to_string()),
|
||||
function: Some(Function {
|
||||
name: Some("shell".to_string()),
|
||||
arguments: Some(serde_json::Value::String("{}".to_string())),
|
||||
}),
|
||||
}]),
|
||||
function_call: None,
|
||||
reasoning_content: Some(" weighing the options ".to_string()),
|
||||
};
|
||||
|
||||
let parsed =
|
||||
OpenAiCompatibleProvider::parse_native_response(wrap_message(message), "deepseek").unwrap();
|
||||
assert_eq!(
|
||||
parsed.reasoning_content.as_deref(),
|
||||
Some("weighing the options")
|
||||
);
|
||||
}
|
||||
|
||||
/// Whitespace-only / empty reasoning is normalised to `None` so it never
|
||||
/// produces a spurious `reasoning_content` key on the wire.
|
||||
#[test]
|
||||
fn parse_native_response_blank_reasoning_is_none() {
|
||||
let message = ResponseMessage {
|
||||
content: Some("hello".to_string()),
|
||||
tool_calls: None,
|
||||
function_call: None,
|
||||
reasoning_content: Some(" ".to_string()),
|
||||
};
|
||||
|
||||
let parsed =
|
||||
OpenAiCompatibleProvider::parse_native_response(wrap_message(message), "deepseek").unwrap();
|
||||
assert!(parsed.reasoning_content.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn convert_messages_for_native_maps_tool_result_payload() {
|
||||
// A `tool` result must be opened by a preceding `assistant(tool_calls)`,
|
||||
@@ -898,6 +941,53 @@ fn tool_invariants_drop_orphan_but_keep_following_cycle() {
|
||||
assert_eq!(converted[1].tool_call_id.as_deref(), Some("call_b"));
|
||||
}
|
||||
|
||||
/// DeepSeek thinking mode (Sentry TAURI-RUST-4KB): an `assistant` turn that
|
||||
/// carries `tool_calls` must replay its `reasoning_content` on the follow-up
|
||||
/// request, otherwise DeepSeek returns
|
||||
/// `400 The reasoning_content in the thinking mode must be passed back to the
|
||||
/// API.` The history JSON written by `build_native_assistant_history` carries
|
||||
/// `reasoning_content`; `convert_messages_for_native` must lift it back onto
|
||||
/// the wire message.
|
||||
#[test]
|
||||
fn convert_preserves_reasoning_content_on_tool_call_turn() {
|
||||
let input = vec![ChatMessage::assistant(
|
||||
r#"{"content":null,"reasoning_content":"let me think about this","tool_calls":[{"id":"call_x","name":"shell","arguments":"{}"}]}"#,
|
||||
)];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(converted.len(), 1);
|
||||
assert_eq!(
|
||||
converted[0].reasoning_content.as_deref(),
|
||||
Some("let me think about this")
|
||||
);
|
||||
|
||||
// The wire payload must actually carry the field for DeepSeek to accept it.
|
||||
let wire = serde_json::to_value(&converted[0]).unwrap();
|
||||
assert_eq!(wire["reasoning_content"], "let me think about this");
|
||||
}
|
||||
|
||||
/// Assistant tool-call turns from non-reasoning models carry no
|
||||
/// `reasoning_content`; it must never appear on the wire for them (most
|
||||
/// OpenAI-compatible providers don't recognise the field).
|
||||
#[test]
|
||||
fn convert_omits_reasoning_content_when_absent() {
|
||||
let input = vec![ChatMessage::assistant(
|
||||
r#"{"content":"sure","tool_calls":[{"id":"call_y","name":"shell","arguments":"{}"}]}"#,
|
||||
)];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(converted.len(), 1);
|
||||
assert!(converted[0].reasoning_content.is_none());
|
||||
|
||||
let wire = serde_json::to_value(&converted[0]).unwrap();
|
||||
assert!(
|
||||
wire.get("reasoning_content").is_none(),
|
||||
"reasoning_content must be omitted from the wire when absent"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn chat_message_identity_metadata_is_not_provider_wire_payload() {
|
||||
let message = ChatMessage {
|
||||
@@ -1561,9 +1651,9 @@ fn enrich_404_message_adds_hint_when_no_fallback() {
|
||||
// ── reasoning_content round-trip tests (issue #2800 / Sentry TAURI-RUST-4WC) ─
|
||||
|
||||
/// `parse_native_response` must capture `reasoning_content` from a non-streaming
|
||||
/// response and surface it on `ChatResponse`.
|
||||
/// `ApiChatResponse` and surface it on `ChatResponse`.
|
||||
#[test]
|
||||
fn parse_native_response_captures_reasoning_content() {
|
||||
fn parse_native_response_captures_reasoning_content_from_api_response() {
|
||||
let api_resp = ApiChatResponse {
|
||||
choices: vec![Choice {
|
||||
message: ResponseMessage {
|
||||
|
||||
Reference in New Issue
Block a user