test(litellm): cover the tool call that carries no arguments - #5010
Open
yasumorishima wants to merge 1 commit into
Open
test(litellm): cover the tool call that carries no arguments#5010yasumorishima wants to merge 1 commit into
yasumorishima wants to merge 1 commit into
Conversation
yasumorishima
force-pushed
the
fix/malformed-tool-call-json
branch
from
March 26, 2026 17:53
c670d6c to
3809fd5
Compare
rohityan
added
models
Collaborator
|
Hi @yasumorishima , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share. |
Collaborator
|
Hi @wukath , can you please review this. |
yasumorishima
force-pushed
the
fix/malformed-tool-call-json
branch
from
May 3, 2026 03:56
a8b3c45 to
9a581fc
Compare
yasumorishima
force-pushed
the
fix/malformed-tool-call-json
branch
from
September 5, 2026 01:03
4a9e34b to
dbe7c09
Compare
yasumorishima
changed the title
yasumorishima
force-pushed
the
fix/malformed-tool-call-json
branch
from
September 5, 2026 01:29
dbe7c09 to
622103a
Compare
`_parse_tool_call_arguments` returns `{}` for a falsy `arguments`
(src/google/adk/models/lite_llm.py:203-204), so a tool call that carries no
arguments is dispatched with empty args and is *not* reported as malformed.
Nothing covered that: test_litellm.py has no case with empty arguments outside
a streaming fixture that accumulates a JSON payload split across chunks, and
6e59663 covers the JSONDecodeError path and its streaming equivalent.
The distinction matters because the output alone cannot tell the two apart.
With the guard removed, `""` falls through to `except json.JSONDecodeError`
and still yields `{}` -- the only difference is the warning. So the test
asserts the empty args *and* that nothing was logged as malformed; removing
the guard makes it fail.
The malformed-JSON handling this PR originally proposed landed in 6e59663
(Close google#5896) with better semantics than mine -- dispatch with empty arguments
so the tool can return a structured error and the model can retry, rather than
dropping the call -- so the source change is dropped in favour of upstream's.
yasumorishima
force-pushed
the
fix/malformed-tool-call-json
branch
from
September 5, 2026 02:02
622103a to
acfb83a
Compare
yasumorishima
changed the title
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
2 participants
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 malformed-JSON handling this PR originally proposed landed in 6e59663 (Close #5896), with better semantics than mine: it dispatches the call with empty arguments so the tool can return a structured error and the model can retry, instead of dropping the call. I have rebased onto that and dropped my source change in favour of it.
What is left is one case that commit did not cover.
_parse_tool_call_argumentsreturns{}for a falsyarguments(src/google/adk/models/lite_llm.py:203-204), so a tool call carrying no arguments is dispatched with empty args and is not reported as malformed. On main the only emptyargumentsoutside that guard is a streaming fixture accumulating a JSON payload split across chunks, and 6e59663's own tests cover theJSONDecodeErrorpath and its streaming equivalent.The distinction is not visible in the output alone: with the guard removed,
""falls through toexcept json.JSONDecodeErrorand still yields{}. The only difference is the warning. So the test asserts the empty args and that nothing was logged as malformed — I checked that removing the guard makes it fail.Tests only, no source change.
pytest tests/unittests/models/test_litellm.py→ 422 passed.🤖 Generated with Claude Code