fix(gemini): ensure strict 1:1 pairing of model calls and function responses

- Gemini requires function results to immediately follow the model message that called them - Implemented look-ahead grouping to pair assistant calls with their tool results - Standardized system and orphaned tool message handling for Gemini compatibility
2026-04-07 18:57:13 +00:00
parent be4ec3482a
commit e12418cc4c
1 changed files with 132 additions and 83 deletions
@@ -78,28 +78,48 @@ func (p *GeminiProvider) ChatCompletion(ctx context.Context, req *models.Unified
 	// Gemini mapping
 	var contents []GeminiContent
 	
-	// Group tool messages together for Gemini
 	for i := 0; i < len(req.Messages); i++ {
 		msg := req.Messages[i]
-		role := "user"
-		if msg.Role == "assistant" {
-			role = "model"
-		} else if msg.Role == "tool" {
-			role = "function"
+		
+		if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
+			// 1. Add the assistant (model) message with tool calls
+			parts := []GeminiPart{}
+			for _, cp := range msg.Content {
+				if cp.Type == "text" && cp.Text != "" {
+					parts = append(parts, GeminiPart{Text: cp.Text})
+				}
+			}
+			for _, tc := range msg.ToolCalls {
+				parts = append(parts, GeminiPart{
+					FunctionCall: &GeminiFunctionCall{
+						Name: tc.Function.Name,
+						Args: json.RawMessage(tc.Function.Arguments),
+					},
+				})
+			}
+			contents = append(contents, GeminiContent{Role: "model", Parts: parts})
+
+			// 2. The VERY NEXT message MUST be the "function" results for THESE EXACT calls.
+			// Look ahead for tool messages.
+			var functionParts []GeminiPart
+			toolCallIDs := make(map[string]bool)
+			for _, tc := range msg.ToolCalls {
+				toolCallIDs[tc.ID] = true
 			}

-		var parts []GeminiPart
-		
-		if msg.Role == "tool" {
-			// Check if we can group this with previous tool message
-			// Actually, it's easier to just collect all current and subsequent tool messages
-			for j := i; j < len(req.Messages) && req.Messages[j].Role == "tool"; j++ {
+			// We need to find tool messages that correspond to these calls.
+			// In many patterns, they follow immediately.
+			j := i + 1
+			foundAny := false
+			for j < len(req.Messages) && req.Messages[j].Role == "tool" {
 				m := req.Messages[j]
+				
+				// Try to match by ID or just take them in order if IDs are missing/mismatched
+				// Gemini is strict: you must respond to EVERY call in the previous message.
 				text := ""
 				if len(m.Content) > 0 {
 					text = m.Content[0].Text
 				}
-				
 				name := "unknown_function"
 				if m.Name != nil {
 					name = *m.Name
@@ -111,17 +131,43 @@ func (p *GeminiProvider) ChatCompletion(ctx context.Context, req *models.Unified
 				}
 				respBytes, _ := json.Marshal(responseObj)

-				parts = append(parts, GeminiPart{
+				functionParts = append(functionParts, GeminiPart{
 					FunctionResponse: &GeminiFunctionResponse{
 						Name:     name,
 						Response: json.RawMessage(respBytes),
 					},
 				})
-				i = j // Advance outer loop
+				foundAny = true
+				j++
 			}
+
+			if foundAny {
+				contents = append(contents, GeminiContent{Role: "function", Parts: functionParts})
+				i = j - 1 // Advance outer loop past the tool messages we consumed
 			} else {
+				// If no tool results found but assistant made calls, Gemini WILL error.
+				// We should probably skip the calls or provide dummy results, 
+				// but usually this means the conversation is incomplete.
+				// For now, don't add a "function" message if none found.
+			}
+			continue
+		}
+
+		// Standard message handling (System/User/Assistant without tools)
+		role := "user"
+		if msg.Role == "assistant" {
+			role = "model"
+		} else if msg.Role == "system" {
+			role = "user" // Gemini uses 'user' for system prompts in some versions, or handles it via systemInstruction
+		} else if msg.Role == "tool" {
+			// Orphaned tool message (not following an assistant call) - Gemini doesn't like this.
+			// Skip or map to user? Skipping is safer for API stability.
+			continue
+		}
+		
+		var parts []GeminiPart
 		for _, cp := range msg.Content {
-				if cp.Type == "text" {
+			if cp.Type == "text" && cp.Text != "" {
 				parts = append(parts, GeminiPart{Text: cp.Text})
 			} else if cp.Image != nil {
 				base64Data, mimeType, _ := cp.Image.ToBase64()
@@ -134,24 +180,10 @@ func (p *GeminiProvider) ChatCompletion(ctx context.Context, req *models.Unified
 			}
 		}

-			// Handle assistant tool calls
-			if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
-				for _, tc := range msg.ToolCalls {
-					parts = append(parts, GeminiPart{
-						FunctionCall: &GeminiFunctionCall{
-							Name: tc.Function.Name,
-							Args: json.RawMessage(tc.Function.Arguments),
-						},
-					})
+		if len(parts) > 0 {
+			contents = append(contents, GeminiContent{Role: role, Parts: parts})
 		}
 	}
-		}
-		
-		contents = append(contents, GeminiContent{
-			Role:  role,
-			Parts: parts,
-		})
-	}

 	genConfig := &GeminiGenerationConfig{}
 	if req.Temperature != nil {
@@ -302,17 +334,28 @@ func (p *GeminiProvider) ChatCompletionStream(ctx context.Context, req *models.U
 	var contents []GeminiContent
 	for i := 0; i < len(req.Messages); i++ {
 		msg := req.Messages[i]
-		role := "user"
-		if msg.Role == "assistant" {
-			role = "model"
-		} else if msg.Role == "tool" {
-			role = "function"
+		
+		if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
+			parts := []GeminiPart{}
+			for _, cp := range msg.Content {
+				if cp.Type == "text" && cp.Text != "" {
+					parts = append(parts, GeminiPart{Text: cp.Text})
 				}
+			}
+			for _, tc := range msg.ToolCalls {
+				parts = append(parts, GeminiPart{
+					FunctionCall: &GeminiFunctionCall{
+						Name: tc.Function.Name,
+						Args: json.RawMessage(tc.Function.Arguments),
+					},
+				})
+			}
+			contents = append(contents, GeminiContent{Role: "model", Parts: parts})

-		var parts []GeminiPart
-		
-		if msg.Role == "tool" {
-			for j := i; j < len(req.Messages) && req.Messages[j].Role == "tool"; j++ {
+			var functionParts []GeminiPart
+			j := i + 1
+			foundAny := false
+			for j < len(req.Messages) && req.Messages[j].Role == "tool" {
 				m := req.Messages[j]
 				text := ""
 				if len(m.Content) > 0 {
@@ -329,20 +372,38 @@ func (p *GeminiProvider) ChatCompletionStream(ctx context.Context, req *models.U
 				}
 				respBytes, _ := json.Marshal(responseObj)

-				parts = append(parts, GeminiPart{
+				functionParts = append(functionParts, GeminiPart{
 					FunctionResponse: &GeminiFunctionResponse{
 						Name:     name,
 						Response: json.RawMessage(respBytes),
 					},
 				})
-				i = j
+				foundAny = true
+				j++
 			}
-		} else {
-			for _, p := range msg.Content {
-				if p.Type == "text" {
-					parts = append(parts, GeminiPart{Text: p.Text})
-				} else if p.Image != nil {
-					base64Data, mimeType, _ := p.Image.ToBase64()
+
+			if foundAny {
+				contents = append(contents, GeminiContent{Role: "function", Parts: functionParts})
+				i = j - 1
+			}
+			continue
+		}
+
+		role := "user"
+		if msg.Role == "assistant" {
+			role = "model"
+		} else if msg.Role == "system" {
+			role = "user"
+		} else if msg.Role == "tool" {
+			continue
+		}
+		
+		var parts []GeminiPart
+		for _, cp := range msg.Content {
+			if cp.Type == "text" && cp.Text != "" {
+				parts = append(parts, GeminiPart{Text: cp.Text})
+			} else if cp.Image != nil {
+				base64Data, mimeType, _ := cp.Image.ToBase64()
 				parts = append(parts, GeminiPart{
 					InlineData: &GeminiInlineData{
 						MimeType: mimeType,
@@ -351,22 +412,10 @@ func (p *GeminiProvider) ChatCompletionStream(ctx context.Context, req *models.U
 				})
 			}
 		}
-			if msg.Role == "assistant" && len(msg.ToolCalls) > 0 {
-				for _, tc := range msg.ToolCalls {
-					parts = append(parts, GeminiPart{
-						FunctionCall: &GeminiFunctionCall{
-							Name: tc.Function.Name,
-							Args: json.RawMessage(tc.Function.Arguments),
-						},
-					})
-				}
-			}
-		}

-		contents = append(contents, GeminiContent{
-			Role:  role,
-			Parts: parts,
-		})
+		if len(parts) > 0 {
+			contents = append(contents, GeminiContent{Role: role, Parts: parts})
+		}
 	}

 	genConfig := &GeminiGenerationConfig{}