Repository navigation
Conversation
ProcessResponse checked len(genResp.Candidates) > 0 but then dereferenced genResp.Candidates[0].FinishReason without a nil check. converters.Genai2LLMResponse, run on the same response one line above, already guards `len(res.Candidates) > 0 && res.Candidates[0] != nil` (a comment notes Vertex gemini-3* streams can emit degenerate chunks), so a nil first candidate is reachable and panics the stream. Add the same nil guard. Add a regression test feeding a chunk with a nil first candidate.
|
Thanks for the PR, @PratikDhanave. One thing before review. Our contribution guide asks for an issue behind each change, and I could not find one linked here.
If this is a small doc or typo fix, ignore me. A maintainer can open the issue for you if you would rather. |
wojtas000
approved these changes
Oct 12, 2026
wojtas000
left a comment
wojtas000
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thanks. I reproduced the panic end to end: a Vertex-backend stream with a null candidate entry reaches ProcessResponse and panics at stream_aggregator.go:64 on main, and completes cleanly with this change. Small note for the record: the gemini-3* comment in converters.go is about chunks with no candidates, not nil ones, so I'll adjust the squash message.
wolo-lab
added a commit
that referenced
this pull request
Oct 14, 2026
#785, #1719, #1804, #1802) (#1837) * fix(utils): respect Nullable field on nil values during schema validation (#1566) (cherry picked from commit 0e5797f) * fix(llminternal): re-evaluate toolsets on every step in toolProcessor (#785) (cherry picked from commit 707561d) * fix(llminternal): omit foreign thoughts from agent context (#1719) (cherry picked from commit a03e29e) * fix(llminternal): correct misleading "global" in appendInstructions error (#1804) (cherry picked from commit a3afd7f) * fix(llminternal): guard nil first candidate in streaming aggregator (#1802) (cherry picked from commit 1862aae)
6 of 8 tasks
wojtas000
added a commit
to Sega757/adk-go
that referenced
this pull request
Nov 4, 2026
The previous commit replaced the file with an older copy, which reverted the JSON path parser (google#1630), the nil first-candidate guard (google#1802) and usage metadata retention (google#1257), and did not compile.
wolo-lab
added a commit
that referenced
this pull request
Nov 6, 2026
…or (#2051) * perf(llminternal): use strings.Builder in streaming response aggregator * perf(llminternal): use strings.Builder for the streaming text buffer * revert(llminternal): restore stream_aggregator.go from main The previous commit replaced the file with an older copy, which reverted the JSON path parser (#1630), the nil first-candidate guard (#1802) and usage metadata retention (#1257), and did not compile. * perf(llminternal): use strings.Builder for the streaming text buffer Appending each streamed chunk with += recopied the whole text received so far, so aggregating a long reply was quadratic in its length. The test checks that doubling the chunk count about doubles the bytes allocated. --------- Co-authored-by: wojtasjan <[email protected]> Co-authored-by: wolo <[email protected]>
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
streamingResponseAggregator.ProcessResponse(internal/llminternal/stream_aggregator.go) checkslen(genResp.Candidates) > 0but then dereferencesgenResp.Candidates[0].FinishReasonwith no nil check.converters.Genai2LLMResponse— run on the same response one line above — already guardslen(res.Candidates) > 0 && res.Candidates[0] != nil(with a comment that Vertexgemini-3*streams can send degenerate chunks). So a nil first candidate is reachable and panics the stream.Change
Add
&& genResp.Candidates[0] != nilto the guard.Verification
Regression test feeds a chunk with a nil first candidate — panics before the fix (nil pointer dereference), passes after.
go test ./internal/llminternal/green.