diff --git a/internal/llm/client.go b/internal/llm/client.go index b77e732..bcc1fc8 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -13,7 +13,7 @@ const ( // chatTemplateEnableThinking is the chat-template argument that selects // whether a reasoning model emits reasoning tokens. chatTemplateEnableThinking = "enable_thinking" - // defaultTimeout limits requests when Config.Timeout is empty. + // defaultTimeout limits requests when Config.Timeout is zero. defaultTimeout = 5 * time.Minute // sseDone is the sentinel that terminates a chat completion stream. sseDone = "[DONE]" @@ -26,20 +26,17 @@ type Config struct { // Provider is returned unchanged by Client.Provider. It is not sent to the // inference endpoint. Provider string `yaml:"provider"` - // Timeout limits each request, including reading a streaming response. It - // uses Go duration syntax and defaults to five minutes when empty. - Timeout string `yaml:"timeout"` + // Timeout limits each request, including reading a streaming response. + // It defaults to five minutes when zero. + Timeout time.Duration `yaml:"timeout"` // URL is the endpoint root. Client appends /chat/completions or /models. URL string `yaml:"url"` } -// Validate reports whether Timeout is a Go duration, when set, and URL is -// non-empty. It does not contact the endpoint. +// Validate reports whether Timeout is non-negative and URL is non-empty. func (cfg Config) Validate() error { - if cfg.Timeout != "" { - if _, err := time.ParseDuration(cfg.Timeout); err != nil { - return fmt.Errorf("invalid timeout: %w", err) - } + if cfg.Timeout < 0 { + return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout) } if cfg.URL == "" { return fmt.Errorf("missing URL") @@ -57,9 +54,7 @@ type Client struct { timeout time.Duration } -// NewClient creates a client from cfg. If cfg.Timeout is empty, requests have a -// five-minute timeout. log must be non-nil; pass -// slog.New(slog.DiscardHandler) to run without logs. +// NewClient creates a client from cfg. func NewClient(cfg Config, log *slog.Logger) (*Client, error) { if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("invalid config: %w", err) @@ -68,13 +63,9 @@ func NewClient(cfg Config, log *slog.Logger) (*Client, error) { return nil, fmt.Errorf("missing logger") } - timeout := defaultTimeout - if cfg.Timeout != "" { - d, err := time.ParseDuration(cfg.Timeout) - if err != nil { - return nil, fmt.Errorf("parse timeout: %v", err) - } - timeout = d + timeout := cfg.Timeout + if timeout == 0 { + timeout = defaultTimeout } return &Client{ diff --git a/internal/llm/client_test.go b/internal/llm/client_test.go index e24fb22..650891b 100644 --- a/internal/llm/client_test.go +++ b/internal/llm/client_test.go @@ -3,6 +3,7 @@ package llm import ( "log/slog" "testing" + "time" ) func TestConfigValidate(t *testing.T) { @@ -22,10 +23,10 @@ func TestConfigValidate(t *testing.T) { wantErr: true, }, { - name: "invalid timeout", + name: "negative timeout", cfg: Config{ URL: "http://localhost:8080", - Timeout: "not-a-duration", + Timeout: -time.Second, }, wantErr: true, }, @@ -40,7 +41,7 @@ func TestConfigValidate(t *testing.T) { name: "valid config with timeout", cfg: Config{ URL: "http://localhost:8080", - Timeout: "15m", + Timeout: 15 * time.Minute, }, wantErr: false, }, @@ -48,7 +49,7 @@ func TestConfigValidate(t *testing.T) { name: "valid config with complex timeout", cfg: Config{ URL: "http://localhost:8080", - Timeout: "1h30m45s", + Timeout: time.Hour + 30*time.Minute + 45*time.Second, }, wantErr: false, }, @@ -56,7 +57,7 @@ func TestConfigValidate(t *testing.T) { name: "valid full config", cfg: Config{ Key: "sk-test-key", - Timeout: "30m", + Timeout: 30 * time.Minute, URL: "http://localhost:8080", }, wantErr: false, @@ -89,6 +90,14 @@ func TestNewClient(t *testing.T) { cfg: Config{}, wantErr: "invalid config: missing URL", }, + { + name: "negative timeout precedes nil logger", + cfg: Config{ + URL: "http://localhost:8080", + Timeout: -time.Second, + }, + wantErr: "invalid config: invalid timeout -1s: must not be negative", + }, { name: "nil logger", cfg: Config{ @@ -107,7 +116,7 @@ func TestNewClient(t *testing.T) { name: "valid config with timeout", cfg: Config{ URL: "http://localhost:8080", - Timeout: "10m", + Timeout: 10 * time.Minute, }, log: log, }, @@ -115,7 +124,7 @@ func TestNewClient(t *testing.T) { name: "valid config with all fields", cfg: Config{ Key: "test-key", - Timeout: "5m", + Timeout: 5 * time.Minute, URL: "http://localhost:8080", }, log: log, @@ -144,7 +153,14 @@ func TestNewClient(t *testing.T) { t.Fatalf("NewClient() error = %v, want nil", err) } if client == nil { - t.Error("NewClient() client = nil, want non-nil") + t.Fatal("NewClient() client = nil, want non-nil") + } + wantTimeout := tt.cfg.Timeout + if wantTimeout == 0 { + wantTimeout = 5 * time.Minute + } + if client.timeout != wantTimeout { + t.Errorf("timeout = %v, want %v", client.timeout, wantTimeout) } }) } diff --git a/internal/llm/llm_integration_test.go b/internal/llm/llm_integration_test.go index ad3fe1d..55f826a 100644 --- a/internal/llm/llm_integration_test.go +++ b/internal/llm/llm_integration_test.go @@ -5,6 +5,7 @@ import ( "log/slog" "os" "testing" + "time" ) // testURL returns the LLM endpoint for integration tests. @@ -41,7 +42,7 @@ func TestIntegration_ListModels(t *testing.T) { cfg := Config{ URL: testURL(t), Key: testKey(t), - Timeout: "30s", + Timeout: 30 * time.Second, } client, err := NewClient(cfg, slog.Default()) if err != nil { @@ -67,7 +68,7 @@ func TestIntegration_Completions(t *testing.T) { cfg := Config{ URL: testURL(t), Key: testKey(t), - Timeout: "60s", + Timeout: 60 * time.Second, } client, err := NewClient(cfg, slog.Default()) if err != nil { @@ -100,7 +101,7 @@ func TestIntegration_CompletionsStream(t *testing.T) { cfg := Config{ URL: testURL(t), Key: testKey(t), - Timeout: "60s", + Timeout: 60 * time.Second, } client, err := NewClient(cfg, slog.Default()) if err != nil { @@ -140,7 +141,7 @@ func TestIntegration_ToolCalling(t *testing.T) { client, err := NewClient(Config{ URL: testURL(t), Key: testKey(t), - Timeout: "60s", + Timeout: 60 * time.Second, }, slog.Default()) if err != nil { t.Fatalf("NewClient: %v", err)