[gemini] Fix parallel function calls in HLI tool loop - #21335
Conversation
23b1d5c to
e5d0bd9
Compare
There was a problem hiding this comment.
Pull request overview
Fixes Gemini parallel tool-call replay by preserving calls and responses as complete model/user turns.
Changes:
- Batches parallel tool calls and results in conversation history.
- Reconstructs matching function calls/responses with per-call IDs.
- Adds batch serialization and validation tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
GeminiHLIService.java |
Batches parallel calls and results. |
GeminiApiClient.java |
Rebuilds batched Gemini turns. |
GeminiLLMToolCall.java |
Adds batch JSON helpers. |
GeminiFunctionResponse.java |
Adds response call IDs. |
GeminiLLMToolCallTest.java |
Tests JSON formats and validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wborn
left a comment
wborn
left a comment
There was a problem hiding this comment.
AI review performed before manual maintainer review.
The parallel function-call handling looks correct. The calls are now kept together as one model turn, the corresponding responses are grouped into the following user turn, and the per-call IDs and thought signature are preserved as required by the Gemini API.
AI found no correctness issues in the implementation. There is one non-blocking test-coverage suggestion inline: it would be useful to cover the reconstructed Gemini request shape in addition to the JSON helper serialization tests.
A human maintainer review is still needed.
|
@wborn Addressed review |
c3863e0 to
4e8ee71
Compare
wborn
left a comment
wborn
left a comment
There was a problem hiding this comment.
The requested regression coverage has been added and verifies the actual Gemini request reconstruction for parallel function calls, including the grouped model/user turns, thought signature, and function call IDs.
AI found no further issues.
A human maintainer review is still needed.
lsiepel
left a comment
lsiepel
left a comment
There was a problem hiding this comment.
Thanks, LGTM
Awaiting confirmation by @florian-h05 as codeowner
|
I’ll have a look in the next days, currently on vacation but will be back home tomorrow evening. |
florian-h05
left a comment
florian-h05
left a comment
There was a problem hiding this comment.
Overall LGTM, however there is a serious violation of the HumanLanguageInterpreter API contract:
Gemini can return several functionCall parts in one model turn, with the thought signature only on the first part. The HLI stored each call as its own TOOL_CALL/TOOL_RETURN pair and the API client replayed them as separate single-call model turns, which Gemini 3.x rejects with 400 INVALID_ARGUMENT (missing thought_signature). Keep storing each call as its own TOOL_CALL/TOOL_RETURN message pair - preserving the message format defined by the HumanLanguageInterpreter API contract (consumed e.g. by Main UI) and the Conversation rule that a tool result must directly follow its tool call - but mark calls 2..N of a parallel batch with a binding-private "parallel":true field in the stored GeminiLLMToolCall JSON, in the same way as the existing id/thoughtSignature extension fields. The field is omitted when null, so single calls keep the exact previous message format and legacy conversations still replay. The API client uses the marker to rebuild a batch as one model turn with all functionCall parts and one user turn with all functionResponse parts, now echoing the per-call id so equal-named parallel calls pair unambiguously. Includes unit tests for the (de-)serialization of the parallel marker. Signed-off-by: Christian Heldt <snaut@tutanota.com>
The parallel marker (de-)serialization is covered by unit tests, but the request shape that actually fixes the parallel call rejection was not. Add a test that drives sendPrompt() with an interleaved two-call history whose second call is marked parallel, captures the serialized request body and asserts that it contains one model content with both functionCall parts followed by one user content with both matching functionResponse parts, preserving the first call's thoughtSignature and both call ids. A second test covers the legacy single-call message format to make sure conversations stored by earlier versions still replay and that absent call ids are omitted from the request. A third test ensures sequential (unmarked) tool calls are not merged and keep their own model turns. Signed-off-by: Christian Heldt <snaut@tutanota.com>
4e8ee71 to
131c6a8
Compare
|
@cheldt Please never force push during review, this resets review progress. In this case the diff luckily isn't that large, just don't do it in the future. Thanks! |
* [gemini] Fix parallel function calls in HLI tool loop Signed-off-by: Christian Heldt <snaut@tutanota.com> Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Fix parallel function calls in HLI tool loop
Gemini can return several functionCall parts in one model turn, with the thought signature only on the first part. The HLI stored each call as its own TOOL_CALL/TOOL_RETURN pair and the API client replayed them as separate single-call model turns, which Gemini 3.x rejects with 400 INVALID_ARGUMENT (missing thought_signature).
Batch parallel calls into a single TOOL_CALL message (JSON array) and their results into a single TOOL_RETURN (JSON array of strings), which also satisfies the conversation rule that a TOOL_RETURN must directly follow its TOOL_CALL. The API client rebuilds them as one model turn with all functionCall parts and one user turn with all functionResponse parts, now echoing the per-call id so equal-named parallel calls pair unambiguously.
Includes unit tests for the JSON batch (de-)serialization helpers.
Fixes #21333