fix(gemini): keep tool_call JSON valid across streaming SSE replays - #25046
Open
Alex-Electron wants to merge 1 commit into
Open
fix(gemini): keep tool_call JSON valid across streaming SSE replays#25046Alex-Electron wants to merge 1 commit into
Alex-Electron wants to merge 1 commit into
Conversation
The native Gemini adapter emitted `json.dumps(args)` on every streaming SSE
event that carried a functionCall. Gemini re-sends the same call across
repeated events (and occasionally mutates its args mid-stream), so the
consumer ended up concatenating fragments like:
{"q": "A"}{"q": "AB"}
This is invalid JSON and breaks downstream tool_call parsing, silently
discarding the call. The previous prefix-grow guard didn't help: JSON's
closing quote/brace means a longer args dict is almost never a textual
prefix of a shorter one (e.g. `{"q":"abcdef"}` does NOT start with
`{"q":"abc"}`), so the fallback path emitted the full new args.
Switch to a flush-on-finish strategy:
- First sighting of a (part_index, name, signature) slot → emit a header
chunk carrying name/id and empty arguments. UI sees the call opening.
- Subsequent events update the slot's cached args; no further deltas are
emitted until the finishReason event.
- On finishReason → emit one chunk per slot with the final full args,
then the finish chunk.
The OpenAI streaming protocol concatenates `delta.tool_calls[].function
.arguments` by `index`; this scheme produces a single valid JSON string
per slot regardless of how often Gemini re-emits or mutates the call.
Tests:
- New `test_stream_event_translation_handles_non_prefix_args_change`
reproduces the bug — concat of emitted fragments must be parseable
JSON even when args change mid-stream.
- New `test_stream_event_translation_handles_prefix_grow_args` covers
growing-args case (final concat == final args).
- `test_stream_event_translation_emits_tool_call_delta_with_stable_index`
rewritten to assert on protocol-level concat instead of fragile
per-chunk ordering.
- `test_stream_event_translation_keeps_identical_calls_in_distinct_parts`
unchanged and still passing.
1 task
Contributor
|
Thanks for the focused provider-boundary fix. The underlying failure is present on current main: Problems
Suggested changes
GitHub reports this branch as conflicting, but the target streaming logic is still present on current main. This is an automated hermes-sweeper review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
json.dumps(args)on every SSE event that carries afunctionCall. Gemini re-sends the same call across repeated events and may mutate args mid-stream, so the consumer (which concatenatesdelta.tool_calls[].function.argumentsby index per OpenAI streaming protocol) ends up with invalid JSON like{"q": "A"}{"q": "AB"}, silently discarding the tool call.args_str.startswith(last_arguments)prefix-grow guard rarely fires on JSON: the trailing"/}mean a longer args dict almost never has the shorter one as a textual prefix (e.g.{"q":"abcdef"}doesn't start with{"q":"abc"}), so the fallback path emits the full new args.(part_index, name, signature)slot, emit a header chunk withname/idand empty arguments; cache the latest args across subsequent events; whenfinishReasonarrives, emit one chunk per slot with the final full args, then the finish chunk. Concatenation by the consumer yields a single valid JSON string per slot, regardless of how often Gemini re-emits or mutates the call.Test plan
tests/agent/test_gemini_native_adapter.py— full file passes (13/13)test_stream_event_translation_handles_non_prefix_args_change— reproduces the bug (would FAIL onmain:json.decoder.JSONDecodeError: Extra data: line 1 column 11 (char 10)on'{"q": "A"}{"q": "AB"}') and PASSES with this fixtest_stream_event_translation_handles_prefix_grow_args— final concat equals final args for the growing-args casetest_stream_event_translation_emits_tool_call_delta_with_stable_indexrewritten to assert on protocol-level concat instead of fragile per-chunk ordering — still passestest_stream_event_translation_keeps_identical_calls_in_distinct_parts— unchanged, still passesmcp_context7_resolve_library_id+web_search+delegate_taskin one assistant response. Pre-fix: tool-call args garbled into concatenated JSON, agent silently fell back to default behaviour. Post-fix: all three calls dispatched correctly and the structured answer returned.Notes
The header chunk carries
arguments: ""so consumers can open the tool-call slot (and UIs that surface "agent is calling X…" still get the signal), while the actual args land in a single chunk just before the finish chunk. This sidesteps both the dedup and the partial-args cases without needing the consumer to handle non-prefix-preserving fragments.