diff --git a/internal/entity/models/google.go b/internal/entity/models/google.go index dc2bb99baa..dc74fbc618 100644 --- 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 diff --git a/internal/entity/models/google_test.go b/internal/entity/models/google_test.go index 9ad8552837..2a4af41e37 100644 --- 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",