From 9dc21277102cb2bc9dcbc3857563e151140fdc7a Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:46:05 +0530 Subject: [PATCH] fix(inference): persist Gemini thought_signature on tool-call history (TAURI-RUST-4PK/4PJ) (#3770) Co-authored-by: Steven Enamakel --- src/openhuman/agent/harness/parse.rs | 24 ++++++++- src/openhuman/agent/harness/parse_tests.rs | 52 +++++++++++++++++++ .../inference/provider/compatible_tests.rs | 44 ++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/openhuman/agent/harness/parse.rs b/src/openhuman/agent/harness/parse.rs index f8863b011..432e25a11 100644 --- a/src/openhuman/agent/harness/parse.rs +++ b/src/openhuman/agent/harness/parse.rs @@ -738,11 +738,31 @@ pub(crate) fn build_native_assistant_history( let calls_json: Vec = tool_calls .iter() .map(|tc| { - serde_json::json!({ + let mut call = serde_json::json!({ "id": tc.id, "name": tc.name, "arguments": tc.arguments, - }) + }); + // Persist Gemini's per-call `thought_signature` (TAURI-RUST-4PK / + // 4PJ) into the stored assistant turn. PR #3553 threaded the + // signature through the live response→request hop and the + // stored-history *parser* (`parse_provider_tool_call_from_value`), + // but this writer — the single sink the agent loop persists every + // native tool-call turn through (engine/core.rs) — dropped it. On a + // history reload the rebuilt assistant turn therefore lacked + // `extra_content`, so the echoed `functionCall` part went out with + // no `thought_signature` and Gemini 400'd ("Function call is + // missing a thought_signature in functionCall parts"). Write it + // per-part so EVERY call in a parallel/multi-call turn round-trips, + // not just the first; `skip_serializing_if = "Option::is_none"` on + // `extra_content` keeps the stored JSON byte-identical for every + // provider that doesn't emit it. + if let Some(extra) = tc.extra_content.clone() { + if let Some(obj) = call.as_object_mut() { + obj.insert("extra_content".to_string(), extra); + } + } + call }) .collect(); diff --git a/src/openhuman/agent/harness/parse_tests.rs b/src/openhuman/agent/harness/parse_tests.rs index 37dab74b9..6add99cd0 100644 --- a/src/openhuman/agent/harness/parse_tests.rs +++ b/src/openhuman/agent/harness/parse_tests.rs @@ -293,6 +293,58 @@ fn structured_tool_call_and_history_helpers_round_trip_expected_shapes() { assert!(xml_history.contains("\"name\":\"echo\"")); } +/// TAURI-RUST-4PK / 4PJ: the persisted assistant-history JSON must carry each +/// tool call's `extra_content` (Gemini's `thought_signature`). This is the +/// writer that the agent loop runs *every* native tool-call turn through, so a +/// dropped signature here is exactly what re-surfaced the 400 on a history +/// reload after PR #3553 fixed only the response→request hop. Every call in a +/// parallel/multi-call turn must keep its own signature; calls without one omit +/// the field so non-Gemini providers stay byte-identical. +#[test] +fn build_native_assistant_history_persists_per_call_extra_content() { + let tool_calls = vec![ + ToolCall { + id: "call-a".into(), + name: "shell".into(), + arguments: "{}".into(), + extra_content: Some(serde_json::json!({"google":{"thought_signature":"SIG_A"}})), + }, + ToolCall { + id: "call-b".into(), + name: "read".into(), + arguments: "{}".into(), + extra_content: Some(serde_json::json!({"google":{"thought_signature":"SIG_B"}})), + }, + // A call that never had a signature must NOT gain an empty key. + ToolCall { + id: "call-c".into(), + name: "noop".into(), + arguments: "{}".into(), + extra_content: None, + }, + ]; + + let native = build_native_assistant_history("on it", None, &tool_calls); + let json: serde_json::Value = serde_json::from_str(&native).expect("valid json"); + + assert_eq!( + json.pointer("/tool_calls/0/extra_content/google/thought_signature") + .and_then(|v| v.as_str()), + Some("SIG_A"), + "first parallel call's signature must be persisted" + ); + assert_eq!( + json.pointer("/tool_calls/1/extra_content/google/thought_signature") + .and_then(|v| v.as_str()), + Some("SIG_B"), + "second parallel call's signature must be persisted (not just the first)" + ); + assert!( + json.pointer("/tool_calls/2/extra_content").is_none(), + "a call without extra_content must omit the field, keeping non-Gemini history byte-identical" + ); +} + #[test] fn tools_to_openai_format_uses_tool_metadata() { let tools: Vec> = vec![Box::new(StubTool("echo")), Box::new(StubTool("shell"))]; diff --git a/src/openhuman/inference/provider/compatible_tests.rs b/src/openhuman/inference/provider/compatible_tests.rs index b83fff67f..7bebc9523 100644 --- a/src/openhuman/inference/provider/compatible_tests.rs +++ b/src/openhuman/inference/provider/compatible_tests.rs @@ -1544,6 +1544,50 @@ fn convert_messages_for_native_tool_call_without_extra_content_stays_none() { .is_none()); } +/// INVARIANT (TAURI-RUST-4PK / 4PJ): a PARALLEL multi-`functionCall` assistant +/// turn reloaded from history must echo a non-empty `thought_signature` on +/// EVERY part of the rebuilt outbound payload — not just the first. The stored +/// JSON here is the exact shape `build_native_assistant_history` now emits (per +/// the writer-side test in `agent::harness::parse_tests`). Before the fix the +/// writer dropped `extra_content`, so a reloaded multi-call turn went out with +/// missing signatures and Gemini 400'd ("Function call is missing a +/// thought_signature in functionCall parts"). Covers both the non-stream and +/// streaming paths since both persist through the single native history writer. +#[test] +fn convert_messages_for_native_echoes_signature_on_every_parallel_call() { + let stored = r#"{"content":"on it","tool_calls":[ + {"id":"call_a","name":"shell","arguments":"{}","extra_content":{"google":{"thought_signature":"SIG_A"}}}, + {"id":"call_b","name":"read","arguments":"{}","extra_content":{"google":{"thought_signature":"SIG_B"}}} + ]}"#; + let input = vec![ + ChatMessage::assistant(stored), + ChatMessage::tool(r#"{"tool_call_id":"call_a","content":"done"}"#), + ChatMessage::tool(r#"{"tool_call_id":"call_b","content":"done"}"#), + ]; + let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input); + let tool_calls = converted[0] + .tool_calls + .as_ref() + .expect("assistant tool_calls survive the reload"); + assert_eq!(tool_calls.len(), 2, "both parallel calls survive"); + + let wire = serde_json::to_value(tool_calls).unwrap(); + for (idx, expected) in ["SIG_A", "SIG_B"].iter().enumerate() { + let sig = wire + .pointer(&format!("/{idx}/extra_content/google/thought_signature")) + .and_then(|v| v.as_str()); + assert_eq!( + sig, + Some(*expected), + "functionCall part {idx} must echo its own thought_signature on the wire" + ); + assert!( + sig.is_some_and(|s| !s.is_empty()), + "functionCall part {idx} thought_signature must be non-empty" + ); + } +} + /// Streaming: Gemini sends the thought_signature in the tool-call delta's /// `extra_content` on the first chunk. The accumulator must preserve it onto the /// aggregated tool call so it reaches history (TAURI-RUST-4PK).