Repository navigation
Conversation
…etions The OpenAI chat completions adapter discarded json.Unmarshal errors for tool call arguments, so a call whose arguments were cut off at the token limit still ran, with no arguments. Fail the turn with MALFORMED_FUNCTION_CALL instead, and run none of the tool calls in that response. Blank arguments still mean a call without arguments, unless the response was cut off at the token limit. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
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.
The OpenAI chat completions adapter ignores the error from
json.Unmarshalwhen it decodes tool call arguments (L462 and L521). When the model hits the token limit in the middle of a tool call, the arguments are cut off, the tool runs with no arguments, and the turn carries on as if nothing went wrong.This fails the turn instead, with
ErrorCodeset toMALFORMED_FUNCTION_CALL, along the lines of this review comment on #2230. None of the tool calls in that response run, not even complete ones next to the broken one, so a batch of parallel calls never runs halfway. Text before the calls is kept. Blank arguments still mean a call without arguments, except when the finish reason islength, because then the call may have been cut off before its arguments. That check relies on the server reportinglength; arguments cut off mid-JSON fail either way.I used
ErrorCoderather than a Go error. This file already reportsSTREAM_ERRORandAPI_ERRORthat way, adka2a marks the task failed on it, and the turn's text and event still reach the session. With a Go error, the session keeps only the user's message. adk-go's own OpenAI model returns a Go error (ErrFunctionCallArgs) for arguments it can't decode, so this is a deliberate difference.Some responses whose tool calls used to run silently now fail the turn: arguments that are valid JSON but not an object (
nullstill means no arguments), streamed parallel calls without anindex(main merges them into one call and runs it with no arguments), and chunks that repeat the full arguments. Azure OpenAI, Mistral and Foundry (except Claude deployments) use this adapter, so they behave the same way.This is separate from #2896, which fails the task when the last response stops at
max_tokens. A cut-off tool call runs before that point.In Go, the Responses API path, SAP AI Core, Anthropic streaming and Bedrock (which passes
_raw) still don't fail the turn. In Python, the OpenAI and SAP AI Core adapters fall back to{}, and Bedrock streaming fails the turn withAPI_ERROR. I'd rather do those in follow-up PRs, as suggested on #2230.Testing
TestOpenAIModelGenerateContentRejectsMalformedToolArgumentsruns both stream modes against an httptest server, with valid arguments, blank andnullarguments, arguments cut off mid-JSON, a call cut off before its arguments, one of two parallel calls cut off, and arguments that aren't an object. The four error cases fail on main and pass here. The other two pass on both.go test -race ./adk/...passes, andmake -C go lintis clean.