From cc47377ac2ac35e07208fa8d068d374a7e2ef870 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 12 Aug 2026 12:53:37 +0100 Subject: [PATCH] Honour llm.max_tokens on the AI analysis paths The estate summary, the Server Info database analysis, and the alerter's tier 3 classification each hardcoded a 512-token output budget, so the configured llm.max_tokens had no bearing on any of them. A reasoning model spends part of its budget on an internal thinking block before it emits any answer text, and with a local model served over an OpenAI-compatible endpoint that small cap was consumed entirely by reasoning; the response then arrived with no text block at all, and because the callers treated the resulting empty string as a successful generation the failure surfaced as a blank panel with no error. Both server paths now take their budget from the shared llmproxy config via a new AnalysisMaxTokens helper, which reports the operator-configured value and falls back to 4096 when the setting is absent or non-positive. The alerter previously had no such setting at all, so it gains an llm.max_tokens option with the same default and fallback rule. A response that yields no usable text is now reported rather than rendered: the overview path returns an error naming the configured budget, the Server Info path logs and skips the cache so the next request retries, and the alerter's Classify returns an error, which its caller already handles by failing safe to an alert. Closes #399 --- alerter/src/internal/config/config.go | 39 ++++- alerter/src/internal/config/config_test.go | 27 ++++ alerter/src/internal/llm/llm.go | 12 +- alerter/src/internal/llm/reasoning.go | 32 +++- alerter/src/internal/llm/reasoning_test.go | 99 ++++++++++--- docs/changelog.md | 13 ++ docs/getting-started/configuration/alerter.md | 9 ++ docs/getting-started/configuration/server.md | 34 ++++- examples/ai-dba-alerter.yaml | 8 + examples/ai-dba-server.yaml | 6 +- .../src/internal/api/server_info_handlers.go | 19 ++- .../internal/api/server_info_handlers_test.go | 139 ++++++++++++++++++ server/src/internal/llmproxy/proxy.go | 21 +++ server/src/internal/llmproxy/proxy_test.go | 44 ++++++ server/src/internal/overview/generator.go | 20 ++- .../src/internal/overview/generator_test.go | 108 ++++++++++++++ 16 files changed, 580 insertions(+), 50 deletions(-) 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