Repository navigation
Conversation
Greptile SummaryThis PR preserves Google GenAI function declarations using the standard parameters field while retaining parametersJsonSchema precedence
Confidence Score: 4/5The PR appears safe to merge, with only non-blocking repository-organization and test-comment cleanup requested The fallback preserves the intended schema and existing precedence without an established runtime regression; remaining findings concern repository conventions and maintainability Files Needing Attention: litellm/google_genai/adapters/transformation.py, tests/test_litellm/google_genai/test_google_genai_adapter.py
|
| Filename | Overview |
|---|---|
| litellm/google_genai/adapters/transformation.py | Correctly adds the schema fallback and normalization path, but extends provider-specific logic outside the required provider hierarchy |
| tests/test_litellm/google_genai/test_google_genai_adapter.py | Adds focused regression and precedence coverage, with unnecessary explanatory docstrings under the repository comment policy |
Reviews (1): Last reviewed commit: "fix(google_genai): read functionDeclarat..." | Re-trigger Greptile
| elif "parameters" in func_decl: | ||
| function_chunk["parameters"] = func_decl["parameters"] |
There was a problem hiding this comment.
Provider logic outside standard hierarchy
This Google GenAI-specific branch extends provider behavior outside litellm/llms, increasing fragmentation and making future provider maintenance harder.
Rule Used: What: Avoid writing provider-specific code outside... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
This sits beside the existing parametersJsonSchema read in the same adapter. Relocating the bridge into litellm/llms is a separate refactor
There was a problem hiding this comment.
Fair point — the new elif is a one-liner that mirrors the existing parametersJsonSchema branch immediately above it, so it doesn't add new fragmentation. The adapter already lives outside litellm/llms, and relocating the whole bridge is a separate concern. The fix is correctly scoped.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88cb413656
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…teContent bridge The generate_content -> chat/completions bridge only read functionDeclarations[].parametersJsonSchema, the field gemini-cli sends. The google-genai SDKs serialize tool schemas to `parameters`, so those requests reached the upstream provider with a function that had a name and description but no arguments, and the model invented its own. Fall back to `parameters` when `parametersJsonSchema` is absent, and declare it on _GenAIFunctionDeclaration.
…only Mapping The new TypedDict field tripped the LIT001 type-discipline budget by adding one more mutable collection annotation. Declare it as Mapping[str, object] instead, which is the read-only view the rule asks for, leaving the budget at its base total.
012bef4 to
24e8619
Compare
The names and assertions already say what each case covers, so the prose only duplicated them.
…ration-parameters Conflicts in litellm/google_genai/adapters/transformation.py: keep the base's imports, and annotate the new `parameters` field as ReadOnly[object] to match `parametersJsonSchema`, which the base now declares that way.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Closing in favor of #42067, which landed the tool parameters fix together with response_schema forwarding. Thanks vovanphuc for the groundwork here |
TLDR
Problem this solves:
How it solves it:
functionDeclarations[].parameterswhenparametersJsonSchemais absentparametersJsonSchemastill wins when both are presentUser Flow
Before: a developer whose app uses the Google GenAI SDK against the proxy gets function calls back with empty arguments as soon as the model name is served by a non-Gemini deployment
POST https://litellm-domain/v1beta/models/my-model:generateContent, wheremy-modelis served by an OpenAI-compatible deployment, declaring one toolget_weatherthat takes a required stringcity200back with{"functionCall":{"name":"get_weather","args":{}}}, the right function with no argumentsget_weather()with no city and raises aKeyError{"args":{"location":"Hanoi"}}, a parameter name that appears nowhere in the tool they declaredmy-gemini-model, send the identical request, and get{"functionCall":{"name":"get_weather","args":{"city":"Hanoi"}}}, so the same client code works on one model and silently misbehaves on anotherAfter: the same request returns the arguments the tool declared, whichever provider serves the model
POST https://litellm-domain/v1beta/models/my-model:generateContent, wheremy-modelis served by an OpenAI-compatible deployment, declaring one toolget_weatherthat takes a required stringcity200back with{"functionCall":{"name":"get_weather","args":{"city":"Hanoi"}}}get_weather(city="Hanoi")and returns the right weathercityargument every timemy-gemini-modelreturns the identical result, so behaviour no longer depends on which provider backs the model nameRelevant issues
Fixes #37567
Linear ticket
Pre-Submission checklist
uv run pytest tests/test_litellm/<your_test_file>.py -vScreenshots / Proof of Fix
Shared setup for both runs:
ghcr.io/berriai/litellm:v1.100.1with the source file under test mounted in, same config and promptThe request, sent 3 times against a live proxy hitting the real provider
Before (c2c2a62)
parametersJsonSchemainstead, same proxy and model and prompt, and the arguments arrive intact, so the upstream model and the provider are fineAfter (9b0cbff)
The prompt token count rises from 49 to 59, which is the schema now actually leaving the proxy
Type
🐛 Bug Fix
Caveats (if any)
parametersJsonSchemakeeps priority, so gemini-cli is untouchedReadOnly[object], matchingparametersJsonSchemaon the baseFinal Attestation