-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(google): add fallback thought signature sentinel #6138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,9 +63,14 @@ def to_chat_ctx( | |
| "args": json.loads(msg.arguments or "{}"), | ||
| } | ||
| } | ||
| # Inject thought_signature if available (Gemini 3 multi-turn function calling) | ||
| if thought_signatures and (sig := thought_signatures.get(msg.call_id)): | ||
| fc_part["thought_signature"] = sig | ||
| # Gemini 2.5+ requires a thought_signature for every multi-turn | ||
| # function_call part. When a previous tool call came from another | ||
| # provider (for example through FallbackAdapter), we do not have a | ||
| # real signature, so use Google's documented validator-bypass sentinel. | ||
| if thought_signatures is not None: | ||
| fc_part["thought_signature"] = thought_signatures.get( | ||
| msg.call_id, b"skip_thought_signature_validator" | ||
| ) | ||
|
Comment on lines
+70
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Behavioral change: function calls without signatures now get sentinel instead of no signature The old formatter code at Was this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good feedback and the main concern. I think, given older models were retired, this is safe. |
||
| parts.append(fc_part) | ||
| elif msg.type == "function_call_output": | ||
| response = {"output": msg.output} if not msg.is_error else {"error": msg.output} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,10 +419,12 @@ async def _run(self) -> None: | |
| request_id = utils.shortuuid() | ||
|
|
||
| try: | ||
| # Pass thought_signatures for Gemini 2.5+ multi-turn function calling | ||
| thought_sigs = ( | ||
| self._llm._thought_signatures if _requires_thought_signatures(self._model) else None | ||
| ) | ||
| # Pass thought_signatures for Gemini 2.5+ multi-turn function calling. | ||
| # New LLM instances may have no stored signatures yet; use an empty | ||
| # mapping so formatter can add the Gemini 2.5+ fallback sentinel. | ||
| thought_sigs = None | ||
| if _requires_thought_signatures(self._model): | ||
| thought_sigs = getattr(self._llm, "_thought_signatures", None) or {} | ||
| turns_dict, extra_data = self._chat_ctx.to_provider_format( | ||
| format="google", thought_signatures=thought_sigs | ||
| ) | ||
|
|
@@ -561,6 +563,8 @@ def _parse_part(self, id: str, part: types.Part) -> llm.ChatChunk | None: | |
| and hasattr(part, "thought_signature") | ||
| and part.thought_signature | ||
| ): | ||
| if getattr(self._llm, "_thought_signatures", None) is None: | ||
| self._llm._thought_signatures = {} | ||
| self._llm._thought_signatures[tool_call.call_id] = part.thought_signature | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Unbounded growth of The Was this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This finding, while true, is not related to this particular PR |
||
|
|
||
| chat_chunk = llm.ChatChunk( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,52 @@ | ||||||||
| import pytest | ||||||||
|
|
||||||||
| from livekit.agents.llm import ChatContext, FunctionCall, FunctionCallOutput | ||||||||
| from livekit.plugins.google.llm import ( | ||||||||
| _is_gemini_3_flash_model, | ||||||||
| _is_gemini_3_model, | ||||||||
| _requires_thought_signatures, | ||||||||
| ) | ||||||||
|
|
||||||||
|
|
||||||||
| class TestGoogleThoughtSignatureFormatting: | ||||||||
| def test_injects_existing_thought_signature_for_function_call(self): | ||||||||
| ctx = ChatContext.empty() | ||||||||
| ctx.add_message(role="user", content="hello") | ||||||||
| ctx.insert(FunctionCall(call_id="call_1", name="tool", arguments="{}")) | ||||||||
| ctx.insert(FunctionCallOutput(call_id="call_1", name="tool", output="ok", is_error=False)) | ||||||||
|
|
||||||||
| turns, _ = ctx.to_provider_format( | ||||||||
| format="google", thought_signatures={"call_1": b"real_signature"} | ||||||||
| ) | ||||||||
|
|
||||||||
| function_call_part = turns[1]["parts"][0] | ||||||||
| assert function_call_part["thought_signature"] == b"real_signature" | ||||||||
|
|
||||||||
| def test_injects_skip_sentinel_for_missing_thought_signature(self): | ||||||||
| ctx = ChatContext.empty() | ||||||||
| ctx.add_message(role="user", content="hello") | ||||||||
| ctx.insert(FunctionCall(call_id="call_from_openai", name="tool", arguments="{}")) | ||||||||
| ctx.insert( | ||||||||
| FunctionCallOutput(call_id="call_from_openai", name="tool", output="ok", is_error=False) | ||||||||
| ) | ||||||||
|
|
||||||||
| turns, _ = ctx.to_provider_format(format="google", thought_signatures={}) | ||||||||
|
|
||||||||
|
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider removing the parameter, to resemble what happens in
Suggested change
|
||||||||
| function_call_part = turns[1]["parts"][0] | ||||||||
| assert function_call_part["thought_signature"] == b"skip_thought_signature_validator" | ||||||||
|
|
||||||||
| def test_omits_thought_signature_when_not_required(self): | ||||||||
| ctx = ChatContext.empty() | ||||||||
| ctx.add_message(role="user", content="hello") | ||||||||
| ctx.insert(FunctionCall(call_id="call_1", name="tool", arguments="{}")) | ||||||||
| ctx.insert(FunctionCallOutput(call_id="call_1", name="tool", output="ok", is_error=False)) | ||||||||
|
|
||||||||
| turns, _ = ctx.to_provider_format(format="google") | ||||||||
|
|
||||||||
| function_call_part = turns[1]["parts"][0] | ||||||||
| assert "thought_signature" not in function_call_part | ||||||||
|
|
||||||||
|
|
||||||||
| class TestGeminiModelDetection: | ||||||||
| """Tests for Gemini model detection helper functions.""" | ||||||||
|
|
||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 Sentinel value relies on undocumented Google API behavior
The sentinel
b"skip_thought_signature_validator"atlivekit-agents/livekit/agents/llm/_provider_format/google.py:72is described in the comment as "Google's documented validator-bypass sentinel." This is a critical dependency on an external API behavior — if Google removes or changes this sentinel in a future API version, all FallbackAdapter multi-turn function calls to Gemini 2.5+ would start failing. It would be worth confirming this sentinel is part of a stable, documented API contract rather than an internal implementation detail that could change without notice.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's pretty stable IMO (6+ months and counting) and I tested locally.