fix(google): add fallback thought signature sentinel#6138
fix(google): add fallback thought signature sentinel#6138nightcityblade wants to merge 2 commits into
Conversation
|
nightcityblade seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if thought_signatures is not None: | ||
| fc_part["thought_signature"] = thought_signatures.get( | ||
| msg.call_id, b"skip_thought_signature_validator" | ||
| ) |
There was a problem hiding this comment.
🚩 Sentinel value relies on undocumented Google API behavior
The sentinel b"skip_thought_signature_validator" at livekit-agents/livekit/agents/llm/_provider_format/google.py:72 is 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.
It's pretty stable IMO (6+ months and counting) and I tested locally.
| turns, _ = ctx.to_provider_format(format="google", thought_signatures={}) | ||
|
|
There was a problem hiding this comment.
Consider removing the parameter, to resemble what happens in livekit-agents
| turns, _ = ctx.to_provider_format(format="google", thought_signatures={}) | |
| turns, _ = ctx.to_provider_format(format="google") | |
|
Thanks @nightcityblade , take into account the code assumes the |
|
Thanks for the feedback — addressed in Changes:
Checks run:
|
| ): | ||
| if getattr(self._llm, "_thought_signatures", None) is None: | ||
| self._llm._thought_signatures = {} | ||
| self._llm._thought_signatures[tool_call.call_id] = part.thought_signature |
There was a problem hiding this comment.
🚩 Unbounded growth of _thought_signatures dict
The _thought_signatures dict on the LLM instance (livekit-plugins/livekit-plugins-google/livekit/plugins/google/llm.py:232) accumulates an entry for every function call made across the LLM's lifetime and is never pruned. For long-lived agents that make many tool calls, this dict will grow indefinitely. This is a pre-existing issue (not introduced by this PR), but worth noting since this PR increases the reliance on this dict by always consulting it for Gemini 2.5+ models. A possible improvement would be to evict entries once their corresponding chat turns are no longer in the active context.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if thought_signatures is not None: | ||
| fc_part["thought_signature"] = thought_signatures.get( | ||
| msg.call_id, b"skip_thought_signature_validator" | ||
| ) |
There was a problem hiding this comment.
🚩 Behavioral change: function calls without signatures now get sentinel instead of no signature
The old formatter code at livekit-agents/livekit/agents/llm/_provider_format/google.py:66-68 (base) used if thought_signatures and (sig := thought_signatures.get(msg.call_id)): — this meant empty dicts were falsy (no injection), and missing call_ids got no signature. The new code at lines 70-73 uses if thought_signatures is not None: with a sentinel default. This is a significant behavioral change: every function_call part in a Gemini 2.5+ request will now carry either a real signature or b"skip_thought_signature_validator". This is the intended fix, but it changes the wire format for all multi-turn requests, not just the FallbackAdapter case. Confirm that Google's API accepts the sentinel for function calls that previously had valid signatures in prior turns (i.e., ensure mixing real signatures and sentinels in the same request is valid).
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Fixes #6135
Summary
Tests