Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR refactors Gemini provider's schema handling to support multiple types in structured outputs by migrating from typed Schema objects to map-based JSON schemas with added normalization logic. Tests, documentation, and changelogs are updated accordingly to reflect the new approach for handling union types and complex nested structures. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (5)📚 Learning: 2025-12-30T05:37:48.365ZApplied to files:
📚 Learning: 2025-12-09T17:07:42.007ZApplied to files:
📚 Learning: 2025-12-29T11:54:55.836ZApplied to files:
📚 Learning: 2025-12-19T09:26:54.961ZApplied to files:
📚 Learning: 2025-12-15T10:16:21.909ZApplied to files:
🧬 Code graph analysis (2)core/providers/gemini/utils.go (2)
core/providers/gemini/gemini_test.go (6)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (51)
🔇 Additional comments (13)
Comment |
Pratham-Mishra04
force-pushed
the
01-05-fix_added_support_for_multiple_types_in_gemini_structure_outputs_properties
branch
from
02bbc2f to
c56dde9
Compare
Pratham-Mishra04
force-pushed
the
01-05-feat_added_structured_output_schema_normalisation_for_anthropic
branch
from
e8d777d to
4ea3f33
Compare
Pratham-Mishra04
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/providers/gemini/utils.go (2)
1001-1111: Consider adding recursion depth protection.The
normalizeSchemaForGeminifunction correctly implements the schema normalization logic:
- Keeps
["string", "null"]as-is (Gemini native support)- Converts
["string", "integer"]to anyOf constructs- Handles various edge cases properly
However, the recursive calls (lines 1071, 1081, 1089, 1102) lack depth protection, which could cause stack overflow with malicious or deeply nested schemas.
🔎 Suggested approach for recursion protection
Add a depth parameter to track recursion depth and return early if exceeded:
-func normalizeSchemaForGemini(schema map[string]interface{}) map[string]interface{} { +func normalizeSchemaForGemini(schema map[string]interface{}) map[string]interface{} { + return normalizeSchemaForGeminiWithDepth(schema, 0, 100) // max depth 100 +} + +func normalizeSchemaForGeminiWithDepth(schema map[string]interface{}, depth, maxDepth int) map[string]interface{} { if schema == nil { return nil } + if depth > maxDepth { + // Return schema as-is if max depth exceeded + return schema + } normalized := make(map[string]interface{}) // ... rest of logic, replacing recursive calls with: - newProps[key] = normalizeSchemaForGemini(propMap) + newProps[key] = normalizeSchemaForGeminiWithDepth(propMap, depth+1, maxDepth)
1051-1052: Add test coverage for enum removal when converting union types to anyOf.The code at lines 1051-1052 removes the
enumfield when converting multiple-type union schemas to anyOf format. While a comment explains the rationale, this significant schema transformation lacks test coverage. The Anthropic provider has explicit tests for this scenario (e.g., "type array with multiple types and enum"), whereas Gemini's tests only validate anyOf conversion for union types without enums. Add a test case covering enum removal with union types to document and validate this behavior.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/changelog.mdcore/providers/gemini/gemini_test.gocore/providers/gemini/responses.gocore/providers/gemini/utils.godocs/providers/supported-providers/gemini.mdxtransports/changelog.md
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
transports/changelog.mdcore/changelog.mddocs/providers/supported-providers/gemini.mdxcore/providers/gemini/gemini_test.gocore/providers/gemini/utils.gocore/providers/gemini/responses.go
🧠 Learnings (5)
📚 Learning: 2025-12-30T05:37:48.365Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1180
File: docs/features/mcp/connecting-to-servers.mdx:452-458
Timestamp: 2025-12-30T05:37:48.365Z
Learning: When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported. Documentation MDX files often reference features implemented in earlier stack PRs; verify that the documented behavior exists in earlier changes and that the docs accurately reflect the implemented state before requesting edits.
Applied to files:
docs/providers/supported-providers/gemini.mdx
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
core/providers/gemini/gemini_test.gocore/providers/gemini/utils.gocore/providers/gemini/responses.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.
Applied to files:
core/providers/gemini/gemini_test.gocore/providers/gemini/utils.gocore/providers/gemini/responses.go
📚 Learning: 2025-12-15T10:16:21.909Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/huggingface/huggingface_test.go:12-63
Timestamp: 2025-12-15T10:16:21.909Z
Learning: In provider tests under core/providers/<provider>/*_test.go, do not require or flag the use of defer for Shutdown(); instead call client.Shutdown() at the end of each test function. This pattern appears consistent across all provider tests. Apply this rule only within this path; for other tests or resources, defer may still be appropriate.
Applied to files:
core/providers/gemini/gemini_test.go
📚 Learning: 2025-12-19T09:26:54.961Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/providers/utils/utils.go:1050-1051
Timestamp: 2025-12-19T09:26:54.961Z
Learning: Update streaming end-marker handling so HuggingFace is treated as a non-[DONE] provider for backends that do not emit a DONE marker (e.g., meta llama on novita). In core/providers/utils/utils.go, adjust ProviderSendsDoneMarker() (or related logic) to detect providers that may not emit DONE and avoid relying on DONE as the sole end signal. Add tests to cover both DONE-emitting and non-DONE backends, with clear documentation in code comments explaining the rationale and any fallback behavior.
Applied to files:
core/providers/gemini/gemini_test.gocore/providers/gemini/utils.gocore/providers/gemini/responses.go
🧬 Code graph analysis (2)
core/providers/gemini/utils.go (1)
core/schemas/responses.go (2)
ResponsesTextConfig(120-123)ResponsesTextConfigFormat(125-130)
core/providers/gemini/responses.go (1)
core/providers/gemini/types.go (2)
Schema(729-780)Type(783-783)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (50)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (16)
core/providers/gemini/gemini_test.go (4)
30-30: Model version updated to target newer Gemini capabilities.The upgrade from
gemini-2.0-flashtogemini-2.5-flashaligns with testing the new structured output features introduced in this PR.
291-514: Comprehensive test coverage for structured output conversion.The test cases thoroughly validate:
- Union type conversion to
anyOfconstructs (e.g.,["string", "integer"]→anyOf: [{type: "string"}, {type: "integer"}])- Nullable types preserved as arrays (e.g.,
["string", "null"]remains as-is)- Complex nested schemas with proper structure
json_objectformat behavior (MIME type set, no schema attached)The test structure is clear, with explicit validation functions that check both the presence and correctness of transformed fields.
516-647: Responses API structured output conversion tests are well-implemented.The tests mirror the Chat Completions tests but target the Responses API conversion path (
ToGeminiResponsesRequest). This ensures consistent behavior across both API surfaces for:
- Union type normalization to
anyOf- Nullable type preservation as arrays
The parallel test structure helps maintain feature parity between the two APIs.
291-647: Excellent test coverage for the new structured output functionality.The new tests comprehensively validate the schema normalization logic for both Chat Completions and Responses APIs. The test structure is clear, assertions are thorough, and edge cases are covered (union types, nullable types, complex nested schemas, and json_object format).
transports/changelog.md (1)
1-1: Changelog appropriately reflects the broader feature scope.The updated entry correctly describes the feature as supporting multiple types in both Gemini and Anthropic structured outputs, rather than limiting the description to Anthropic-specific schema normalization.
core/changelog.md (1)
1-1: Changelog correctly reflects the feature expansion.The entry appropriately describes the broader support for multiple types in both Gemini and Anthropic structured outputs.
docs/providers/supported-providers/gemini.mdx (3)
43-43: Documentation accurately reflects the parameter mapping changes.The update from
responseSchematoresponseJsonSchemaaligns with the code changes and ensures users have the correct mapping information.
47-47: Gemini-specific parameter properly documented.The
top_kparameter is correctly documented as requiringextra_paramsfor Gemini-specific usage.
197-197: Responses API documentation updated consistently.The Responses API text mapping now correctly references
responseJsonSchemainstead ofresponseSchema, maintaining consistency with the Chat Completions API documentation.core/providers/gemini/responses.go (1)
1964-2009: Schema reconstruction enhanced with normalization and safer handling.The updated logic improves robustness:
- Type-safe casting: The Schema field is safely cast to
map[string]interface{}with fallback to the original value on failure (lines 1969-1972)- Field-by-field construction: When Schema is nil, individual fields are assembled into a schema map (lines 1975-2005)
- Empty schema handling: Returns
nilif no fields are populated, preventing empty schema objects (lines 2002-2005)- Normalization: Calls
normalizeSchemaForGeminito handle union types and ensure Gemini compatibility (line 2009)The normalization step is crucial for converting union types (e.g.,
["string", "integer"]) toanyOfconstructs, as validated by the test suite.core/providers/gemini/utils.go (6)
77-77: LGTM: Function call updated to match new signature.The call to
buildOpenAIResponseFormatcorrectly uses only theResponseJSONSchemaparameter, matching the refactored function signature at line 940.
420-423: LGTM: Schema extraction updated to use normalization.The updated logic correctly extracts and normalizes the schema using
extractSchemaMapFromResponseFormat, which now includes Gemini-specific normalization (handling union types). The assignment toconfig.ResponseJSONSchemaproperly reflects this normalized schema.
441-444: LGTM: ExtraParams handling updated consistently.The comment and parameter key are correctly updated to use
response_json_schema, maintaining consistency with the broader refactoring fromResponseSchematoResponseJSONSchema.
940-979: LGTM: Function refactored to handle schema-based response formatting.The refactored
buildOpenAIResponseFormatcorrectly:
- Returns
json_objectmode when no schema is provided- Returns
json_schemamode with proper structure when a schema exists- Extracts the title/name from the schema or defaults to "json_response"
- Includes appropriate fallback logic for invalid inputs
981-999: LGTM: Type extraction helper is well-implemented.The
extractTypesFromValuefunction correctly handles the different type representations (string, []string, []interface{}) that can appear in JSON schemas. The logic is straightforward and robust with proper type checking.
1113-1143: LGTM: Schema extraction now includes Gemini normalization.The addition of
normalizeSchemaForGeminiat line 1142 ensures that all schemas extracted from OpenAI's response_format structure are properly normalized for Gemini's requirements, handling union types and nullable types correctly.
Merge activity
|
akshaydeo
changed the base branch from
01-05-feat_added_structured_output_schema_normalisation_for_anthropic
to
graphite-base/1243
akshaydeo
deleted the
01-05-fix_added_support_for_multiple_types_in_gemini_structure_outputs_properties
branch
2 participants
Summary
Added support for multiple types in JSON schema properties for Gemini and Anthropic structured outputs, allowing for proper handling of union types like
["string", "integer"]in schema definitions.Changes
anyOfconstructs for Gemini when needed["string", "null"]) which Gemini supports nativelyType of change
Affected areas
How to test
Breaking changes
Related issues
Extends the schema normalization support previously added for Anthropic to Gemini provider.
Security considerations
No security implications as this only affects schema transformation logic.
Checklist