-
Notifications
You must be signed in to change notification settings - Fork 73
fix: prevent premature loop termination for Gemini 3 tool calls #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Gemini 3 models send an explicit STOP finish reason after tool calls, causing runAsyncImpl to terminate before processing the tool response. Changes: - Track function responses in runAsyncImpl loop - Continue loop if pending function response exists - Include gemini-3 in isGemini2Model() for code executor compat
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
core/src/utils/model_name.ts
Outdated
| const modelName = extractModelName(modelString); | ||
|
|
||
| return modelName.startsWith('gemini-2'); | ||
| return modelName.startsWith('gemini-2') || modelName.startsWith('gemini-3'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it contradicts the function name isGemini2Model, it would be better to create a separate function for checking gemini-3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| while (true) { | ||
| let lastEvent: Event|undefined = undefined; | ||
| let hasFunctionResponse = false; | ||
| for await (const event of this.runOneStepAsync(context)) { | ||
| lastEvent = event; | ||
| if (getFunctionResponses(event).length > 0) { | ||
| hasFunctionResponse = true; | ||
| } | ||
| this.maybeSaveOutputToState(event); | ||
| yield event; | ||
| } | ||
|
|
||
| if (!lastEvent || isFinalResponse(lastEvent)) { | ||
| if (!lastEvent || (isFinalResponse(lastEvent) && !hasFunctionResponse)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be wrong as it will not break when there is final response but there is no any function response. It can lead to infinity loop as we are using unsafe while (true) practice.
I think we might need to find out another solution for that problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review ,based on my reading of the code, it looks like the loop does break when there is a final response and no function response. In that case, isFinalResponse(lastEvent) is true and hasFunctionResponse remains false, so the condition evaluates to true && true and exits the loop.
Basically this:
while (true) {
let hasFunctionResponse = false; // resets each iteration
for await (const event of this.runOneStepAsync(context)) {
if (getFunctionResponses(event).length > 0) {
hasFunctionResponse = true;
}
}
if (!lastEvent || (isFinalResponse(lastEvent) && !hasFunctionResponse)) {
break; // line 1512
}
}The hasFunctionResponse flag resets to false at the start of each iteration. As we process events from runOneStepAsync, if we encounter any function responses, we set it to true. After processing all events, we check whether we received a final response without any function responses. If so, we break. The only case where we continue looping is when hasFunctionResponse is true, meaning we just processed a tool response and need to send it back to the model for another turn.
Note the while(true) pattern was already present in the original code:
while (true) {
// ...
if (!lastEvent || isFinalResponse(lastEvent)) {
break;
}
}I only modified the break condition, not the loop structure .
Confirming this issue - same behavior observedI'm experiencing the exact same problem with gemini-3-flash-preview. The Runner.runAsync terminates immediately after receiving the tool response with finishReason: 'STOP', preventing the model from generating a synthesis response. Environment:
Dependencies: Use Case: SSE Event Flow Observed:phase: thinking → agent_start → search_start → phase: searching → tool_call The tool_result contains complete data from the sub-agent, but the stream terminates without any content event from the parent agent. Workaround:Switching to gemini-2.5-flash resolves the issue - the model correctly continues generating responses after tool execution. Looking forward to this fix being merged. Thanks @victorbash400 for the PR! |
Gemini 3 models send an explicit STOP finish reason after tool calls, causing runAsyncImpl to terminate before processing the tool response.
Changes: