fix(gemini): drop empty enum values from translated schemas - #4688
fix(gemini): drop empty enum values from translated schemas#4688lyx1311 wants to merge 2 commits into
Conversation
Gemini rejects enum entries that become empty strings after
gjson null→"" conversion ("enum[n]: cannot be empty"). Filter
null/empty values and delete the enum keyword when nothing valid
remains, preserving forceStringType behavior for tool vs response
schemas.
Field-validated against v7.2.110; ports the approach from router-for-me#4545 onto
the current convertEnumValuesToStrings(jsonStr, forceStringType) API.
e748916 to
447c5d0
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 447c5d0264
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if len(stringVals) == 0 { | ||
| jsonStr, _ = sjson.Delete(jsonStr, p) | ||
| continue |
There was a problem hiding this comment.
In the Gemini/Antigravity tool cleaners forceStringType is true, but this all-invalid branch returns before the type rewrite below. For generated schemas such as {"type":"null","enum":[null]} or {"const":null} (which convertConstToEnum turns into an all-null enum), the cleaner now removes enum and leaves type:"null" or no type instead of the normalized tool-schema string type used for other enum nodes, so these requests can still reach Gemini in an incompatible shape. Please still apply the forced parent type when forceStringType is set before continuing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in d5200e5: when all enum values are dropped, still apply forceStringType on the parent type (with tests covering Gemini tool cleaner and Antigravity). Ready for re-review.
When all enum values are null/empty and the enum keyword is removed, still apply forceStringType for tool cleaners so const:null and type:null schemas do not leave an incompatible type:null parent. Addresses Codex review on router-for-me#4688.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Problem
Gemini rejects tool/request schemas with:
(HTTP 400) whenever an enum array contains
nullor empty string values. This shape is common for optional enums produced by schema generators (for example listingnullnext to string members). The whole request fails, not just the affected property.Cause
convertEnumValuesToStringsininternal/util/gemini_schema.gonormalizes every enum entry withgjson.Result.String(). For JSONnullthat returns"", so the empty string is written back into the enum array and forwarded to Gemini, which only accepts non-empty strings there.Fix
In the current two-arg API
convertEnumValuesToStrings(jsonStr, forceStringType):nulland empty-string entries while normalizingenumkeyword instead of emitting an empty arrayforceStringTypebehavior unchanged (tools still forcetype: string; response schemas keep declared types)convertEnumValuesToStringsruns beforeaddEnumHints, so downstream hint text no longer picks up empty values either.Relation to #4545
This PR implements the same fix intent as #4545 by @RsLuna7, rebased onto the current
forceStringTypeAPI.#4545 still targets the older one-arg signature and is currently merge-conflicted (
dirty) againstdev. This PR is a clean port for maintainers to land without resolving that conflict first. Please close #4545 as superseded if this merges (or the reverse if you prefer cherry-picking from there).Credit for the original diagnosis and approach: @RsLuna7 / #4545.
Tests
TestCleanJSONSchemaRemovesEmptyEnumValues— mixed enum keeps valid values; empty-only enum is removed; covered for Gemini / Antigravity / Antigravity response cleanersTestCleanJSONSchemaForAntigravity_RemovesNullAndEmptyEnumValues— field-shaped nullable enums (todos[].status,capability_mode) matching real tool schemasVerification
gofmtclean on both filesgo test ./internal/util -count=1— passProduction evidence
The same filtering logic was applied on a deployment based on release v7.2.110 (where
gemini_schema.gomatches currentdevfor this function). Previously failing requests that returned Geminienum[n]: cannot be empty(HTTP 400) returned HTTP 200 after the change; Gemini tool-calling paths (gemini-3-flashand related) succeeded end-to-end.Notes
internal/util/gemini_schema.go+ tests.