mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-11 22:12:23 +00:00
Revert #38 QuickAnalysis change (introduced a patrol regression)
Reverts b39cfc3a0. That port of v5 fix #1360 made QuickAnalysis build a
per-model provider via NewForModel, but v6 deliberately uses the configured
s.provider (it already has createPatrolProviderForModel for the per-model
case elsewhere), and NewForModel resolves unknown prefixes to a real Ollama
client rather than erroring — so the fallback never fired. This broke the
existing TestPatrolService_AskAIAboutAlert tests (panic via an empty
response) and changed v6's intended behavior. Restored to v6's original
QuickAnalysis. #1360 needs re-evaluation against v6's provider model before
any re-attempt.
This commit is contained in:
+7
-21
@@ -1752,36 +1752,22 @@ func (s *Service) createPatrolProviderForModel(modelStr string) (providers.Strea
|
||||
// It uses a single-turn, no-tools call for efficiency.
|
||||
func (s *Service) QuickAnalysis(ctx context.Context, req QuickAnalysisRequest) (string, error) {
|
||||
s.mu.RLock()
|
||||
defaultProvider := s.provider
|
||||
provider := s.provider
|
||||
cfg := s.cfg
|
||||
costStore := s.costStore
|
||||
s.mu.RUnlock()
|
||||
|
||||
if cfg == nil || !cfg.Enabled {
|
||||
return "", fmt.Errorf("Pulse Assistant is not enabled or configured")
|
||||
}
|
||||
|
||||
// Use the configured patrol model and create a provider for that exact model,
|
||||
// so quick patrol analysis goes to the selected Patrol provider instead of
|
||||
// whichever default provider the service booted with (#1360).
|
||||
model := ""
|
||||
if cfg.PatrolModel != "" {
|
||||
model = cfg.PatrolModel
|
||||
}
|
||||
provider := defaultProvider
|
||||
if model != "" {
|
||||
if modelProvider, err := providers.NewForModel(cfg, model); err == nil {
|
||||
provider = modelProvider
|
||||
} else {
|
||||
log.Debug().Err(err).Str("model", model).Msg("Could not create provider for patrol quick analysis, using default")
|
||||
}
|
||||
}
|
||||
if provider == nil {
|
||||
return "", fmt.Errorf("Pulse Assistant is not enabled or configured")
|
||||
}
|
||||
|
||||
// Use a fast model for quick analysis if available
|
||||
model := ""
|
||||
if cfg != nil && cfg.PatrolModel != "" {
|
||||
model = cfg.PatrolModel
|
||||
}
|
||||
sanitizerModel := model
|
||||
if sanitizerModel == "" {
|
||||
if sanitizerModel == "" && cfg != nil {
|
||||
sanitizerModel = cfg.GetChatModel()
|
||||
}
|
||||
|
||||
|
||||
@@ -2,9 +2,6 @@ package ai
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -26,12 +23,11 @@ func TestService_QuickAnalysis(t *testing.T) {
|
||||
t.Errorf("Expected error about provider not enabled, got: %v", err)
|
||||
}
|
||||
|
||||
// Case 2: Configured without a patrol model -> falls back to the default
|
||||
// provider with no model override.
|
||||
// Case 2: Configured
|
||||
mockProv := &mockProvider{
|
||||
chatFunc: func(ctx context.Context, req providers.ChatRequest) (*providers.ChatResponse, error) {
|
||||
if req.Model != "" {
|
||||
t.Fatalf("expected default-provider fallback with empty model override, got %q", req.Model)
|
||||
if req.Model != "fast-model" {
|
||||
return nil, nil // Should use patrol model
|
||||
}
|
||||
if req.ExecutionID != "patrol-run-123" {
|
||||
t.Fatalf("execution_id=%q want patrol-run-123", req.ExecutionID)
|
||||
@@ -43,7 +39,8 @@ func TestService_QuickAnalysis(t *testing.T) {
|
||||
}
|
||||
svc.provider = mockProv
|
||||
svc.cfg = &config.AIConfig{
|
||||
Enabled: true,
|
||||
Enabled: true,
|
||||
PatrolModel: "fast-model",
|
||||
}
|
||||
|
||||
res, err := svc.QuickAnalysis(context.Background(), QuickAnalysisRequest{
|
||||
@@ -68,71 +65,6 @@ func TestService_QuickAnalysis(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestService_QuickAnalysis_UsesPatrolModelProviderInsteadOfDefaultProvider(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
openAI := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/v1/chat/completions" {
|
||||
t.Fatalf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
|
||||
var req map[string]interface{}
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
t.Fatalf("decode request: %v", err)
|
||||
}
|
||||
if got, _ := req["model"].(string); got != "gpt-4o-mini" {
|
||||
t.Fatalf("model = %q, want gpt-4o-mini", got)
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]interface{}{
|
||||
"id": "chatcmpl-test",
|
||||
"object": "chat.completion",
|
||||
"created": 1,
|
||||
"model": "gpt-4o-mini",
|
||||
"choices": []map[string]interface{}{
|
||||
{
|
||||
"index": 0,
|
||||
"message": map[string]interface{}{
|
||||
"role": "assistant",
|
||||
"content": "analysis from patrol model",
|
||||
},
|
||||
"finish_reason": "stop",
|
||||
},
|
||||
},
|
||||
"usage": map[string]interface{}{
|
||||
"prompt_tokens": 11,
|
||||
"completion_tokens": 7,
|
||||
"total_tokens": 18,
|
||||
},
|
||||
})
|
||||
}))
|
||||
defer openAI.Close()
|
||||
|
||||
svc := NewService(nil, nil)
|
||||
svc.provider = &mockProvider{
|
||||
chatFunc: func(ctx context.Context, req providers.ChatRequest) (*providers.ChatResponse, error) {
|
||||
t.Fatal("expected QuickAnalysis to avoid the stale default provider")
|
||||
return nil, nil
|
||||
},
|
||||
}
|
||||
svc.cfg = &config.AIConfig{
|
||||
Enabled: true,
|
||||
Model: "gemini:gemini-2.5-pro",
|
||||
PatrolModel: "openai:gpt-4o-mini",
|
||||
OpenAIAPIKey: "test-key",
|
||||
OpenAIBaseURL: openAI.URL,
|
||||
}
|
||||
|
||||
res, err := svc.QuickAnalysis(context.Background(), QuickAnalysisRequest{Prompt: "Analysis prompt"})
|
||||
if err != nil {
|
||||
t.Fatalf("QuickAnalysis failed: %v", err)
|
||||
}
|
||||
if res != "analysis from patrol model" {
|
||||
t.Fatalf("unexpected result: %s", res)
|
||||
}
|
||||
}
|
||||
|
||||
func TestService_QuickAnalysisSanitizesExternalModelRequest(t *testing.T) {
|
||||
svc := NewService(nil, nil)
|
||||
resource := unifiedresources.Resource{
|
||||
|
||||
Reference in New Issue
Block a user