Refactor llm client timeout

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
+	// Timeout limits each request, including reading a streaming response.
-	// uses Go duration syntax and defaults to five minutes when empty.
+	// It defaults to five minutes when zero.
-	Timeout string `yaml:"timeout"`
+	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
+// Validate reports whether Timeout is non-negative and URL is non-empty.
-// non-empty. It does not contact the endpoint.
 func (cfg Config) Validate() error {
-	if cfg.Timeout != "" {
+	if cfg.Timeout < 0 {
-		if _, err := time.ParseDuration(cfg.Timeout); err != nil {
+		return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout)
-			return fmt.Errorf("invalid timeout: %w", err)
-		}
 	}
 	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
+// NewClient creates a client from cfg.
-// five-minute timeout. log must be non-nil; pass
-// slog.New(slog.DiscardHandler) to run without logs.
 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
+	timeout := cfg.Timeout
-	if cfg.Timeout != "" {
+	if timeout == 0 {
-		d, err := time.ParseDuration(cfg.Timeout)
+		timeout = defaultTimeout
-		if err != nil {
-			return nil, fmt.Errorf("parse timeout: %v", err)
-		}
-		timeout = d
 	}
 
 	return &Client{

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)
 			}
 		})
 	}

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)