fix(agent): prefix stripping and suffix finalization for Gemini native streaming - #57165
Conversation
Competing fix with #25046 for the same Gemini native-streaming tool-call corruption (stacked/duplicated JSON args) in
Maintainer picks the canonical approach. |
|
Thank you for the triage and for linking the two competing PRs, @alt-glitch! To assist the maintainers in evaluating the two approaches, here is a quick technical comparison of this PR vs #25046 (flush-on-finish): 1. The Core Problem with Gemini's StreamGemini's native stream API returns fully accumulated arguments dicts in each chunk (e.g.,
2. Architectural ComparisonApproach A: Prefix Stripping & Suffix Restoration (This PR)
Approach B: Flush-on-Finish (#25046)
3. Conclusion & RecommendationWhile Approach B (#25046) provides a valid workaround for JSON compliance, it compromises the core visual benefit of stream-based execution by turning a live, interactive streaming experience into a blocking non-streaming experience under the hood. This PR (#57165) preserves both 100% JSON compliance and the rich, real-time interactive stream UX of the Hermes CLI. We welcome the maintainers' feedback on which design paradigm fits the project's standards best! |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real native-Gemini streaming defect; current main can emit a second full argument object at agent/gemini_native_adapter.py:714-721, while the stream consumer appends fragments at agent/chat_completion_helpers.py:2500-2501.
Problems
agent/gemini_native_adapter.py:723-739drops JSON closers when a non-final event is followed by an identical final replay: the equality path emits nothing and setsfinal_emitted, so the finish sweep cannot emit the stored suffix.agent/gemini_native_adapter.py:671-692resetsfunction_call_counterfor each SSE event. Distinct one-part events for the same tool both select ordinal0, reuse a slot, and the fallback at:732-735appends a replacement JSON object into that slot.- The tests at
tests/agent/test_gemini_native_adapter.py:464-556do not concatenate every delta and parse the final per-slot argument string; they therefore miss both cases above.
Suggested changes
- Cover non-final → identical-final replay and assert concatenated deltas parse as the final arguments.
- Use persistent call disambiguation across events, and never append a full replacement object to an existing append-only slot.
Automated hermes-sweeper review.
| last_stripped = str(slot.get("last_stripped") or "") | ||
| emitted_arguments = "" | ||
|
|
||
| if args_str == slot.get("last_full", ""): |
There was a problem hiding this comment.
If a non-final event emitted the stripped prefix and the finish event repeats identical args, this branch emits nothing; final_emitted below then prevents the finish sweep from restoring the closing quote/brace. Please emit the outstanding suffix before finalizing and add that exact replay sequence as a regression.
| call_key = json.dumps( | ||
| { | ||
| "part_index": part_index, | ||
| "function_call_index": function_call_index, |
There was a problem hiding this comment.
function_call_counter is reset for every translate_stream_event call, so distinct one-part SSE events for the same tool both key as ordinal 0 and reuse this slot. A non-prefix second call then hits the full-object fallback and is concatenated downstream; identity must persist or be disambiguated across events.
Summary of Changes
This PR fixes a critical bug in the native Gemini adapter (
agent/gemini_native_adapter.py) that causes duplicated and concatenated tool call arguments (resulting in invalid stacked JSON like{"pattern": "*.py"}{"pattern": "*.py", "target": "files"}) during streaming tool calls.Root Cause
Unlike OpenAI or Anthropic, Google's Gemini native SSE stream returns fully accumulated tool argument dictionaries in each chunk instead of incremental string deltas.
In the original translation logic:
args_str.startswith(last_arguments).",},]) representing the closed state of the JSON up to that point.last_argumentsmismatch with the new, expanding stream content at that position (e.g.,,vs}).startswithto returnFalse, forcing the adapter to emit the entire updated argument string again. The final aggregated stream is corrupted with repeated and stacked JSON objects, crashing any stream-based JSON parser.Solution
We implemented a robust prefix stripping and suffix finalization sliding window:
is_final = False), we right-strip trailing JSON closures (rstrip(' \t\n\r"}]')) before storing them inlast_strippedand calculating the emitted delta. This prevents trailing closures from breaking prefix matching in subsequent stream events.finishReasonor EOF), we do not strip the suffixes. The adapter calculates the remaining delta from the full string, safely emitting the final trailing closures (e.g.,def"}) to cleanly terminate the JSON object.finishReasonbeing set on a candidate, the generator loops through and finalizes any remaining unfinalized tool calls.Testing
We added a dedicated regression test suite in
tests/agent/test_gemini_native_adapter.py:test_stream_event_translation_with_prefix_stripping_and_suffix_finalizationtest_stream_event_translation_parallel_calls_with_disappearing_partsThese tests verify that arguments are correctly diffed, stripped, and finalized across simulated sequential events, ensuring 100% valid JSON generation.
All tests passed successfully on our local environment:
pytest tests/agent/test_gemini_native_adapter.py -k "prefix_stripping or parallel"