handle gemini thought signatures#3
Conversation
|
@codity review |
1 similar comment
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaTool Call Handling: Extracts and stores Gemini thought signatures from incoming tool calls in a Response Generation: Re-injects stored signatures into outgoing tool calls via Files Changed
Review Focus Areas
ArchitectureDesign Decisions: Uses Risks: This is a provider-specific workaround (Gemini via OpenAI-compatible endpoint) embedded in the generic OpenAI partner package. If Google changes their API shape, this code needs updating. The risk is intentional and acceptable given the current lack of a provider-agnostic way to handle tool call metadata persistence. Merge StatusMERGEABLE — PR Score 73/100, above threshold (50). All gates passed. |
Greptile SummaryThis PR adds support for Google's Gemini thought signatures that are returned on tool calls via the OpenAI-compatible endpoint (
Confidence Score: 4/5The change is narrowly scoped to Gemini thought-signature handling and does not affect standard OpenAI or Azure paths; it is safe to merge for general use. The extraction and re-injection logic is correct for the happy path covered by the tests. The only issue found is a truthiness guard ( libs/partners/openai/langchain_openai/chat_models/base.py — the two signature-collection loops in Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant LangChain
participant GeminiAPI
User->>LangChain: invoke / stream (Turn 1)
LangChain->>GeminiAPI: POST /chat/completions
GeminiAPI-->>LangChain: "tool_calls[{id, function, extra_content.google.thought_signature}]"
LangChain->>LangChain: extract thought_signatures → additional_kwargs[__gemini_function_call_thought_signatures__]
LangChain-->>User: AIMessage (tool_calls + additional_kwargs with signatures)
User->>LangChain: invoke / stream (Turn 2 with history + ToolMessage)
LangChain->>LangChain: re-inject extra_content.google.thought_signature onto each tool call
LangChain->>GeminiAPI: POST /chat/completions (tool_calls with thought_signature echoed back)
GeminiAPI-->>LangChain: assistant reply
LangChain-->>User: final AIMessage
Reviews (1): Last reviewed commit: "handle gemini thought signatures" | Re-trigger Greptile |
| if (signature := _extract_gemini_thought_signature(raw_tool_call)) and ( | ||
| tool_call_id := raw_tool_call.get("id") | ||
| ): | ||
| thought_signatures[tool_call_id] = signature | ||
| if thought_signatures: |
There was a problem hiding this comment.
The walrus-operator condition uses truthiness to guard the signature, so an empty-string
thought_signature (falsy) would be silently dropped instead of being stored and echoed back. The same pattern appears a second time in _convert_delta_to_message_chunk. Prefer an explicit is not None check to match the intent of the extractor, which already returns None for absent/wrong-type values.
| if (signature := _extract_gemini_thought_signature(raw_tool_call)) and ( | |
| tool_call_id := raw_tool_call.get("id") | |
| ): | |
| thought_signatures[tool_call_id] = signature | |
| if thought_signatures: | |
| if (signature := _extract_gemini_thought_signature(raw_tool_call)) is not None and ( | |
| tool_call_id := raw_tool_call.get("id") | |
| ): | |
| thought_signatures[tool_call_id] = signature | |
| if thought_signatures: |
| for rtc in raw_tool_calls: | ||
| if (signature := _extract_gemini_thought_signature(rtc)) and ( | ||
| tool_call_id := rtc.get("id") | ||
| ): | ||
| thought_signatures[tool_call_id] = signature | ||
| if thought_signatures: |
There was a problem hiding this comment.
Same empty-string silent-drop issue in the streaming (
_convert_delta_to_message_chunk) path as in the non-streaming path above — is not None is the correct guard here.
| for rtc in raw_tool_calls: | |
| if (signature := _extract_gemini_thought_signature(rtc)) and ( | |
| tool_call_id := rtc.get("id") | |
| ): | |
| thought_signatures[tool_call_id] = signature | |
| if thought_signatures: | |
| for rtc in raw_tool_calls: | |
| if (signature := _extract_gemini_thought_signature(rtc)) is not None and ( | |
| tool_call_id := rtc.get("id") | |
| ): | |
| thought_signatures[tool_call_id] = signature | |
| if thought_signatures: |
Security Scan Summary
No critical security issues detected Scan completed in 26.9sSecurity scan powered by Codity.ai |
License Compliance Scan
All licenses are low-risk and compliant Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/langchain · PR #3Scanned: 2026-05-18 12:40 UTC | Score: 55/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
libs/partners/openai/langchain_openai/chat_models/base.py |
0 | 0 | 2 | 8 | 10 |
libs/partners/openai/tests/unit_tests/chat_models/test_base.py |
0 | 0 | 1 | 13 | 14 |
Recommendations
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaOpenAI Chat Models: Added thought signature extraction and re-attachment in message conversion methods for both streaming and non-streaming scenarios. Testing: Added parametrized roundtrip test covering streaming and non-streaming tool call scenarios. Files Changed
Review Focus Areas
ArchitectureDesign Decisions: Uses a dedicated constant and helper function to isolate Gemini-specific logic, keeping changes localized to conversion layer rather than message schema. Storage in Risks: Thought signatures are provider-specific metadata that may not be portable to other model providers. This is intentional and acceptable for Gemini compatibility, but creates a subtle dependency on Google's internal format that could change. Merge StatusMERGEABLE — PR Score 73/100, above threshold (50). All gates passed. |
Security Scan Summary
No critical security issues detected Scan completed in 26.5sSecurity scan powered by Codity.ai |
License Compliance Scan
All licenses are low-risk and compliant Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/langchain · PR #3Scanned: 2026-05-18 16:43 UTC | Score: 55/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-003]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
libs/partners/openai/langchain_openai/chat_models/base.py |
0 | 0 | 2 | 8 | 10 |
libs/partners/openai/tests/unit_tests/chat_models/test_base.py |
0 | 0 | 1 | 13 | 14 |
Recommendations
- Run automated tests after applying fixes to verify no regressions.
Fixes #
Read the full contributing guidelines: https://docs.langchain.com/oss/python/contributing/overview
If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!
Thank you for contributing to LangChain! Follow these steps to have your pull request considered as ready for review.
Fixes #xxline at the top is required for external contributions — update the issue number and keep the keyword. This links your PR to the approved issue and auto-closes it on merge.make format,make lintandmake testfrom the root of the package(s) you've modified.Additional guidelines:
uv.lockfiles or add dependencies topyproject.tomlfiles (even optional ones) unless you have explicit permission to do so by a maintainer.Social handles (optional)
Twitter: @
LinkedIn: https://linkedin.com/in/