Skip to content

Commit 4031075

Browse files
committed
Fix code review issues: allOf determinism, field preservation, number precision, JSON Schema support
- Fix non-deterministic allOf merging by processing before main iteration - Preserve all schema fields in mergeAllOf (description, title, enum, etc.) - Use json.NewDecoder with UseNumber() to preserve number precision - Change response_schema to responseJsonSchema for full JSON Schema support - Preserve additionalProperties constraints instead of removing them
1 parent 7d50806 commit 4031075

1 file changed

Lines changed: 73 additions & 35 deletions

File tree

internal/translator/gemini/openai/responses/gemini_openai-responses_request.go

Lines changed: 73 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -415,12 +415,12 @@ func ConvertOpenAIResponsesRequestToGemini(modelName string, inputRawJSON []byte
415415
}
416416
out, _ = sjson.SetBytes(out, "generationConfig.response_mime_type", "application/json")
417417

418-
// Set response_schema from the schema field, normalizing it for Gemini's
419-
// supported JSON Schema subset (removes unsupported keywords like additionalProperties,
420-
// pattern, minLength, multipleOf, etc.)
418+
// Set responseJsonSchema from the schema field, normalizing it for Gemini's
419+
// supported JSON Schema subset (removes unsupported keywords like pattern,
420+
// minLength, multipleOf, etc. but preserves additionalProperties, $defs, $ref)
421421
if schema := textFormat.Get("schema"); schema.Exists() {
422422
schemaStr := normalizeJSONSchema(schema.Raw)
423-
out, _ = sjson.SetRawBytes(out, "generationConfig.response_schema", []byte(schemaStr))
423+
out, _ = sjson.SetRawBytes(out, "generationConfig.responseJsonSchema", []byte(schemaStr))
424424
}
425425
}
426426
}
@@ -514,14 +514,61 @@ func openAIResponsesGeminiThoughtSignature(rawSignature string) string {
514514
func normalizeSchemaForGemini(schema interface{}) interface{} {
515515
switch v := schema.(type) {
516516
case map[string]interface{}:
517+
// If allOf exists, merge it first to avoid non-deterministic overwrites during map iteration
518+
if allOf, exists := v["allOf"]; exists {
519+
if allOfArray, ok := allOf.([]interface{}); ok {
520+
merged := mergeAllOf(allOfArray)
521+
for k, val := range v {
522+
if k == "allOf" {
523+
continue
524+
}
525+
if k == "properties" {
526+
mergedProps, _ := merged["properties"].(map[string]interface{})
527+
if mergedProps == nil {
528+
mergedProps = make(map[string]interface{})
529+
} else {
530+
// Copy to avoid mutating original
531+
newProps := make(map[string]interface{})
532+
for pk, pv := range mergedProps {
533+
newProps[pk] = pv
534+
}
535+
mergedProps = newProps
536+
}
537+
if vProps, ok := val.(map[string]interface{}); ok {
538+
for pk, pv := range vProps {
539+
mergedProps[pk] = pv
540+
}
541+
}
542+
merged["properties"] = mergedProps
543+
} else if k == "required" {
544+
var mergedReq []interface{}
545+
if mr, ok := merged["required"].([]interface{}); ok {
546+
mergedReq = append(mergedReq, mr...)
547+
}
548+
if vr, ok := val.([]interface{}); ok {
549+
mergedReq = append(mergedReq, vr...)
550+
}
551+
merged["required"] = mergedReq
552+
} else {
553+
merged[k] = val
554+
}
555+
}
556+
v = merged
557+
}
558+
}
559+
517560
result := make(map[string]interface{})
518561

519562
for key, val := range v {
520563
switch key {
521-
case "additionalProperties":
522-
// Remove this - Gemini rejects it in some contexts
564+
case "allOf":
565+
// Already merged pre-iteration
523566
continue
524567

568+
case "additionalProperties":
569+
// Preserve additionalProperties - Gemini JSON Schema mode supports it
570+
result[key] = val
571+
525572
case "type":
526573
// Handle nullable types: convert ["string", "null"] to "string" + "nullable": true
527574
if typeArray, ok := val.([]interface{}); ok {
@@ -587,16 +634,8 @@ func normalizeSchemaForGemini(schema interface{}) interface{} {
587634
}
588635

589636
case "allOf":
590-
// Merge all schemas in allOf into a single schema
591-
if allOfArray, ok := val.([]interface{}); ok {
592-
mergedSchema := mergeAllOf(allOfArray)
593-
normalized := normalizeSchemaForGemini(mergedSchema)
594-
if normalizedMap, ok := normalized.(map[string]interface{}); ok {
595-
for k, v := range normalizedMap {
596-
result[k] = v
597-
}
598-
}
599-
}
637+
// Already merged pre-iteration
638+
continue
600639

601640
case "oneOf":
602641
// Convert oneOf to anyOf
@@ -678,24 +717,20 @@ func mergeAllOf(schemas []interface{}) map[string]interface{} {
678717

679718
for _, schema := range schemas {
680719
if schemaMap, ok := schema.(map[string]interface{}); ok {
681-
// Merge type
682-
if typ, exists := schemaMap["type"]; exists {
683-
merged["type"] = typ
684-
}
685-
686-
// Merge properties
687-
if props, exists := schemaMap["properties"]; exists {
688-
if propsMap, ok := props.(map[string]interface{}); ok {
689-
for k, v := range propsMap {
690-
properties[k] = v
720+
for k, v := range schemaMap {
721+
switch k {
722+
case "properties":
723+
if propsMap, ok := v.(map[string]interface{}); ok {
724+
for pk, pv := range propsMap {
725+
properties[pk] = pv
726+
}
691727
}
692-
}
693-
}
694-
695-
// Merge required fields
696-
if req, exists := schemaMap["required"]; exists {
697-
if reqArray, ok := req.([]interface{}); ok {
698-
required = append(required, reqArray...)
728+
case "required":
729+
if reqArray, ok := v.([]interface{}); ok {
730+
required = append(required, reqArray...)
731+
}
732+
default:
733+
merged[k] = v
699734
}
700735
}
701736
}
@@ -730,15 +765,18 @@ func mergeAllOf(schemas []interface{}) map[string]interface{} {
730765
//
731766
// The function handles the complete conversion from OpenAI's JSON Schema format to Gemini's
732767
// supported subset, including:
733-
// - Removing unsupported keywords (additionalProperties, pattern, minLength, multipleOf, etc.)
768+
// - Removing unsupported keywords (pattern, minLength, multipleOf, etc.)
734769
// - Preserving nullable type arrays ["string", "null"]
770+
// - Preserving additionalProperties constraints
735771
// - Filtering format values to only date-time, date, time
736772
// - Recursively processing all nested schemas
737773
//
738774
// If parsing fails, the original schema is returned unchanged.
739775
func normalizeJSONSchema(schemaJSON string) string {
740776
var schema interface{}
741-
if err := json.Unmarshal([]byte(schemaJSON), &schema); err != nil {
777+
decoder := json.NewDecoder(strings.NewReader(schemaJSON))
778+
decoder.UseNumber()
779+
if err := decoder.Decode(&schema); err != nil {
742780
return schemaJSON
743781
}
744782

0 commit comments

Comments
 (0)