fix(gemini): infer missing type:"object" for schema nodes with properties#1941
fix(gemini): infer missing type:"object" for schema nodes with properties#1941Blue-B wants to merge 2 commits intorouter-for-me:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical compatibility issue with the Gemini API by enhancing the schema cleanup process. It introduces a mechanism to automatically infer and set the type of schema nodes to object when they contain properties but omit an explicit type declaration. This ensures that tool schemas, particularly from systems like MCP and Droid CLI, are correctly interpreted and accepted by the Gemini API, preventing tool-use failures. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix an issue with Gemini tool schemas by ensuring that any schema node with a properties field also has type: "object". The implementation introduces a new function, inferMissingObjectType, into the schema cleanup pipeline. However, there's a critical issue in the new function where a conditional check always evaluates to true, making the function a no-op and preventing the fix from working. My review includes a comment to address this.
internal/util/gemini_schema.go
Outdated
| if isPropertyDefinition(p) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The condition isPropertyDefinition(p) will always evaluate to true for any path p returned by findPaths(jsonStr, "properties"). The findPaths function returns paths that end with the properties key (e.g., a.b.properties), and isPropertyDefinition checks for exactly that pattern. As a result, the continue statement is always executed, which makes this new function a no-op and prevents it from fixing the issue described in the pull request. This check appears to be erroneous and should be removed to allow the function to work as intended.
Add inferMissingObjectType to the Gemini schema cleanup pipeline. When a schema node has "properties" but no "type" field, Gemini API rejects it. This fix infers type:"object" for such nodes, preventing tool_use failures with Gemini models.
Blue-B
force-pushed
the
fix/gemini-infer-missing-object-type
branch
from
e433d91 to
fe7b1ee
Compare
|
Fixed — moved the |
luispater
left a comment
luispater
left a comment
There was a problem hiding this comment.
Summary:
This moves the Gemini schema cleanup in the right direction, but there is still a correctness gap for valid schemas that contain a literal property named properties.
Key findings:
- Blocking: the new inference still skips schema nodes under a field literally named
properties.Walk()builds paths likeproperties.properties.properties, andisPropertyDefinition()still classifies those by suffix, so the target node is treated as apropertiescontainer instead of a field definition. In that case we still fail to addtype: "object", so the original Gemini rejection remains possible. - Non-blocking: please add regression coverage for both (1) a normal node with
propertiesbut notype, and (2) the edge case where a field is literally namedproperties.
Test plan:
- Reviewed the diff and the schema traversal helpers.
- Verified the path-classification behavior with a minimal reproduction based on the current
Walk/isPropertyDefinitionlogic.
…gObjectType Replace isPropertyDefinition with isSchemaPropertiesKeyword that correctly distinguishes JSON Schema 'properties' keywords from fields named 'properties' by walking the path and tracking schema-vs-field depth. Add regression tests for both normal and edge-case schemas.
|
Addressed both blocking and non-blocking feedback — pushed a fix:
Please take another look when you get a chance. |
2 participants
Gemini API rejects tool schemas where a node has properties but no type field
Added inferMissingObjectType to the schema cleanup pipeline. It walks the schema tree and sets type: "object" on any node that has properties without an explicit type.
Fixes tool_use failures when MCP tools or Droid CLI send schemas without explicit object types.