diff options
| -rw-r--r-- | internal/llm/anthropic.go | 4 | ||||
| -rw-r--r-- | internal/llm/ollama.go | 4 | ||||
| -rw-r--r-- | internal/llm/openai.go | 4 | ||||
| -rw-r--r-- | internal/llm/openrouter.go | 4 | ||||
| -rw-r--r-- | internal/llm/policy/policy.go | 68 | ||||
| -rw-r--r-- | internal/llm/policy/policy_test.go | 67 | ||||
| -rw-r--r-- | internal/llm/provider.go | 2 | ||||
| -rw-r--r-- | internal/llm/resilience.go | 18 | ||||
| -rw-r--r-- | internal/llm/yousearch.go | 6 |
9 files changed, 164 insertions, 13 deletions
diff --git a/internal/llm/anthropic.go b/internal/llm/anthropic.go index 3f7a616..00af17a 100644 --- a/internal/llm/anthropic.go +++ b/internal/llm/anthropic.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -116,7 +117,8 @@ func newAnthropicWithTimeout(baseURL, model, apiKey string, defaultTemp *float64 model = "claude-3-5-sonnet-20240620" } if timeoutSec <= 0 { - timeoutSec = 30 + // Fall back to the shared default chat timeout from the policy package. + timeoutSec = policy.DefaultRequestTimeoutSeconds } return anthropicClient{ httpClient: &http.Client{Timeout: time.Duration(timeoutSec) * time.Second}, diff --git a/internal/llm/ollama.go b/internal/llm/ollama.go index d8f911f..83823bc 100644 --- a/internal/llm/ollama.go +++ b/internal/llm/ollama.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -79,7 +80,8 @@ func newOllamaWithTimeout(baseURL, model, apiKey string, defaultTemp *float64, t model = "gemma4:31b-cloud" } if timeoutSec <= 0 { - timeoutSec = 30 + // Fall back to the shared default chat timeout from the policy package. + timeoutSec = policy.DefaultRequestTimeoutSeconds } return ollamaClient{ httpClient: &http.Client{Timeout: time.Duration(timeoutSec) * time.Second}, diff --git a/internal/llm/openai.go b/internal/llm/openai.go index d475de0..983723a 100644 --- a/internal/llm/openai.go +++ b/internal/llm/openai.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -125,7 +126,8 @@ func newOpenAIWithTimeout(baseURL, model, apiKey string, defaultTemp *float64, t model = "gpt-4.1" } if timeoutSec <= 0 { - timeoutSec = 30 + // Fall back to the shared default chat timeout from the policy package. + timeoutSec = policy.DefaultRequestTimeoutSeconds } return openAIClient{ httpClient: &http.Client{Timeout: time.Duration(timeoutSec) * time.Second}, diff --git a/internal/llm/openrouter.go b/internal/llm/openrouter.go index d728275..970ec29 100644 --- a/internal/llm/openrouter.go +++ b/internal/llm/openrouter.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -56,7 +57,8 @@ func newOpenRouterWithTimeout(baseURL, model, apiKey string, defaultTemp *float6 model = "openrouter/auto" } if timeoutSec <= 0 { - timeoutSec = 30 + // Fall back to the shared default chat timeout from the policy package. + timeoutSec = policy.DefaultRequestTimeoutSeconds } return openRouterClient{ httpClient: &http.Client{Timeout: time.Duration(timeoutSec) * time.Second}, diff --git a/internal/llm/policy/policy.go b/internal/llm/policy/policy.go new file mode 100644 index 0000000..e133e27 --- /dev/null +++ b/internal/llm/policy/policy.go @@ -0,0 +1,68 @@ +// Package policy centralizes the timeout, retry, and circuit-breaker tuning +// values used throughout the LLM HTTP call path. Previously these durations and +// counts were scattered as bare literals across each provider constructor and +// across resilience.go / circuitbreaker.go, which made the operational policy +// hard to discover and easy to drift out of sync. +// +// Consolidating them here gives a single, documented source of truth: anyone +// adjusting how aggressively Hexai times out, retries, or sheds load against an +// upstream LLM API only has to look in one place, and every provider inherits +// the same defaults automatically. +package policy + +import "time" + +// Request timeouts. +// +// DefaultRequestTimeout is the per-request HTTP client timeout applied to chat +// providers (Anthropic, OpenAI, OpenRouter, Ollama) when the user has not +// configured an explicit RequestTimeout. Thirty seconds is generous enough for +// normal completions yet short enough that a hung connection fails fast rather +// than blocking interactive use indefinitely. +// +// ResearchRequestTimeout applies to the You.com Research provider, whose +// multi-step research pipeline routinely runs much longer than a single chat +// completion. It therefore gets a substantially larger default so legitimate +// long-running research is not cut off prematurely. +const ( + DefaultRequestTimeout = 30 * time.Second + ResearchRequestTimeout = 120 * time.Second +) + +// DefaultRequestTimeoutSeconds and ResearchRequestTimeoutSeconds expose the +// timeouts as whole seconds, matching the int-seconds shape that provider +// constructors and the user-facing Config.RequestTimeout field use. +const ( + DefaultRequestTimeoutSeconds = int(DefaultRequestTimeout / time.Second) + ResearchRequestTimeoutSeconds = int(ResearchRequestTimeout / time.Second) +) + +// Retry policy. +// +// These govern how transient LLM HTTP failures (network resets, 5xx, 429 rate +// limits) are retried with exponential backoff plus jitter. The defaults are +// intentionally conservative: a handful of fast attempts that smooth over brief +// upstream blips without hammering the API or stalling interactive use. +// +// - RetryMaxAttempts: total tries including the first; <=1 disables retries. +// - RetryBaseDelay: delay before the first retry; doubles each subsequent attempt. +// - RetryMaxDelay: upper bound on any single backoff so growth stays bounded. +// - RetryJitterFraction: +/- fraction of random jitter to avoid thundering-herd retries. +const ( + RetryMaxAttempts = 3 + RetryBaseDelay = 200 * time.Millisecond + RetryMaxDelay = 2 * time.Second + RetryJitterFraction = 0.2 +) + +// Circuit breaker. +// +// The shared circuit breaker guards the whole LLM HTTP path. After +// CircuitFailureThreshold consecutive transient failures it trips open and +// rejects requests for CircuitCooldown, giving an unhealthy upstream time to +// recover instead of forcing every caller to wait out full retry+timeout +// cycles. +const ( + CircuitFailureThreshold = 5 + CircuitCooldown = 30 * time.Second +) diff --git a/internal/llm/policy/policy_test.go b/internal/llm/policy/policy_test.go new file mode 100644 index 0000000..976b41e --- /dev/null +++ b/internal/llm/policy/policy_test.go @@ -0,0 +1,67 @@ +package policy + +import ( + "testing" + "time" +) + +// TestRequestTimeouts verifies the documented default and research timeouts and +// that their whole-second mirrors stay in sync with the duration constants. +func TestRequestTimeouts(t *testing.T) { + if DefaultRequestTimeout != 30*time.Second { + t.Fatalf("DefaultRequestTimeout = %v, want 30s", DefaultRequestTimeout) + } + if ResearchRequestTimeout != 120*time.Second { + t.Fatalf("ResearchRequestTimeout = %v, want 120s", ResearchRequestTimeout) + } + if DefaultRequestTimeoutSeconds != 30 { + t.Fatalf("DefaultRequestTimeoutSeconds = %d, want 30", DefaultRequestTimeoutSeconds) + } + if ResearchRequestTimeoutSeconds != 120 { + t.Fatalf("ResearchRequestTimeoutSeconds = %d, want 120", ResearchRequestTimeoutSeconds) + } + // The integer mirrors must equal the duration constants converted to seconds. + if got := int(DefaultRequestTimeout / time.Second); got != DefaultRequestTimeoutSeconds { + t.Fatalf("DefaultRequestTimeoutSeconds out of sync: %d vs %d", DefaultRequestTimeoutSeconds, got) + } + if got := int(ResearchRequestTimeout / time.Second); got != ResearchRequestTimeoutSeconds { + t.Fatalf("ResearchRequestTimeoutSeconds out of sync: %d vs %d", ResearchRequestTimeoutSeconds, got) + } + // Research must allow a longer window than a normal chat completion. + if ResearchRequestTimeout <= DefaultRequestTimeout { + t.Fatalf("ResearchRequestTimeout (%v) must exceed DefaultRequestTimeout (%v)", ResearchRequestTimeout, DefaultRequestTimeout) + } +} + +// TestRetryPolicyConstants verifies the retry tuning values stay within sane, +// documented bounds. +func TestRetryPolicyConstants(t *testing.T) { + if RetryMaxAttempts != 3 { + t.Fatalf("RetryMaxAttempts = %d, want 3", RetryMaxAttempts) + } + if RetryBaseDelay != 200*time.Millisecond { + t.Fatalf("RetryBaseDelay = %v, want 200ms", RetryBaseDelay) + } + if RetryMaxDelay != 2*time.Second { + t.Fatalf("RetryMaxDelay = %v, want 2s", RetryMaxDelay) + } + if RetryBaseDelay > RetryMaxDelay { + t.Fatalf("RetryBaseDelay (%v) must not exceed RetryMaxDelay (%v)", RetryBaseDelay, RetryMaxDelay) + } + if RetryJitterFraction <= 0 || RetryJitterFraction >= 1 { + t.Fatalf("RetryJitterFraction = %v, want in (0,1)", RetryJitterFraction) + } +} + +// TestCircuitBreakerConstants verifies the circuit-breaker tuning values. +func TestCircuitBreakerConstants(t *testing.T) { + if CircuitFailureThreshold != 5 { + t.Fatalf("CircuitFailureThreshold = %d, want 5", CircuitFailureThreshold) + } + if CircuitCooldown != 30*time.Second { + t.Fatalf("CircuitCooldown = %v, want 30s", CircuitCooldown) + } + if CircuitFailureThreshold <= 0 { + t.Fatalf("CircuitFailureThreshold must be positive to trip the breaker") + } +} diff --git a/internal/llm/provider.go b/internal/llm/provider.go index dbfa603..27d8cc2 100644 --- a/internal/llm/provider.go +++ b/internal/llm/provider.go @@ -75,7 +75,7 @@ func WithStop(stop ...string) RequestOption { // Config defines provider configuration read from the Hexai config file. type Config struct { Provider string - RequestTimeout int // seconds; 0 means use default (30s) + RequestTimeout int // seconds; 0 means use the provider default (see internal/llm/policy) // OpenAI options OpenAIBaseURL string OpenAIModel string diff --git a/internal/llm/resilience.go b/internal/llm/resilience.go index 4d82153..ea6a32b 100644 --- a/internal/llm/resilience.go +++ b/internal/llm/resilience.go @@ -9,6 +9,7 @@ import ( "net/http" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -37,15 +38,17 @@ type retryPolicy struct { randFloat func() float64 } -// defaultRetryPolicy returns the policy used for all LLM HTTP calls. Three -// total attempts (two retries) with 200ms base backoff keeps recovery fast for +// defaultRetryPolicy returns the policy used for all LLM HTTP calls. The +// concrete tuning values (attempts, backoff, jitter) live in the policy package +// so the whole resilience policy has a single source of truth. Three total +// attempts (two retries) with a 200ms base backoff keeps recovery fast for // transient errors while staying well within typical request timeouts. func defaultRetryPolicy() retryPolicy { return retryPolicy{ - maxAttempts: 3, - baseDelay: 200 * time.Millisecond, - maxDelay: 2 * time.Second, - jitterFraction: 0.2, + maxAttempts: policy.RetryMaxAttempts, + baseDelay: policy.RetryBaseDelay, + maxDelay: policy.RetryMaxDelay, + jitterFraction: policy.RetryJitterFraction, sleep: sleepWithContext, randFloat: rand.Float64, } @@ -54,7 +57,8 @@ func defaultRetryPolicy() retryPolicy { // sharedBreaker guards the whole LLM HTTP path. It trips after several // consecutive transient failures and stays open briefly so a single unhealthy // upstream does not cause every caller to wait out full retry+timeout cycles. -var sharedBreaker = newCircuitBreaker(5, 30*time.Second) +// Its threshold and cooldown come from the policy package. +var sharedBreaker = newCircuitBreaker(policy.CircuitFailureThreshold, policy.CircuitCooldown) // sleepWithContext sleeps for d unless ctx is cancelled first, in which case it // returns ctx.Err(). This keeps backoff waits responsive to cancellation and diff --git a/internal/llm/yousearch.go b/internal/llm/yousearch.go index 7539e8d..66cfb35 100644 --- a/internal/llm/yousearch.go +++ b/internal/llm/yousearch.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "codeberg.org/snonux/hexai/internal/llm/policy" "codeberg.org/snonux/hexai/internal/logging" ) @@ -52,7 +53,10 @@ func youSearchProviderFactory(cfg Config, keys ProviderKeys) (Client, error) { } timeoutSec := cfg.RequestTimeout if timeoutSec <= 0 { - timeoutSec = 120 + // The Research API runs a long multi-step pipeline, so it uses the + // larger research timeout from the policy package rather than the + // default chat timeout. + timeoutSec = policy.ResearchRequestTimeoutSeconds } return youSearchClient{ httpClient: &http.Client{Timeout: time.Duration(timeoutSec) * time.Second}, |
