fix(go): send Gemini system prompts via system_instruction instead of user turn (#17449)

2026-08-01 13:33:48 +08:00 · 2026-07-31 16:10:11 +05:30
parent fe02c9b95f
commit c8fbd97595
2 changed files with 207 additions and 31 deletions
--- a/internal/entity/models/google.go
+++ b/internal/entity/models/google.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"ragflow/internal/common"
 	"strings"

@@ -120,12 +121,33 @@ func (g *GoogleModel) baseURL(apiConfig *APIConfig) string {
 	return strings.TrimSpace(baseURL)
 }

+func googleSystemInstruction(messages []Message) (*genai.Content, error) {
+	var parts []*genai.Part
+	for _, msg := range messages {
+		if msg.Role != "system" {
+			continue
+		}
+		for _, part := range googleMessageParts(msg.Content) {
+			if part.FileData != nil {
+				return nil, fmt.Errorf("gemini: system message must be text only, got image content")
+			}
+			parts = append(parts, part)
+		}
+	}
+	if len(parts) == 0 {
+		return nil, nil
+	}
+	return &genai.Content{Parts: parts}, nil
+}
+
 func googleChatContents(messages []Message) []*genai.Content {
 	var contents []*genai.Content
 	toolCallNames := make(map[string]string)

 	for _, msg := range messages {
 		switch msg.Role {
+		case "system":
+			continue
 		case "tool":
 			name := toolCallNames[msg.ToolCallID]
 			if name == "" {
@@ -230,37 +252,38 @@ func googleFunctionResponse(content interface{}) map[string]any {
 	}
 }

-func googleGenerateContentConfig(chatModelConfig *ChatConfig) *genai.GenerateContentConfig {
-	if chatModelConfig == nil {
-		return nil
-	}
-
-	cfg := &genai.GenerateContentConfig{}
-	if chatModelConfig.Temperature != nil {
-		value := float32(*chatModelConfig.Temperature)
-		cfg.Temperature = &value
-	}
-	if chatModelConfig.TopP != nil {
-		value := float32(*chatModelConfig.TopP)
-		cfg.TopP = &value
-	}
-	if chatModelConfig.MaxTokens != nil {
-		cfg.MaxOutputTokens = int32(*chatModelConfig.MaxTokens)
-	}
-	if chatModelConfig.Stop != nil {
-		cfg.StopSequences = *chatModelConfig.Stop
-	}
-	if tools := googleTools(chatModelConfig.Tools); len(tools) > 0 {
-		cfg.Tools = tools
-		cfg.ToolConfig = &genai.ToolConfig{
-			FunctionCallingConfig: &genai.FunctionCallingConfig{Mode: googleFunctionCallingMode(chatModelConfig.ToolChoice)},
+func googleGenerateContentConfig(chatModelConfig *ChatConfig, systemInstruction *genai.Content) (*genai.GenerateContentConfig, error) {
+	cfg := &genai.GenerateContentConfig{SystemInstruction: systemInstruction}
+	if chatModelConfig != nil {
+		if chatModelConfig.Temperature != nil {
+			value := float32(*chatModelConfig.Temperature)
+			cfg.Temperature = &value
+		}
+		if chatModelConfig.TopP != nil {
+			value := float32(*chatModelConfig.TopP)
+			cfg.TopP = &value
+		}
+		if chatModelConfig.MaxTokens != nil {
+			if *chatModelConfig.MaxTokens < 0 || *chatModelConfig.MaxTokens > math.MaxInt32 {
+				return nil, fmt.Errorf("gemini: max_tokens %d is out of range for int32", *chatModelConfig.MaxTokens)
+			}
+			cfg.MaxOutputTokens = int32(*chatModelConfig.MaxTokens)
+		}
+		if chatModelConfig.Stop != nil {
+			cfg.StopSequences = *chatModelConfig.Stop
+		}
+		if tools := googleTools(chatModelConfig.Tools); len(tools) > 0 {
+			cfg.Tools = tools
+			cfg.ToolConfig = &genai.ToolConfig{
+				FunctionCallingConfig: &genai.FunctionCallingConfig{Mode: googleFunctionCallingMode(chatModelConfig.ToolChoice)},
+			}
 		}
 	}

-	if cfg.Temperature == nil && cfg.TopP == nil && cfg.MaxOutputTokens == 0 && len(cfg.StopSequences) == 0 && len(cfg.Tools) == 0 {
-		return nil
+	if cfg.SystemInstruction == nil && cfg.Temperature == nil && cfg.TopP == nil && cfg.MaxOutputTokens == 0 && len(cfg.StopSequences) == 0 && len(cfg.Tools) == 0 {
+		return nil, nil
 	}
-	return cfg
+	return cfg, nil
 }

 func googleFunctionCallingMode(toolChoice *string) genai.FunctionCallingConfigMode {
@@ -372,9 +395,20 @@ func (g *GoogleModel) ChatWithMessages(ctx context.Context, modelName string, me
 	}

 	contents := googleChatContents(messages)
+	if len(contents) == 0 {
+		return nil, fmt.Errorf("gemini: no conversational message after excluding system messages")
+	}
+	systemInstruction, err := googleSystemInstruction(messages)
+	if err != nil {
+		return nil, err
+	}
+	generateContentConfig, err := googleGenerateContentConfig(chatModelConfig, systemInstruction)
+	if err != nil {
+		return nil, err
+	}

 	// Generate content (non-streaming)
-	response, err := client.Models.GenerateContent(ctx, modelName, contents, googleGenerateContentConfig(chatModelConfig))
+	response, err := client.Models.GenerateContent(ctx, modelName, contents, generateContentConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -407,13 +441,24 @@ func (g *GoogleModel) ChatStreamlyWithSender(ctx context.Context, modelName stri
 	}

 	contents := googleChatContents(messages)
+	if len(contents) == 0 {
+		return fmt.Errorf("gemini: no conversational message after excluding system messages")
+	}
+	systemInstruction, err := googleSystemInstruction(messages)
+	if err != nil {
+		return err
+	}
+	generateContentConfig, err := googleGenerateContentConfig(chatModelConfig, systemInstruction)
+	if err != nil {
+		return err
+	}
 	var toolCalls []map[string]interface{}

 	for response, err := range client.Models.GenerateContentStream(
 		ctx,
 		modelName,
 		contents,
-		googleGenerateContentConfig(chatModelConfig),
+		generateContentConfig,
 	) {
 		if err != nil {
 			return err
--- a/internal/entity/models/google_test.go
+++ b/internal/entity/models/google_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"math"
 	"reflect"
 	"strings"
 	"sync"
@@ -265,6 +266,35 @@ func TestGoogleModelChatRequiresModelName(t *testing.T) {
 	}
 }

+func TestGoogleModelChatRequiresConversationalMessage(t *testing.T) {
+	ctx := t.Context()
+	model := &GoogleModel{}
+	apiKey := "test-api-key"
+	messages := []Message{{Role: "system", Content: "You are a helpful assistant."}}
+
+	response, err := model.ChatWithMessages(ctx, "gemini-2.5-flash", messages, &APIConfig{ApiKey: &apiKey}, nil, nil)
+	if err == nil {
+		t.Fatal("expected an error for system-only messages")
+	}
+	if !strings.Contains(err.Error(), "no conversational message") {
+		t.Fatalf("expected no-conversational-message error, got %v", err)
+	}
+	if response != nil {
+		t.Fatalf("expected no response, got %v", response)
+	}
+
+	err = model.ChatStreamlyWithSender(ctx, "gemini-2.5-flash", messages, &APIConfig{ApiKey: &apiKey}, nil, nil, func(*string, *string) error {
+		t.Errorf("sender should not be called for system-only messages")
+		return nil
+	})
+	if err == nil {
+		t.Fatal("expected an error for system-only messages")
+	}
+	if !strings.Contains(err.Error(), "no conversational message") {
+		t.Fatalf("expected no-conversational-message error, got %v", err)
+	}
+}
+
 func TestGoogleModelNewInstancePreservesCustomBaseURL(t *testing.T) {
 	model := NewGoogleModel(map[string]string{"default": "https://generativelanguage.googleapis.com"}, URLSuffix{Models: "v1beta/models"})
 	customBaseURL := map[string]string{"default": "https://example.test/google"}
@@ -394,7 +424,7 @@ func TestCollectGoogleModelNamesReturnsPageError(t *testing.T) {

 func TestGoogleGenerateContentConfigConvertsTools(t *testing.T) {
 	toolChoice := "required"
-	cfg := googleGenerateContentConfig(&ChatConfig{
+	cfg, err := googleGenerateContentConfig(&ChatConfig{
 		Tools: []map[string]interface{}{{
 			"type": "function",
 			"function": map[string]interface{}{
@@ -410,7 +440,10 @@ func TestGoogleGenerateContentConfigConvertsTools(t *testing.T) {
 			},
 		}},
 		ToolChoice: &toolChoice,
-	})
+	}, nil)
+	if err != nil {
+		t.Fatalf("googleGenerateContentConfig error = %v", err)
+	}
 	if cfg == nil || len(cfg.Tools) != 1 || len(cfg.Tools[0].FunctionDeclarations) != 1 {
 		t.Fatalf("tools = %#v, want one function declaration", cfg)
 	}
@@ -429,6 +462,26 @@ func TestGoogleGenerateContentConfigConvertsTools(t *testing.T) {
 	}
 }

+func TestGoogleGenerateContentConfigRejectsMaxTokensOverflow(t *testing.T) {
+	overflow := int(math.MaxInt32) + 1
+	cfg, err := googleGenerateContentConfig(&ChatConfig{MaxTokens: &overflow}, nil)
+	if err == nil {
+		t.Fatalf("expected an error for max_tokens overflowing int32, got cfg = %#v", cfg)
+	}
+	if cfg != nil {
+		t.Fatalf("cfg = %#v, want nil on error", cfg)
+	}
+
+	maxInt32 := int(math.MaxInt32)
+	cfg, err = googleGenerateContentConfig(&ChatConfig{MaxTokens: &maxInt32}, nil)
+	if err != nil {
+		t.Fatalf("googleGenerateContentConfig error = %v", err)
+	}
+	if cfg == nil || cfg.MaxOutputTokens != math.MaxInt32 {
+		t.Fatalf("cfg.MaxOutputTokens = %#v, want %d", cfg, int32(math.MaxInt32))
+	}
+}
+
 func TestGoogleChatContentsConvertsToolHistory(t *testing.T) {
 	contents := googleChatContents([]Message{
 		{
@@ -464,6 +517,84 @@ func TestGoogleChatContentsConvertsToolHistory(t *testing.T) {
 	}
 }

+func TestGoogleSystemInstructionExtractedFromMessages(t *testing.T) {
+	messages := []Message{
+		{Role: "system", Content: "You are a helpful assistant."},
+		{Role: "user", Content: "Hello"},
+		{Role: "system", Content: "Always answer in pirate speak."},
+	}
+
+	contents := googleChatContents(messages)
+	if len(contents) != 1 {
+		t.Fatalf("contents len = %d, want 1 (both system messages must be excluded)", len(contents))
+	}
+	if contents[0].Role != genai.RoleUser {
+		t.Fatalf("contents[0].Role = %s, want user", contents[0].Role)
+	}
+
+	systemInstruction, err := googleSystemInstruction(messages)
+	if err != nil {
+		t.Fatalf("googleSystemInstruction error = %v", err)
+	}
+	if systemInstruction == nil || len(systemInstruction.Parts) != 2 {
+		t.Fatalf("systemInstruction = %#v, want two parts", systemInstruction)
+	}
+	if systemInstruction.Parts[0].Text != "You are a helpful assistant." {
+		t.Fatalf("systemInstruction first part text = %q", systemInstruction.Parts[0].Text)
+	}
+	if systemInstruction.Parts[1].Text != "Always answer in pirate speak." {
+		t.Fatalf("systemInstruction second part text = %q", systemInstruction.Parts[1].Text)
+	}
+
+	cfg, err := googleGenerateContentConfig(nil, systemInstruction)
+	if err != nil {
+		t.Fatalf("googleGenerateContentConfig error = %v", err)
+	}
+	if cfg == nil || cfg.SystemInstruction != systemInstruction {
+		t.Fatalf("cfg.SystemInstruction = %#v, want %#v", cfg, systemInstruction)
+	}
+}
+
+func TestGoogleSystemInstructionNilWhenNoSystemMessage(t *testing.T) {
+	got, err := googleSystemInstruction([]Message{{Role: "user", Content: "Hello"}})
+	if err != nil {
+		t.Fatalf("googleSystemInstruction error = %v", err)
+	}
+	if got != nil {
+		t.Fatalf("systemInstruction = %#v, want nil", got)
+	}
+	cfg, err := googleGenerateContentConfig(nil, nil)
+	if err != nil {
+		t.Fatalf("googleGenerateContentConfig error = %v", err)
+	}
+	if cfg != nil {
+		t.Fatalf("cfg = %#v, want nil", cfg)
+	}
+}
+
+func TestGoogleSystemInstructionRejectsImageContent(t *testing.T) {
+	messages := []Message{
+		{
+			Role: "system",
+			Content: []interface{}{
+				map[string]interface{}{
+					"type":      "image_url",
+					"image_url": map[string]interface{}{"url": "https://example.com/cat.png"},
+				},
+			},
+		},
+		{Role: "user", Content: "Hello"},
+	}
+
+	systemInstruction, err := googleSystemInstruction(messages)
+	if err == nil {
+		t.Fatalf("googleSystemInstruction error = nil, want error for image content in system message")
+	}
+	if systemInstruction != nil {
+		t.Fatalf("systemInstruction = %#v, want nil on error", systemInstruction)
+	}
+}
+
 func TestGoogleToolCallsConvertsFunctionCalls(t *testing.T) {
 	toolCalls := googleToolCalls([]*genai.FunctionCall{{
 		ID:   "call-1",