-
Notifications
You must be signed in to change notification settings - Fork 750
fix: handle complex nested schemas in gemini function calling & structured outputs #1233
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
fix: handle complex nested schemas in gemini function calling & structured outputs #1233
Conversation
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.
This PR adds a useful utility function to transform JSON schemas for Gemini tool parameters. The implementation looks solid, but I have a few suggestions to improve error handling and code clarity.
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.
I would recommend adding comprehensive test cases for this function as it is in the critical path of the requests.
Please add test cases with all supported combinators like anyOf, allOf and not, etc.
} else if ( | ||
key === 'anyOf' && | ||
Array.isArray(value) && | ||
value.length === 2 |
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.
does google not support nullable field with more than one anyOf fields?
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.
Yes, type: null is not allowed. If we want to imply a field is optional, we should use nullable
field
References I found:
- Nullable types not supported in pydantic classes for json googleapis/python-genai#62
- https://cloud.google.com/vertex-ai/generative-ai/docs/model-reference/function-calling#schema
Input should be 'TYPE_UNSPECIFIED', 'STRING', 'NUMBER', 'INTEGER', 'BOOLEAN', 'ARRAY' or 'OBJECT'

Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
if (Object.hasOwn(schema, '$schema')) { | ||
delete schema['$schema']; | ||
} |
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.
This has been already handled at recursivelyDeleteUnsupportedParameters, hence removed
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
@narengogi have added tests, updated code to fix both function calling and structured outputs |
for (const [key, value] of Object.entries(node)) { | ||
if (key === 'enum' && Array.isArray(value)) { | ||
transformed.enum = value; | ||
transformed.format = 'enum'; |
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.
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.
I have tested by removing format: enum customization for both function calling and structured outputs that I followed from instructor library, it worked. Hence removed and updated PR
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
if (stack.has(defKey)) return node; | ||
stack.add(defKey); | ||
const resolved = derefer(target, activeDefs, stack); | ||
stack.delete(defKey); |
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.
I think stack
will always be empty here?
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.
stack won’t always be empty here, it is shared across the current recursion path. After stack.delete(defKey) we’ve only removed the current key, but ancestor defKeys from higher frames can still be in the set.
Example:
A -> B -> A (cycle)
After B finishes: stack = {A}
if (defKey && target) { | ||
if (stack.has(defKey)) return node; | ||
stack.add(defKey); | ||
const resolved = derefer(target, activeDefs, stack); |
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.
Please correct me if wrong, the derefered object won't be having any $ref
key right? atleast I didn't find from the schema you've attached in test file. Why are we trying to derefer the object with no $ref
.
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.
target itself often has no $ref at the root, but it can contain nested $refs. I updated the tests to show this: contact -> $ref: ContactInfo, and inside ContactInfo we have address: { $ref: "#/$defs/PostalAddress" }.
Calling derefer(target, …) walks that subtree so contact.properties.address becomes an inline object.
If a def truly has no nested refs (like Job / StatusEnum), the call is a quick no-op (we just traverse its fields and return unchanged), but it keeps the behavior consistent.
const defKey = getDefFromRef(node.$ref); | ||
const target = getDefObject(activeDefs, defKey); | ||
if (defKey && target) { | ||
if (stack.has(defKey)) return node; |
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.
if at all stack is not empty and we find defKey
in stack we're returning node
, does node
we're returning is already defered?
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.
This return is intentional, when stack.has(defKey) is true, it means we have hit a cycle. We keep the $ref to avoid infinite recursion rather than fully expanding it again.
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.
Example:
from pydantic import BaseModel
class A(BaseModel):
field1: Optional["A"] = None
schema = A.model_json_schema()
const schema = {
"$defs": {
"A": {
"properties": {
"field1": {
"anyOf": [
{"$ref": "#/$defs/A"},
{"type": "null"}
],
"default": null
}
},
"title": "A",
"type": "object"
}
},
"$ref": "#/$defs/A"
};
let derefed = derefer(schema);
delete derefed.$defs;
console.log(JSON.stringify(derefed, null, 2));
{
"properties": {
"field1": {
"anyOf": [
{
"properties": {
"field1": {
"anyOf": [ {"$ref": "#/$defs/A"}, {"type": "null"} ],
"default": null
}
},
"title": "A",
"type": "object"
},
{ "type": "null" }
],
"default": null
}
},
"title": "A",
"type": "object"
}
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.
Got it, I think this will still cause failure at vertex as anyOf
is not at all supported.
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
const keys = Object.keys(node); | ||
if (keys.length === 1) return resolved; | ||
const { $ref: _, ...siblings } = node; | ||
return derefer({ ...resolved, ...siblings }, activeDefs, stack); |
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.
I'm not sure if this intended or not, why do we need to de-ref the siblings when there's a loop for every iteration that does part? L231-235(diff), am I missing something?
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.
My bad, fixed it. Thanks for flagging.
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.
Added few comments, but rest looks good to me.
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
Description
This PR fixes multiple issues of using function calling and structured outputs with gemini models.
Primarily supporting / fixing enums, nested schemas, null property and completely removing $defs and $ref which were sent as is in most cases
Motivation
Without this, it is not possible to use Gemini's structured data / function calling except with simplest schemas - with absolutely no complexity like:
Type of Change
How Has This Been Tested?
Testing code:
Base code:
Using Function calling:
Current
main
branch:Request sent from Portkey, wrapped up as request is too long (click to expand)
fix/response-schemas-for-gemini-models
branch:Request sent from Portkey (click to expand)
Response:
Using Gemini Structured outputs (JSON Schema) - Ignore system prompt being added, that is from instructor library being added in JSON_SCHEMA mode
Current
main
branch:Request sent from Portkey (click to expand)
Response:
fix/response-schemas-for-gemini-models
branch:Request sent from Portkey (click to expand)
Response:
Checklist
Related Issues
#837 (comment)
https://discord.com/channels/1143393887742861333/1317117120676364390/1317125306124992523