From 19cee9af112fb42a2de75544a4ac61d39e858ae1 Mon Sep 17 00:00:00 2001 From: Rogee Date: Mon, 13 Jul 2026 00:50:32 +0800 Subject: [PATCH] fix(copilot): align runtime configuration contracts --- .../api/v1/copilot_config_handler_test.go | 3 + backend/internal/llm/provider_manager.go | 20 +++ backend/internal/llm/provider_manager_test.go | 90 +++++++++++ backend/internal/model/captain_models.go | 5 + .../service/auto_reply_rule_service.go | 3 +- .../service/captain_assistant_service.go | 9 ++ .../service/captain_conversation_service.go | 3 +- .../service/copilot_config_service.go | 2 +- .../copilot_feature_model_integration_test.go | 150 ++++++++++++++++++ backend/internal/service/rag_service.go | 5 +- .../components/ProviderConfiguration.spec.js | 2 +- .../components/ProviderConfiguration.vue | 4 +- 12 files changed, 288 insertions(+), 8 deletions(-) create mode 100644 backend/internal/service/copilot_feature_model_integration_test.go diff --git a/backend/internal/handler/api/v1/copilot_config_handler_test.go b/backend/internal/handler/api/v1/copilot_config_handler_test.go index 482bc8e4..51e02011 100644 --- a/backend/internal/handler/api/v1/copilot_config_handler_test.go +++ b/backend/internal/handler/api/v1/copilot_config_handler_test.go @@ -122,6 +122,8 @@ func TestCopilotConfigHandlerPlatformPermissionsAndSecretPresentation(t *testing require.Equal(t, http.StatusOK, updated.Code, updated.Body.String()) require.NotContains(t, updated.Body.String(), "plain-secret-key") require.Contains(t, updated.Body.String(), "pla****-key") + require.Contains(t, updated.Body.String(), `"masked_value":"pla****-key"`) + require.NotContains(t, updated.Body.String(), `"masked":`) var stored model.InstallationConfig require.NoError(t, f.db.Where("name = ?", "COPILOT_CHAT_API_KEY").First(&stored).Error) @@ -140,6 +142,7 @@ func TestCopilotConfigHandlerPlatformPermissionsAndSecretPresentation(t *testing require.Equal(t, http.StatusOK, accountPayload.Code, accountPayload.Body.String()) require.NotContains(t, accountPayload.Body.String(), "plain-secret-key") require.NotContains(t, accountPayload.Body.String(), "pla****-key") + require.NotContains(t, accountPayload.Body.String(), "masked_value") require.Contains(t, accountPayload.Body.String(), `"configured":true`) agentPayload := f.request(http.MethodGet, "/forbidden"+accountPath, nil) diff --git a/backend/internal/llm/provider_manager.go b/backend/internal/llm/provider_manager.go index 8d930ad2..5b64fd84 100644 --- a/backend/internal/llm/provider_manager.go +++ b/backend/internal/llm/provider_manager.go @@ -48,6 +48,12 @@ type accountFeatureContext struct { Feature string } +type generationOverrideContextKey struct{} + +type generationOverrideContext struct { + Temperature *float64 +} + // WithAccountFeature annotates an LLM request so ProviderManager can resolve // the account-specific model selected on the Copilot settings page. func WithAccountFeature(ctx context.Context, accountID uint, feature string) context.Context { @@ -60,6 +66,17 @@ func WithAccountFeature(ctx context.Context, accountID uint, feature string) con }) } +// WithTemperatureOverride preserves an explicitly configured Assistant +// temperature while keeping the platform value as the default elsewhere. +func WithTemperatureOverride(ctx context.Context, temperature float64) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, generationOverrideContextKey{}, generationOverrideContext{ + Temperature: &temperature, + }) +} + type providerSnapshot struct { chat Provider embedding Provider @@ -253,6 +270,9 @@ func resolveFeatureModel(ctx context.Context, resolver AccountModelResolver, fal func applyRuntimeChatConfig(ctx context.Context, req ChatRequest, cfg RuntimeProviderConfig, resolver AccountModelResolver) ChatRequest { req.Model = resolveFeatureModel(ctx, resolver, cfg.ChatModel) req.Temperature = cfg.Temperature + if override, ok := ctx.Value(generationOverrideContextKey{}).(generationOverrideContext); ok && override.Temperature != nil { + req.Temperature = *override.Temperature + } req.MaxTokens = cfg.MaxTokens return req } diff --git a/backend/internal/llm/provider_manager_test.go b/backend/internal/llm/provider_manager_test.go index 7acdfd7a..3544de84 100644 --- a/backend/internal/llm/provider_manager_test.go +++ b/backend/internal/llm/provider_manager_test.go @@ -51,6 +51,33 @@ func TestProviderManagerUsesAccountFeatureModelAndGenerationSettings(t *testing. assert.Equal(t, 777, request.MaxTokens) } +func TestProviderManagerPreservesExplicitAssistantTemperature(t *testing.T) { + var request ChatRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"chat-1","choices":[{"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`)) + })) + defer server.Close() + + manager := NewProviderManager() + require.NoError(t, manager.Configure(RuntimeProviderConfig{ + ChatProvider: "openai_compatible", + ChatBaseURL: server.URL, + ChatAPIKey: "test-key", + ChatModel: "platform-model", + EmbeddingMode: EmbeddingModeReuseChat, + Temperature: 0.9, + MaxTokens: 777, + })) + + ctx := WithTemperatureOverride(context.Background(), 0.2) + _, err := manager.ChatCompletion(ctx, ChatRequest{}) + require.NoError(t, err) + assert.Equal(t, 0.2, request.Temperature) + assert.Equal(t, 777, request.MaxTokens) +} + func TestProviderManagerUsesSeparateEmbeddingProvider(t *testing.T) { var chatCalls atomic.Int32 chatServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -91,6 +118,69 @@ func TestProviderManagerUsesSeparateEmbeddingProvider(t *testing.T) { assert.Equal(t, 3, embeddingRequest.Dimensions) } +func TestProviderManagerUsesAnthropicChatWithSeparateCompatibleEmbedding(t *testing.T) { + var ( + anthropicRequest anthropicRequest + embeddingRequest EmbeddingRequest + anthropicKey string + embeddingAuth string + ) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/v1/messages": + anthropicKey = r.Header.Get("x-api-key") + require.NoError(t, json.NewDecoder(r.Body).Decode(&anthropicRequest)) + _, _ = w.Write([]byte(`{"id":"msg-1","type":"message","role":"assistant","content":[{"type":"text","text":"anthropic ok"}],"model":"claude-test","stop_reason":"end_turn","usage":{"input_tokens":2,"output_tokens":3}}`)) + case "/embeddings": + embeddingAuth = r.Header.Get("Authorization") + require.NoError(t, json.NewDecoder(r.Body).Decode(&embeddingRequest)) + _, _ = w.Write([]byte(`{"data":[{"index":0,"embedding":[0.1,0.2,0.3]}],"model":"embed-test"}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + manager := NewProviderManager() + require.NoError(t, manager.Configure(RuntimeProviderConfig{ + ChatProvider: "anthropic", + ChatBaseURL: server.URL, + ChatAPIKey: "anthropic-key", + ChatModel: "claude-test", + EmbeddingMode: EmbeddingModeSeparate, + EmbeddingProvider: "openai_compatible", + EmbeddingBaseURL: server.URL, + EmbeddingAPIKey: "embedding-key", + EmbeddingModel: "embed-test", + EmbeddingDimensions: 3, + Temperature: 0.4, + MaxTokens: 321, + })) + + chatResponse, err := manager.ChatCompletion(context.Background(), ChatRequest{ + Messages: []ChatMessage{ + {Role: "system", Content: "system prompt"}, + {Role: "user", Content: "hello"}, + }, + }) + require.NoError(t, err) + require.Len(t, chatResponse.Choices, 1) + assert.Equal(t, "anthropic ok", chatResponse.Choices[0].Message.Content) + assert.Equal(t, "anthropic-key", anthropicKey) + assert.Equal(t, "claude-test", anthropicRequest.Model) + assert.Equal(t, "system prompt", anthropicRequest.System) + assert.Equal(t, 0.4, anthropicRequest.Temperature) + assert.Equal(t, 321, anthropicRequest.MaxTokens) + + embeddingResponse, err := manager.CreateEmbedding(context.Background(), EmbeddingRequest{Input: []string{"hello"}}) + require.NoError(t, err) + require.Len(t, embeddingResponse.Data, 1) + assert.Equal(t, "Bearer embedding-key", embeddingAuth) + assert.Equal(t, "embed-test", embeddingRequest.Model) + assert.Equal(t, 3, embeddingRequest.Dimensions) +} + func TestProviderManagerExplicitZeroRetriesDoesNotRetry(t *testing.T) { var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { diff --git a/backend/internal/model/captain_models.go b/backend/internal/model/captain_models.go index 10b65a2c..a521aafb 100644 --- a/backend/internal/model/captain_models.go +++ b/backend/internal/model/captain_models.go @@ -103,6 +103,7 @@ func DefaultAssistantConfig() map[string]interface{} { // Reference: Chatwoot store_accessor :config, :temperature, :feature_faq, etc. type AssistantConfig struct { Temperature float64 `json:"temperature,omitempty"` + TemperatureConfigured bool `json:"-"` FeatureFAQ bool `json:"feature_faq,omitempty"` FeatureMemory bool `json:"feature_memory,omitempty"` FeatureContactAttributes bool `json:"feature_contact_attributes,omitempty"` @@ -119,6 +120,10 @@ func (a *CaptainAssistant) GetConfig() (*AssistantConfig, error) { if err := json.Unmarshal(a.Config, &cfg); err != nil { return nil, err } + var fields map[string]json.RawMessage + if err := json.Unmarshal(a.Config, &fields); err == nil { + _, cfg.TemperatureConfigured = fields["temperature"] + } return &cfg, nil } diff --git a/backend/internal/service/auto_reply_rule_service.go b/backend/internal/service/auto_reply_rule_service.go index 52fca360..a64e627d 100644 --- a/backend/internal/service/auto_reply_rule_service.go +++ b/backend/internal/service/auto_reply_rule_service.go @@ -416,6 +416,7 @@ func (s *AutoReplyRuleService) composeLLMReply(ctx context.Context, rule *model. cfg, _ := assistant.GetConfig() ctx = llm.WithAccountFeature(ctx, assistant.AccountID, "assistant") + ctx = withAssistantGenerationConfig(ctx, cfg) // Build system prompt systemPrompt := s.promptBuilder.BuildAssistantPrompt(assistant, cfg) @@ -435,7 +436,7 @@ func (s *AutoReplyRuleService) composeLLMReply(ctx context.Context, rule *model. modelName := cfg.Model temperature := cfg.Temperature - if temperature == 0 { + if temperature == 0 && !cfg.TemperatureConfigured { temperature = 0.7 } diff --git a/backend/internal/service/captain_assistant_service.go b/backend/internal/service/captain_assistant_service.go index 3d03f50a..9f459231 100644 --- a/backend/internal/service/captain_assistant_service.go +++ b/backend/internal/service/captain_assistant_service.go @@ -294,6 +294,7 @@ func (s *CaptainAssistantService) GenerateResponse(ctx context.Context, assistan // Build system prompt from assistant config and response guidelines cfg, _ := assistant.GetConfig() + ctx = withAssistantGenerationConfig(ctx, cfg) systemPrompt := buildSystemPrompt(assistant, cfg) // Build messages for LLM @@ -363,6 +364,7 @@ func (s *CaptainAssistantService) generatePlaygroundLLMResponse(ctx context.Cont } cfg, _ := assistant.GetConfig() + ctx = withAssistantGenerationConfig(ctx, cfg) messages := []llm.ChatMessage{{Role: "system", Content: buildSystemPrompt(assistant, cfg)}} for _, message := range history { if message.Role == "" || message.Content == "" { @@ -387,6 +389,13 @@ func (s *CaptainAssistantService) generatePlaygroundLLMResponse(ctx context.Cont return resp.Choices[0].Message.Content, nil } +func withAssistantGenerationConfig(ctx context.Context, cfg *model.AssistantConfig) context.Context { + if cfg == nil || !cfg.TemperatureConfigured { + return ctx + } + return llm.WithTemperatureOverride(ctx, cfg.Temperature) +} + func (s *CaptainAssistantService) captainV2Enabled(ctx context.Context, accountID uint) bool { flags, err := s.assistantRepo.GetAccountFeatureFlags(ctx, accountID) if err != nil { diff --git a/backend/internal/service/captain_conversation_service.go b/backend/internal/service/captain_conversation_service.go index d61e7d58..33af9e95 100644 --- a/backend/internal/service/captain_conversation_service.go +++ b/backend/internal/service/captain_conversation_service.go @@ -135,6 +135,7 @@ func (s *CaptainConversationService) generateConversationResponse(ctx context.Co // Build system prompt from assistant config (not hardcoded) cfg, _ := assistant.GetConfig() + ctx = withAssistantGenerationConfig(ctx, cfg) systemPrompt := fmt.Sprintf("You are %s, a customer support assistant.", assistant.Name) if cfg.ProductName != "" { systemPrompt += fmt.Sprintf(" You represent the product: %s.", cfg.ProductName) @@ -154,7 +155,7 @@ func (s *CaptainConversationService) generateConversationResponse(ctx context.Co modelName := cfg.Model temperature := cfg.Temperature - if temperature == 0 { + if temperature == 0 && !cfg.TemperatureConfigured { temperature = 0.7 } diff --git a/backend/internal/service/copilot_config_service.go b/backend/internal/service/copilot_config_service.go index 59f5ce1d..c3b9435c 100644 --- a/backend/internal/service/copilot_config_service.go +++ b/backend/internal/service/copilot_config_service.go @@ -96,7 +96,7 @@ type CopilotProviderConfigInput struct { type CopilotSecretPayload struct { Configured bool `json:"configured"` - Masked string `json:"masked,omitempty"` + Masked string `json:"masked_value,omitempty"` } type CopilotChatConfigPayload struct { diff --git a/backend/internal/service/copilot_feature_model_integration_test.go b/backend/internal/service/copilot_feature_model_integration_test.go new file mode 100644 index 00000000..c494c8ee --- /dev/null +++ b/backend/internal/service/copilot_feature_model_integration_test.go @@ -0,0 +1,150 @@ +package service + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/datatypes" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/llm" + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" +) + +func TestCopilotFeatureModelsReachProviderRequests(t *testing.T) { + type capturedRequest struct { + Model string + Temperature float64 + MaxTokens int + } + var ( + mu sync.Mutex + captured []capturedRequest + ) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request llm.ChatRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + mu.Lock() + captured = append(captured, capturedRequest{ + Model: request.Model, + Temperature: request.Temperature, + MaxTokens: request.MaxTokens, + }) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"chat-1","choices":[{"message":{"role":"assistant","content":"billing,vip"},"finish_reason":"stop"}]}`)) + })) + defer server.Close() + + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate( + &model.Account{}, + &model.CaptainAssistant{}, + &model.Conversation{}, + &model.Message{}, + )) + t.Cleanup(func() { + sqlDB, dbErr := db.DB() + require.NoError(t, dbErr) + require.NoError(t, sqlDB.Close()) + }) + + account := &model.Account{ + Name: "Feature Models", + CaptainModels: datatypes.JSON([]byte(`{ + "editor":"editor-model", + "copilot":"copilot-model", + "assistant":"assistant-model", + "label_suggestion":"label-model" + }`)), + } + require.NoError(t, db.Create(account).Error) + assistant := &model.CaptainAssistant{ + AccountID: account.ID, + Name: "Feature Assistant", + Status: model.AssistantStatusActive, + Config: json.RawMessage(`{"temperature":0.2}`), + } + require.NoError(t, db.Create(assistant).Error) + conversation := &model.Conversation{AccountID: account.ID, Status: "open"} + require.NoError(t, db.Create(conversation).Error) + require.NoError(t, db.Create(&model.Message{ + AccountID: account.ID, + ConversationID: conversation.ID, + MessageType: string(model.MessageTypeIncoming), + SenderType: "contact", + Content: "I need billing help", + }).Error) + + accountRepo := repository.NewAccountRepo(db) + manager := llm.NewProviderManager() + manager.SetAccountModelResolver(func(ctx context.Context, accountID uint, feature string) (string, error) { + current, findErr := accountRepo.FindByID(ctx, accountID) + if findErr != nil { + return "", findErr + } + models := map[string]string{} + require.NoError(t, json.Unmarshal(current.CaptainModels, &models)) + return models[feature], nil + }) + require.NoError(t, manager.Configure(llm.RuntimeProviderConfig{ + ChatProvider: "openai_compatible", + ChatBaseURL: server.URL, + ChatAPIKey: "test-key", + ChatModel: "platform-model", + EmbeddingMode: llm.EmbeddingModeReuseChat, + Temperature: 0.9, + MaxTokens: 777, + })) + + editorService := NewCaptainTaskService(nil, nil, nil, nil, nil, manager, nil) + _, err = editorService.Rewrite(context.Background(), account.ID, &TaskRewriteRequest{Content: "draft", Operation: "improve"}) + require.NoError(t, err) + + copilotService := NewCopilotService(nil, nil, nil, manager) + _, err = copilotService.SummarizeConversation(context.Background(), account.ID, "customer conversation") + require.NoError(t, err) + + assistantService := NewCaptainAssistantService(repository.NewCaptainAssistantRepo(db), nil, nil, nil, manager) + _, err = assistantService.GenerateResponse(context.Background(), assistant.ID, "help me") + require.NoError(t, err) + + labelService := NewCaptainTaskExtendedService( + repository.NewConversationRepo(db), + repository.NewMessageRepo(db), + nil, + nil, + manager, + ) + _, err = labelService.LabelSuggestion(context.Background(), account.ID, &ChatwootLabelSuggestionRequest{ + ConversationDisplayID: conversation.ID, + }) + require.NoError(t, err) + + mu.Lock() + defer mu.Unlock() + require.Len(t, captured, 4) + assert.Equal(t, []string{"editor-model", "copilot-model", "assistant-model", "label-model"}, []string{ + captured[0].Model, + captured[1].Model, + captured[2].Model, + captured[3].Model, + }) + assert.Equal(t, 0.9, captured[0].Temperature) + assert.Equal(t, 0.9, captured[1].Temperature) + assert.Equal(t, 0.2, captured[2].Temperature) + assert.Equal(t, 0.9, captured[3].Temperature) + for _, request := range captured { + assert.Equal(t, 777, request.MaxTokens) + } +} diff --git a/backend/internal/service/rag_service.go b/backend/internal/service/rag_service.go index 9f96acdf..8ea934f8 100644 --- a/backend/internal/service/rag_service.go +++ b/backend/internal/service/rag_service.go @@ -101,6 +101,7 @@ func (s *RAGService) Query(ctx context.Context, accountID uint, req *RAGQueryReq applogger.L().Warnf("RAG get assistant config: %v", err) cfg = &model.AssistantConfig{} } + ctx = withAssistantGenerationConfig(ctx, cfg) // Step 2: Generate embedding for the question embedResp, err := s.llmProvider.CreateEmbedding(ctx, llm.EmbeddingRequest{ @@ -171,7 +172,7 @@ func (s *RAGService) Query(ctx context.Context, accountID uint, req *RAGQueryReq // Step 6: Call LLM for answer generation modelName := cfg.Model temperature := cfg.Temperature - if temperature == 0 { + if temperature == 0 && !cfg.TemperatureConfigured { temperature = 0.3 // lower temp for factual answers } @@ -211,7 +212,7 @@ func (s *RAGService) queryWithoutContext(ctx context.Context, assistant *model.C modelName := cfg.Model temperature := cfg.Temperature - if temperature == 0 { + if temperature == 0 && !cfg.TemperatureConfigured { temperature = 0.5 } diff --git a/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.spec.js b/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.spec.js index 2af36056..b42dabc8 100644 --- a/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.spec.js +++ b/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.spec.js @@ -7,7 +7,7 @@ const configuredProvider = { provider: 'openai', base_url: 'https://api.openai.com/v1', model: 'gpt-4o-mini', - api_key: { configured: true, masked: 'sec****-key' }, + api_key: { configured: true, masked_value: 'sec****-key' }, }, embedding: { mode: 'reuse_chat_credentials', diff --git a/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.vue b/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.vue index 994df013..c1e7f5fb 100644 --- a/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.vue +++ b/frontend/app/javascript/dashboard/routes/dashboard/settings/captain/components/ProviderConfiguration.vue @@ -296,7 +296,7 @@ const resultClass = ok => 'CAPTAIN_SETTINGS.PROVIDER.API_KEY_CONFIGURED', { masked: - config.chat.api_key.masked || + config.chat.api_key.masked_value || t( 'CAPTAIN_SETTINGS.PROVIDER.API_KEY_MASKED' ), @@ -393,7 +393,7 @@ const resultClass = ok => 'CAPTAIN_SETTINGS.PROVIDER.API_KEY_CONFIGURED', { masked: - config.embedding.api_key.masked || + config.embedding.api_key.masked_value || t( 'CAPTAIN_SETTINGS.PROVIDER.API_KEY_MASKED' ),