diff --git a/alerter/src/internal/config/config.go b/alerter/src/internal/config/config.go index 4fc9f8c8..5659a165 100644 --- a/alerter/src/internal/config/config.go +++ b/alerter/src/internal/config/config.go @@ -20,6 +20,12 @@ import ( "gopkg.in/yaml.v3" ) +// DefaultLLMMaxTokens is the output-token budget applied to reasoning +// requests when llm.max_tokens is unset or non-positive. It is generous +// enough that a reasoning model can emit a thinking block and still have +// room for the classification that follows it. +const DefaultLLMMaxTokens = 4096 + // Config holds all configuration options for the alerter type Config struct { // Datastore connection settings @@ -147,13 +153,31 @@ type CorrelationConfig struct { // LLMConfig holds LLM provider settings type LLMConfig struct { - EmbeddingProvider string `yaml:"embedding_provider"` - ReasoningProvider string `yaml:"reasoning_provider"` - Ollama OllamaConfig `yaml:"ollama"` - OpenAI OpenAIConfig `yaml:"openai"` - Anthropic AnthropicConfig `yaml:"anthropic"` - Voyage VoyageConfig `yaml:"voyage"` - Gemini GeminiConfig `yaml:"gemini"` + EmbeddingProvider string `yaml:"embedding_provider"` + ReasoningProvider string `yaml:"reasoning_provider"` + + // MaxTokens is the output-token budget for a reasoning request. + // A reasoning model spends part of that budget on its thinking block + // before it emits any answer, so a tight cap can leave no room for + // the classification itself. Defaults to DefaultLLMMaxTokens when + // unset or non-positive. + MaxTokens int `yaml:"max_tokens"` + + Ollama OllamaConfig `yaml:"ollama"` + OpenAI OpenAIConfig `yaml:"openai"` + Anthropic AnthropicConfig `yaml:"anthropic"` + Voyage VoyageConfig `yaml:"voyage"` + Gemini GeminiConfig `yaml:"gemini"` +} + +// ReasoningMaxTokens returns the output-token budget for a reasoning +// request, falling back to DefaultLLMMaxTokens when the operator has not +// configured a positive llm.max_tokens. +func (l *LLMConfig) ReasoningMaxTokens() int { + if l == nil || l.MaxTokens <= 0 { + return DefaultLLMMaxTokens + } + return l.MaxTokens } // NotificationsConfig holds notification settings @@ -340,6 +364,7 @@ func NewConfig() *Config { LLM: LLMConfig{ EmbeddingProvider: "ollama", ReasoningProvider: "ollama", + MaxTokens: DefaultLLMMaxTokens, Ollama: OllamaConfig{ BaseURL: "http://localhost:11434", EmbeddingModel: "nomic-embed-text", diff --git a/alerter/src/internal/config/config_test.go b/alerter/src/internal/config/config_test.go index f6d55415..2f3776c1 100644 --- a/alerter/src/internal/config/config_test.go +++ b/alerter/src/internal/config/config_test.go @@ -51,6 +51,7 @@ func TestNewConfig(t *testing.T) { {"correlation window", cfg.Correlation.WindowSeconds, 120}, {"llm embedding provider", cfg.LLM.EmbeddingProvider, "ollama"}, {"llm reasoning provider", cfg.LLM.ReasoningProvider, "ollama"}, + {"llm max tokens", cfg.LLM.MaxTokens, DefaultLLMMaxTokens}, {"gemini embedding model", cfg.LLM.Gemini.EmbeddingModel, "gemini-embedding-001"}, {"gemini reasoning model", cfg.LLM.Gemini.ReasoningModel, "gemini-2.5-flash"}, } @@ -64,6 +65,32 @@ func TestNewConfig(t *testing.T) { } } +// TestReasoningMaxTokens verifies that the reasoning budget comes from +// llm.max_tokens and falls back to DefaultLLMMaxTokens whenever the +// setting is absent or non-positive. A nil receiver is covered because +// callers may hold a nil *LLMConfig. +func TestReasoningMaxTokens(t *testing.T) { + tests := []struct { + name string + cfg *LLMConfig + want int + }{ + {name: "nil config", cfg: nil, want: DefaultLLMMaxTokens}, + {name: "unset", cfg: &LLMConfig{}, want: DefaultLLMMaxTokens}, + {name: "zero", cfg: &LLMConfig{MaxTokens: 0}, want: DefaultLLMMaxTokens}, + {name: "negative", cfg: &LLMConfig{MaxTokens: -5}, want: DefaultLLMMaxTokens}, + {name: "configured", cfg: &LLMConfig{MaxTokens: 8192}, want: 8192}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.cfg.ReasoningMaxTokens(); got != tt.want { + t.Errorf("ReasoningMaxTokens() = %d, want %d", got, tt.want) + } + }) + } +} + // TestNewConfigAnomalyDefaults verifies the defaults for the new // anomaly knobs introduced for the tier-1 warmup gate and the // hybrid variance floor. Every newly added field is asserted so diff --git a/alerter/src/internal/llm/llm.go b/alerter/src/internal/llm/llm.go index d3e9d73b..e84bf65a 100644 --- a/alerter/src/internal/llm/llm.go +++ b/alerter/src/internal/llm/llm.go @@ -174,28 +174,32 @@ func NewReasoningProvider(cfg *config.Config) (ReasoningProvider, error) { if apiKey == "" && cfg.LLM.OpenAI.BaseURL == "" { return nil, fmt.Errorf("openai: %w", ErrAPIKeyMissing) } - return newLibReasoning("openai", apiKey, cfg.LLM.OpenAI.ReasoningModel, cfg.LLM.OpenAI.BaseURL) + return newLibReasoning("openai", apiKey, cfg.LLM.OpenAI.ReasoningModel, + cfg.LLM.OpenAI.BaseURL, cfg.LLM.ReasoningMaxTokens()) case "anthropic": apiKey := cfg.GetAnthropicAPIKey() if apiKey == "" { return nil, fmt.Errorf("anthropic: %w", ErrAPIKeyMissing) } - return newLibReasoning("anthropic", apiKey, cfg.LLM.Anthropic.ReasoningModel, cfg.LLM.Anthropic.BaseURL) + return newLibReasoning("anthropic", apiKey, cfg.LLM.Anthropic.ReasoningModel, + cfg.LLM.Anthropic.BaseURL, cfg.LLM.ReasoningMaxTokens()) case "gemini": apiKey := cfg.GetGeminiAPIKey() if apiKey == "" { return nil, fmt.Errorf("gemini: %w", ErrAPIKeyMissing) } - return newLibReasoning("gemini", apiKey, cfg.LLM.Gemini.ReasoningModel, cfg.LLM.Gemini.BaseURL) + return newLibReasoning("gemini", apiKey, cfg.LLM.Gemini.ReasoningModel, + cfg.LLM.Gemini.BaseURL, cfg.LLM.ReasoningMaxTokens()) case "ollama": baseURL := cfg.LLM.Ollama.BaseURL if baseURL == "" { baseURL = "http://localhost:11434" } - return newLibReasoning("ollama", "", cfg.LLM.Ollama.ReasoningModel, baseURL) + return newLibReasoning("ollama", "", cfg.LLM.Ollama.ReasoningModel, baseURL, + cfg.LLM.ReasoningMaxTokens()) case "", "none", "disabled": return nil, nil diff --git a/alerter/src/internal/llm/reasoning.go b/alerter/src/internal/llm/reasoning.go index f22f44d7..570db07f 100644 --- a/alerter/src/internal/llm/reasoning.go +++ b/alerter/src/internal/llm/reasoning.go @@ -19,6 +19,8 @@ import ( // Register all built-in LLM providers (anthropic, openai, gemini, // ollama, voyage) so pgllm.NewClient can construct them by name. _ "github.com/pgEdge/pgedge-go-llm-lib/llm/all" + + "github.com/pgedge/ai-workbench/alerter/internal/config" ) // defaultReasoningModels maps a provider name to the reasoning model used @@ -35,17 +37,24 @@ var defaultReasoningModels = map[string]string{ // pgedge-go-llm-lib Client.Chat method. A single implementation serves // every provider; the provider-specific behavior lives in the library. type libReasoning struct { - client pgllm.Client + client pgllm.Client + maxTokens int } // Classify sends the classification system prompt and the supplied prompt // to the underlying LLM and returns the concatenated text of the // response's text blocks. Non-text content blocks are ignored. +// +// The output-token budget comes from the operator-configured +// llm.max_tokens; a response that carries no text at all is reported as an +// error rather than an empty classification, because that is what a +// reasoning model produces when it exhausts the budget on its thinking +// block before emitting an answer. func (r *libReasoning) Classify(ctx context.Context, prompt string) (string, error) { resp, err := r.client.Chat(ctx, pgllm.ChatRequest{ SystemPrompt: classificationSystemPrompt, Messages: []pgllm.Message{pgllm.UserText(prompt)}, - MaxTokens: pgllm.Int(500), + MaxTokens: pgllm.Int(r.maxTokens), Temperature: pgllm.Float(0.1), }) if err != nil { @@ -58,7 +67,13 @@ func (r *libReasoning) Classify(ctx context.Context, prompt string) (string, err sb.WriteString(b.Text) } } - return sb.String(), nil + text := sb.String() + if strings.TrimSpace(text) == "" { + return "", fmt.Errorf("LLM returned no text content; the output token "+ + "budget (llm.max_tokens = %d) may be too small for this model", + r.maxTokens) + } + return text, nil } // ModelName returns the reasoning model configured on the client. @@ -68,11 +83,16 @@ func (r *libReasoning) ModelName() string { // newLibReasoning builds a libReasoning for the named provider. When model // is empty, the per-provider default from defaultReasoningModels is used. -// apiKey and baseURL are passed straight through to the library. -func newLibReasoning(provider, apiKey, model, baseURL string) (ReasoningProvider, error) { +// apiKey and baseURL are passed straight through to the library. A +// non-positive maxTokens falls back to config.DefaultLLMMaxTokens so a +// caller that omits the setting still gets a workable budget. +func newLibReasoning(provider, apiKey, model, baseURL string, maxTokens int) (ReasoningProvider, error) { if model == "" { model = defaultReasoningModels[provider] } + if maxTokens <= 0 { + maxTokens = config.DefaultLLMMaxTokens + } client, err := pgllm.NewClient(provider, pgllm.Options{ APIKey: apiKey, @@ -83,5 +103,5 @@ func newLibReasoning(provider, apiKey, model, baseURL string) (ReasoningProvider return nil, fmt.Errorf("create %s reasoning client: %w", provider, err) } - return &libReasoning{client: client}, nil + return &libReasoning{client: client, maxTokens: maxTokens}, nil } diff --git a/alerter/src/internal/llm/reasoning_test.go b/alerter/src/internal/llm/reasoning_test.go index 8e349170..838e75ec 100644 --- a/alerter/src/internal/llm/reasoning_test.go +++ b/alerter/src/internal/llm/reasoning_test.go @@ -14,6 +14,7 @@ import ( "context" "errors" "os" + "strings" "testing" pgllm "github.com/pgEdge/pgedge-go-llm-lib/llm" @@ -49,7 +50,7 @@ func TestLibReasoningClassify(t *testing.T) { {Type: pgllm.BlockText, Text: `"confidence":0.9}`}, }}, } - r := &libReasoning{client: fc} + r := &libReasoning{client: fc, maxTokens: 8192} out, err := r.Classify(context.Background(), "analyze this anomaly") if err != nil { t.Fatalf("Classify: %v", err) @@ -68,8 +69,8 @@ func TestLibReasoningClassify(t *testing.T) { fc.gotReq.Messages[0].Content[0].Text != "analyze this anomaly" { t.Fatalf("user message content = %+v", fc.gotReq.Messages[0].Content) } - if fc.gotReq.MaxTokens == nil || *fc.gotReq.MaxTokens != 500 { - t.Fatalf("MaxTokens = %v, want 500", fc.gotReq.MaxTokens) + if fc.gotReq.MaxTokens == nil || *fc.gotReq.MaxTokens != 8192 { + t.Fatalf("MaxTokens = %v, want 8192", fc.gotReq.MaxTokens) } if fc.gotReq.Temperature == nil || *fc.gotReq.Temperature != 0.1 { t.Fatalf("Temperature = %v, want 0.1", fc.gotReq.Temperature) @@ -80,27 +81,85 @@ func TestLibReasoningClassify(t *testing.T) { } func TestLibReasoningClassifyError(t *testing.T) { - r := &libReasoning{client: &fakeChatClient{err: errors.New("boom")}} + r := &libReasoning{client: &fakeChatClient{err: errors.New("boom")}, maxTokens: 4096} if _, err := r.Classify(context.Background(), "x"); err == nil { t.Fatal("expected error") } } -// TestLibReasoningClassifyEmptyContent verifies that Classify returns ("", nil) -// when the response contains no text blocks; the caller's parser treats an -// empty string as the "no-decision" fallback. +// TestLibReasoningClassifyEmptyContent verifies that Classify reports an +// error when the response carries no text blocks, which is what a reasoning +// model produces once its thinking block exhausts the output budget. The +// error names the setting so an operator can act on it. func TestLibReasoningClassifyEmptyContent(t *testing.T) { - fc := &fakeChatClient{ - model: "gpt-4o-mini", - resp: &pgllm.ChatResponse{Content: []pgllm.ContentBlock{}}, + cases := []struct { + name string + content []pgllm.ContentBlock + }{ + {name: "no blocks", content: []pgllm.ContentBlock{}}, + { + name: "whitespace only", + content: []pgllm.ContentBlock{ + {Type: pgllm.BlockText, Text: " \n "}, + }, + }, + { + name: "non-text block only", + content: []pgllm.ContentBlock{ + {Type: pgllm.BlockToolUse}, + }, + }, } - r := &libReasoning{client: fc} - out, err := r.Classify(context.Background(), "prompt") - if err != nil { - t.Fatalf("Classify: %v", err) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fc := &fakeChatClient{ + model: "gpt-4o-mini", + resp: &pgllm.ChatResponse{Content: tc.content}, + } + r := &libReasoning{client: fc, maxTokens: 512} + out, err := r.Classify(context.Background(), "prompt") + if err == nil { + t.Fatal("expected an error for a response with no text content") + } + if !strings.Contains(err.Error(), "llm.max_tokens = 512") { + t.Fatalf("error = %v, want it to name the configured budget", err) + } + if out != "" { + t.Fatalf("out = %q, want empty string", out) + } + }) + } +} + +// TestNewLibReasoningMaxTokens verifies that the configured budget is +// carried onto the provider and that a non-positive value falls back to +// config.DefaultLLMMaxTokens. +func TestNewLibReasoningMaxTokens(t *testing.T) { + cases := []struct { + name string + given int + want int + }{ + {name: "explicit", given: 16384, want: 16384}, + {name: "zero falls back", given: 0, want: config.DefaultLLMMaxTokens}, + {name: "negative falls back", given: -1, want: config.DefaultLLMMaxTokens}, } - if out != "" { - t.Fatalf("out = %q, want empty string", out) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r, err := newLibReasoning("openai", "test-key", "gpt-4o", "", tc.given) + if err != nil { + t.Fatalf("newLibReasoning: %v", err) + } + lr, ok := r.(*libReasoning) + if !ok { + t.Fatalf("provider type = %T, want *libReasoning", r) + } + if lr.maxTokens != tc.want { + t.Fatalf("maxTokens = %d, want %d", lr.maxTokens, tc.want) + } + }) } } @@ -108,7 +167,7 @@ func TestLibReasoningClassifyEmptyContent(t *testing.T) { // accepted and that the default model (llama3.2) is applied when none is given. // No network is attempted: the library constructs the client without dialing. func TestNewLibReasoningOllamaExplicitBaseURL(t *testing.T) { - r, err := newLibReasoning("ollama", "", "", "http://localhost:11434") + r, err := newLibReasoning("ollama", "", "", "http://localhost:11434", 4096) if err != nil { t.Fatalf("newLibReasoning: %v", err) } @@ -120,7 +179,7 @@ func TestNewLibReasoningOllamaExplicitBaseURL(t *testing.T) { // TestNewLibReasoningDefaultModel verifies that an empty model falls back // to the per-provider default and that the resulting client reports it. func TestNewLibReasoningDefaultModel(t *testing.T) { - r, err := newLibReasoning("openai", "test-key", "", "") + r, err := newLibReasoning("openai", "test-key", "", "", 4096) if err != nil { t.Fatalf("newLibReasoning: %v", err) } @@ -131,7 +190,7 @@ func TestNewLibReasoningDefaultModel(t *testing.T) { // TestNewLibReasoningExplicitModel verifies an explicit model is preserved. func TestNewLibReasoningExplicitModel(t *testing.T) { - r, err := newLibReasoning("openai", "test-key", "gpt-4o", "") + r, err := newLibReasoning("openai", "test-key", "gpt-4o", "", 4096) if err != nil { t.Fatalf("newLibReasoning: %v", err) } @@ -143,7 +202,7 @@ func TestNewLibReasoningExplicitModel(t *testing.T) { // TestNewLibReasoningUnknownProvider verifies NewClient errors propagate // through newLibReasoning as a wrapped error. func TestNewLibReasoningUnknownProvider(t *testing.T) { - if _, err := newLibReasoning("does-not-exist", "k", "m", ""); err == nil { + if _, err := newLibReasoning("does-not-exist", "k", "m", "", 4096); err == nil { t.Fatal("expected error for unknown provider") } } diff --git a/docs/changelog.md b/docs/changelog.md index 1d847e9f..6e21023d 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -61,6 +61,19 @@ project adheres to ### Fixed +- Honour the configured `llm.max_tokens` on the AI estate summary, + the Server Info database analysis, and the alerter's tier 3 + classification, each of which previously hardcoded a 512-token + output budget. A reasoning model spends part of its budget on an + internal thinking block, so the small cap was consumed before the + model emitted any answer and the estate summary rendered as an + empty panel. Each path now reads the operator-configured budget + and falls back to `4096` when the setting is absent, and a + response that carries no text content is reported as an error + rather than being cached and rendered as an empty result. The + alerter gains a matching `llm.max_tokens` setting, which it + previously lacked. (#399) + - Fix every chat request that included a tool list failing with `anthropic (400): tools.0.custom.input_schema: Input does not match the expected shape`, which broke Ask Ellie and the Server, diff --git a/docs/getting-started/configuration/alerter.md b/docs/getting-started/configuration/alerter.md index 5fb1212d..438a3d1e 100644 --- a/docs/getting-started/configuration/alerter.md +++ b/docs/getting-started/configuration/alerter.md @@ -317,6 +317,15 @@ an older pgvector causes the embedding-column migration to fail. |--------|------|---------|-------------| | `embedding_provider` | string | `ollama` | Embedding provider | | `reasoning_provider` | string | `ollama` | Classification provider | +| `max_tokens` | int | `4096` | Max response tokens for classification | + +The `max_tokens` option sets the output-token budget for a tier 3 +classification request. A reasoning model spends part of that budget on +its internal thinking block before it emits any answer, so a tight +budget can be consumed entirely by reasoning; the alerter then reports +the classification as failed rather than treating the empty response as +a decision. Values less than or equal to zero fall back to the default +of `4096`. #### Ollama Configuration diff --git a/docs/getting-started/configuration/server.md b/docs/getting-started/configuration/server.md index 07ae1f12..850541dd 100644 --- a/docs/getting-started/configuration/server.md +++ b/docs/getting-started/configuration/server.md @@ -425,7 +425,7 @@ contain a valid key. | `openai_api_key_file` | string | | OpenAI key path | | `gemini_api_key_file` | string | | Gemini key path | | `ollama_url` | string | `http://localhost:11434` | Ollama URL | -| `max_tokens` | int | `4096` | Max response tokens | +| `max_tokens` | int | `4096` | Max response tokens for chat and analysis | | `temperature` | float | `0.7` | Sampling temperature | | `max_iterations` | int | `50` | Max tool-call iterations | | `compact_tool_descriptions` | string | `auto` | Tool description mode | @@ -434,6 +434,38 @@ contain a valid key. | `openai_base_url` | string | `https://api.openai.com/v1` | OpenAI base URL | | `gemini_base_url` | string | `https://generativelanguage.googleapis.com` | Gemini base URL | +#### Response Length (`max_tokens`) + +The `max_tokens` option sets the output-token budget +for every LLM request the server makes. The budget +governs Ask Ellie chat, the AI estate summary on the +overview page, and the AI database analysis in the +Server Info dialog. The default value is `4096`; +values less than or equal to zero fall back to that +default. + +A reasoning model spends part of the budget on its +internal thinking block before it emits any answer +text, so a tight budget can be consumed entirely by +reasoning. When that happens the response arrives +with no text content, and the server reports an +error rather than displaying an empty summary. +Raise `max_tokens` when a local reasoning model +produces such errors, or disable the model's +thinking output. + +In the following example, the `llm` section widens +the budget for a local reasoning model served over +an OpenAI-compatible endpoint: + +```yaml +llm: + provider: "openai" + model: "qwen3" + openai_base_url: "http://localhost:8080/v1" + max_tokens: 8192 +``` + #### Request Timeout (`timeout_seconds`) The `timeout_seconds` option sets the HTTP client diff --git a/examples/ai-dba-alerter.yaml b/examples/ai-dba-alerter.yaml index d86f1db0..7bb6d434 100644 --- a/examples/ai-dba-alerter.yaml +++ b/examples/ai-dba-alerter.yaml @@ -324,6 +324,14 @@ llm: # Default: ollama reasoning_provider: ollama + # Maximum tokens for a tier 3 classification response. A reasoning + # model spends part of the budget on its thinking block before it + # emits any answer, so raise this value when a local reasoning model + # returns no classification text. Values of zero or less fall back to + # the default. + # Default: 4096 + max_tokens: 4096 + #------------------------------------------------------------------------- # Ollama Settings (local LLM) #------------------------------------------------------------------------- diff --git a/examples/ai-dba-server.yaml b/examples/ai-dba-server.yaml index 55cdafcf..85767e6c 100644 --- a/examples/ai-dba-server.yaml +++ b/examples/ai-dba-server.yaml @@ -342,7 +342,11 @@ llm: # Generation Parameters #----------------------------------------------------------------------- - # Maximum tokens for LLM response + # Maximum tokens for an LLM response. The budget applies to Ask Ellie + # chat, the AI estate summary, and the AI database analysis in the + # Server Info dialog. A reasoning model spends part of the budget on + # its thinking block before it emits any answer, so raise this value + # when a local reasoning model returns no summary text. # Default: 4096 max_tokens: 4096 diff --git a/server/src/internal/api/server_info_handlers.go b/server/src/internal/api/server_info_handlers.go index ccbdb17d..5228f8ca 100644 --- a/server/src/internal/api/server_info_handlers.go +++ b/server/src/internal/api/server_info_handlers.go @@ -30,9 +30,6 @@ import ( // aiAnalysisCacheTTL is how long AI database analysis is cached. const aiAnalysisCacheTTL = 5 * time.Minute -// llmAnalysisMaxTokens caps the AI analysis response length. -const llmAnalysisMaxTokens = 512 - // llmAnalysisTemperature controls response creativity for analysis. const llmAnalysisTemperature = 0.3 @@ -681,10 +678,11 @@ func (h *ServerInfoHandler) getAIAnalysis( return nil } + maxTokens := h.llmConfig.AnalysisMaxTokens() resp, err := client.Chat(ctx, pgllm.ChatRequest{ Messages: []pgllm.Message{pgllm.UserText(prompt)}, SystemPrompt: "", - MaxTokens: pgllm.Int(llmAnalysisMaxTokens), + MaxTokens: pgllm.Int(maxTokens), Temperature: pgllm.Float(llmAnalysisTemperature), }) if err != nil { @@ -696,6 +694,17 @@ func (h *ServerInfoHandler) getAIAnalysis( // Parse the response into per-database analysis analysis := parseDatabaseAnalysisResponse(resp, databases) + // A response that yields nothing usable is a failure rather than an + // empty analysis; it happens when a reasoning model spends the whole + // output budget on its thinking block. Report it and skip the cache so + // the next request retries instead of serving a blank result. + if len(analysis) == 0 { + log.Printf("[ERROR] AI analysis for connection %d returned no usable "+ + "content; the output token budget (llm.max_tokens = %d) may be "+ + "too small for this model", connectionID, maxTokens) + return nil + } + // Cache the result now := time.Now().UTC() h.cacheMu.Lock() @@ -819,7 +828,7 @@ func (h *ServerInfoHandler) createLLMClient() (pgllm.Client, error) { // Credential selection, custom-header wiring, and the // timeout-only-when-positive rule live in the shared llmproxy helper // so the overview and server-info analysis paths stay in lock-step. - opts := h.llmConfig.BuildClientOptions(llmAnalysisMaxTokens, llmAnalysisTemperature) + opts := h.llmConfig.BuildClientOptions(h.llmConfig.AnalysisMaxTokens(), llmAnalysisTemperature) client, err := pgllm.NewClient(provider, opts) if err != nil { diff --git a/server/src/internal/api/server_info_handlers_test.go b/server/src/internal/api/server_info_handlers_test.go index f317e6d9..9bc0a654 100644 --- a/server/src/internal/api/server_info_handlers_test.go +++ b/server/src/internal/api/server_info_handlers_test.go @@ -11,6 +11,7 @@ package api import ( "context" + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -642,6 +643,144 @@ func TestServerInfoGetAIAnalysis(t *testing.T) { } }) + t.Run("sends the configured max tokens", func(t *testing.T) { + // The wire budget must come from llm.max_tokens rather than a + // compile-time constant. The stub accepts either OpenAI token + // field, since the provider chooses between them by model name. + var got int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + MaxTokens *int `json:"max_tokens"` + MaxCompletionTokens *int `json:"max_completion_tokens"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "bad body", http.StatusBadRequest) + return + } + switch { + case body.MaxTokens != nil: + got = *body.MaxTokens + case body.MaxCompletionTokens != nil: + got = *body.MaxCompletionTokens + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "choices": [{"message": {"role": "assistant", "content": "mydb: A primary application store.\n"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15} + }`)) + })) + defer srv.Close() + + h := &ServerInfoHandler{ + llmConfig: &llmproxy.Config{ + Provider: "openai", + Model: "gpt-4o", + OpenAIAPIKey: "test-key", + OpenAIBaseURL: srv.URL, + MaxTokens: 8192, + }, + cache: make(map[int]*aiCacheEntry), + } + + if result := h.getAIAnalysis( + context.Background(), 11, + []DatabaseInfo{{Name: "mydb"}}, nil, + ); result == nil { + t.Fatal("expected non-nil analysis") + } + if got != 8192 { + t.Errorf("max tokens on the wire = %d, want 8192", got) + } + }) + + t.Run("falls back to the default max tokens when unset", func(t *testing.T) { + // An unset llm.max_tokens must yield the generous shared default, + // not the old hardcoded 512. + var got int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + MaxTokens *int `json:"max_tokens"` + MaxCompletionTokens *int `json:"max_completion_tokens"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "bad body", http.StatusBadRequest) + return + } + switch { + case body.MaxTokens != nil: + got = *body.MaxTokens + case body.MaxCompletionTokens != nil: + got = *body.MaxCompletionTokens + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "choices": [{"message": {"role": "assistant", "content": "mydb: A primary application store.\n"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15} + }`)) + })) + defer srv.Close() + + h := &ServerInfoHandler{ + llmConfig: &llmproxy.Config{ + Provider: "openai", + Model: "gpt-4o", + OpenAIAPIKey: "test-key", + OpenAIBaseURL: srv.URL, + }, + cache: make(map[int]*aiCacheEntry), + } + + if result := h.getAIAnalysis( + context.Background(), 12, + []DatabaseInfo{{Name: "mydb"}}, nil, + ); result == nil { + t.Fatal("expected non-nil analysis") + } + if got != llmproxy.DefaultAnalysisMaxTokens { + t.Errorf("max tokens on the wire = %d, want %d", + got, llmproxy.DefaultAnalysisMaxTokens) + } + }) + + t.Run("returns nil and skips cache when no usable content", func(t *testing.T) { + // A reasoning model that spends its whole budget on thinking + // returns no parsable line; the result must be reported as a + // failure and left uncached so the next request retries. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "choices": [{"message": {"role": "assistant", "content": ""}, "finish_reason": "length"}], + "usage": {"prompt_tokens": 10, "completion_tokens": 512, "total_tokens": 522} + }`)) + })) + defer srv.Close() + + h := &ServerInfoHandler{ + llmConfig: &llmproxy.Config{ + Provider: "openai", + Model: "gpt-4o", + OpenAIAPIKey: "test-key", + OpenAIBaseURL: srv.URL, + MaxTokens: 512, + }, + cache: make(map[int]*aiCacheEntry), + } + + result := h.getAIAnalysis( + context.Background(), 13, + []DatabaseInfo{{Name: "mydb"}}, nil, + ) + if result != nil { + t.Error("expected nil when the response carries no usable content") + } + h.cacheMu.RLock() + _, ok := h.cache[13] + h.cacheMu.RUnlock() + if ok { + t.Error("expected no cache entry when the response is unusable") + } + }) + t.Run("returns nil when LLM call fails", func(t *testing.T) { // A non-retryable 400 makes Chat fail fast; getAIAnalysis logs and // returns nil without caching. diff --git a/server/src/internal/llmproxy/proxy.go b/server/src/internal/llmproxy/proxy.go index 0795f36e..d174541b 100644 --- a/server/src/internal/llmproxy/proxy.go +++ b/server/src/internal/llmproxy/proxy.go @@ -62,6 +62,14 @@ type Config struct { LLMConfig *config.LLMConfig // LLMConfig for accessing custom headers (may be nil) } +// DefaultAnalysisMaxTokens is the output-token budget the analysis paths +// (estate overview and server-info database analysis) apply when the +// operator has not configured llm.max_tokens. It is deliberately generous +// because a reasoning model spends part of the budget on its thinking +// block before it emits any answer text; a tight cap leaves no room for +// the answer and the response arrives with no text content at all. +const DefaultAnalysisMaxTokens = 4096 + // maxChatBodySize caps the chat/embed/rerank request body at 5MB to // accommodate tool definitions and message history, consistent with the // DecodeJSONBody pattern used elsewhere in the API layer. @@ -320,6 +328,19 @@ func (c *Config) BuildClientOptions(maxTokens int, temperature float64) pgllm.Op return opts } +// AnalysisMaxTokens returns the output-token budget the analysis paths +// should request. It reports the operator-configured llm.max_tokens when +// that setting is positive, and DefaultAnalysisMaxTokens otherwise, so a +// missing or zero setting still leaves a reasoning model room for both +// its thinking block and an answer. The receiver may be nil, which the +// overview generator relies on when AI is disabled. +func (c *Config) AnalysisMaxTokens() int { + if c == nil || c.MaxTokens <= 0 { + return DefaultAnalysisMaxTokens + } + return c.MaxTokens +} + // authorize is the proxy's Authorize hook. The public discovery // endpoints (providers/models/health) require no credentials, matching // the old behavior that let the login page list providers. Every other diff --git a/server/src/internal/llmproxy/proxy_test.go b/server/src/internal/llmproxy/proxy_test.go index b9a2264f..79b9b88e 100644 --- a/server/src/internal/llmproxy/proxy_test.go +++ b/server/src/internal/llmproxy/proxy_test.go @@ -1343,3 +1343,47 @@ func TestBuildClientOptions_ZeroTimeoutNotApplied(t *testing.T) { t.Errorf("expected zero timeout when TimeoutSeconds=0, got %v", opts.RequestTimeout) } } + +// TestAnalysisMaxTokens verifies that the analysis paths pick up the +// operator-configured llm.max_tokens and fall back to +// DefaultAnalysisMaxTokens whenever the setting is absent or non-positive. +// A nil receiver is included because the overview generator holds a nil +// *Config when AI is disabled. +func TestAnalysisMaxTokens(t *testing.T) { + tests := []struct { + name string + cfg *Config + want int + }{ + {name: "nil config", cfg: nil, want: DefaultAnalysisMaxTokens}, + {name: "unset", cfg: &Config{}, want: DefaultAnalysisMaxTokens}, + {name: "zero", cfg: &Config{MaxTokens: 0}, want: DefaultAnalysisMaxTokens}, + {name: "negative", cfg: &Config{MaxTokens: -1}, want: DefaultAnalysisMaxTokens}, + {name: "configured", cfg: &Config{MaxTokens: 8192}, want: 8192}, + {name: "small but positive", cfg: &Config{MaxTokens: 64}, want: 64}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := tc.cfg.AnalysisMaxTokens(); got != tc.want { + t.Errorf("AnalysisMaxTokens() = %d, want %d", got, tc.want) + } + }) + } +} + +// TestBuildClientOptions_AnalysisMaxTokensFlowsThrough verifies that the +// budget AnalysisMaxTokens reports reaches the library Options the analysis +// call sites construct. +func TestBuildClientOptions_AnalysisMaxTokensFlowsThrough(t *testing.T) { + cfg := &Config{ + Provider: "anthropic", + Model: "m", + AnthropicAPIKey: "k", + MaxTokens: 12288, + } + opts := cfg.BuildClientOptions(cfg.AnalysisMaxTokens(), 0.3) + if opts.MaxTokens == nil || *opts.MaxTokens != 12288 { + t.Errorf("opts.MaxTokens = %v, want 12288", opts.MaxTokens) + } +} diff --git a/server/src/internal/overview/generator.go b/server/src/internal/overview/generator.go index ed23ffef..4aa77d2f 100644 --- a/server/src/internal/overview/generator.go +++ b/server/src/internal/overview/generator.go @@ -31,9 +31,6 @@ const ( // staleDuration is how long an overview is considered fresh. staleDuration = 5 * time.Minute - // llmMaxTokens caps the summary length for concise output. - llmMaxTokens = 512 - // llmTemperature controls response creativity; low for factual output. llmTemperature = 0.3 @@ -510,17 +507,28 @@ func (g *Generator) generateSummaryFromPrompt(ctx context.Context, system, data return "", fmt.Errorf("no LLM provider configured: %w", err) } + maxTokens := g.llmConfig.AnalysisMaxTokens() resp, err := client.Chat(ctx, pgllm.ChatRequest{ Messages: []pgllm.Message{pgllm.UserText(data)}, SystemPrompt: system, - MaxTokens: pgllm.Int(llmMaxTokens), + MaxTokens: pgllm.Int(maxTokens), Temperature: pgllm.Float(llmTemperature), }) if err != nil { return "", fmt.Errorf("LLM chat failed: %w", err) } - return extractTextFromResponse(resp), nil + // A response carrying no text block is a failure, not an empty + // summary. It happens when a reasoning model spends the whole output + // budget on its thinking block, so report it rather than letting the + // caller cache and render a blank panel. + summary := extractTextFromResponse(resp) + if strings.TrimSpace(summary) == "" { + return "", fmt.Errorf("LLM returned no text content; the output token "+ + "budget (llm.max_tokens = %d) may be too small for this model", maxTokens) + } + + return summary, nil } // createLLMClient builds a pgedge-go-llm-lib client based on the @@ -542,7 +550,7 @@ func (g *Generator) createLLMClient() (pgllm.Client, error) { // Credential selection, custom-header wiring, and the // timeout-only-when-positive rule live in the shared llmproxy helper // so the overview and server-info analysis paths stay in lock-step. - opts := g.llmConfig.BuildClientOptions(llmMaxTokens, llmTemperature) + opts := g.llmConfig.BuildClientOptions(g.llmConfig.AnalysisMaxTokens(), llmTemperature) client, err := pgllm.NewClient(provider, opts) if err != nil { diff --git a/server/src/internal/overview/generator_test.go b/server/src/internal/overview/generator_test.go index f369b99c..8baab31c 100644 --- a/server/src/internal/overview/generator_test.go +++ b/server/src/internal/overview/generator_test.go @@ -11,6 +11,7 @@ package overview import ( "context" + "encoding/json" "fmt" "io" "net/http" @@ -911,6 +912,113 @@ func TestGenerateSummaryFromPrompt_Success(t *testing.T) { } } +// TestGenerateSummaryFromPrompt_MaxTokens verifies that the output-token +// budget on the wire comes from the configured llm.max_tokens, and that an +// unset setting falls back to llmproxy.DefaultAnalysisMaxTokens rather than +// the old hardcoded 512. The stub decodes the request body and accepts +// either OpenAI token field, since the provider chooses between them by +// model name. +func TestGenerateSummaryFromPrompt_MaxTokens(t *testing.T) { + tests := []struct { + name string + configured int + want int + }{ + {name: "configured budget is applied", configured: 8192, want: 8192}, + {name: "unset falls back to the default", configured: 0, + want: llmproxy.DefaultAnalysisMaxTokens}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var got int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + MaxTokens *int `json:"max_tokens"` + MaxCompletionTokens *int `json:"max_completion_tokens"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + http.Error(w, "bad body", http.StatusBadRequest) + return + } + switch { + case body.MaxTokens != nil: + got = *body.MaxTokens + case body.MaxCompletionTokens != nil: + got = *body.MaxCompletionTokens + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "choices": [{"message": {"role": "assistant", "content": "All servers healthy."}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 10, "completion_tokens": 5, "total_tokens": 15} + }`)) + })) + defer srv.Close() + + g := NewGenerator(nil, &llmproxy.Config{ + Provider: "openai", + Model: "gpt-4o", + OpenAIAPIKey: "test-key", + OpenAIBaseURL: srv.URL, + MaxTokens: tc.configured, + }) + + if _, err := g.generateSummaryFromPrompt(context.Background(), "system", "data"); err != nil { + t.Fatalf("expected no error, got %v", err) + } + if got != tc.want { + t.Errorf("max tokens on the wire = %d, want %d", got, tc.want) + } + }) + } +} + +// TestGenerateSummaryFromPrompt_NoTextContentIsError verifies that a +// response carrying no usable text is reported as an error rather than +// rendered as a blank summary. A reasoning model produces exactly this when +// its thinking block exhausts the output budget. +func TestGenerateSummaryFromPrompt_NoTextContentIsError(t *testing.T) { + tests := []struct { + name string + content string + }{ + {name: "empty content", content: `""`}, + {name: "whitespace only", content: `" \n "`}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{ + "choices": [{"message": {"role": "assistant", "content": %s}, "finish_reason": "length"}], + "usage": {"prompt_tokens": 10, "completion_tokens": 512, "total_tokens": 522} + }`, tc.content) + })) + defer srv.Close() + + g := NewGenerator(nil, &llmproxy.Config{ + Provider: "openai", + Model: "gpt-4o", + OpenAIAPIKey: "test-key", + OpenAIBaseURL: srv.URL, + MaxTokens: 512, + }) + + summary, err := g.generateSummaryFromPrompt(context.Background(), "system", "data") + if err == nil { + t.Fatal("expected an error when the response carries no text") + } + if summary != "" { + t.Errorf("expected an empty summary on error, got %q", summary) + } + if !strings.Contains(err.Error(), "llm.max_tokens = 512") { + t.Errorf("expected the error to name the configured budget, got %v", err) + } + }) + } +} + func TestGenerateSummaryFromPrompt_ChatError(t *testing.T) { // A provider whose endpoint returns a non-retryable 4xx status makes // Chat fail immediately; the wrapped 'LLM chat failed' error must