Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions README.md
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I bother you with the addition of these lines into other READMEs? AI generated translation is fine.

Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ picoclaw onboard
{
"model_name": "gpt4",
"model": "openai/gpt-5.2",
"api_key": "your-api-key"
"api_key": "your-api-key",
"request_timeout": 300
},
{
"model_name": "claude-sonnet-4.6",
Expand Down Expand Up @@ -262,6 +263,7 @@ picoclaw onboard
```

> **New**: The `model_list` configuration format allows zero-code provider addition. See [Model Configuration](#model-configuration-model_list) for details.
> `request_timeout` is optional and uses seconds. If omitted or set to `<= 0`, PicoClaw uses the default timeout (120s).

**3. Get API Keys**

Expand Down Expand Up @@ -915,7 +917,8 @@ This design also enables **multi-agent support** with flexible provider selectio
"model_name": "my-custom-model",
"model": "openai/custom-model",
"api_base": "https://my-proxy.com/v1",
"api_key": "sk-..."
"api_key": "sk-...",
"request_timeout": 300
}
```

Expand Down
7 changes: 5 additions & 2 deletions README.zh.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ picoclaw onboard
{
"model_name": "gpt4",
"model": "openai/gpt-5.2",
"api_key": "your-api-key"
"api_key": "your-api-key",
"request_timeout": 300
},
{
"model_name": "claude-sonnet-4.6",
Expand Down Expand Up @@ -263,6 +264,7 @@ picoclaw onboard
```

> **新功能**: `model_list` 配置格式支持零代码添加 provider。详见[模型配置](#模型配置-model_list)章节。
> `request_timeout` 为可选项,单位为秒。若省略或设置为 `<= 0`,PicoClaw 使用默认超时(120 秒)。

**3. 获取 API Key**

Expand Down Expand Up @@ -550,7 +552,8 @@ Agent 读取 HEARTBEAT.md
"model_name": "my-custom-model",
"model": "openai/custom-model",
"api_base": "https://my-proxy.com/v1",
"api_key": "sk-..."
"api_key": "sk-...",
"request_timeout": 300
}
```

Expand Down
1 change: 1 addition & 0 deletions docs/migration/model-list-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ The `model` field uses a protocol prefix format: `[protocol/]model-identifier`
| `connect_mode` | No | Connection mode for CLI providers: `stdio`, `grpc` |
| `rpm` | No | Requests per minute limit |
| `max_tokens_field` | No | Field name for max tokens |
| `request_timeout` | No | HTTP request timeout in seconds; `<=0` uses default `120s` |

*`api_key` is required for HTTP-based protocols unless `api_base` points to a local server.

Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ type ModelConfig struct {
// Optional optimizations
RPM int `json:"rpm,omitempty"` // Requests per minute limit
MaxTokensField string `json:"max_tokens_field,omitempty"` // Field name for max tokens (e.g., "max_completion_tokens")
RequestTimeout int `json:"request_timeout,omitempty"`
}

// Validate checks if the ModelConfig has all required fields.
Expand Down
35 changes: 35 additions & 0 deletions pkg/config/model_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,38 @@ func TestConfig_ValidateModelList(t *testing.T) {
})
}
}

func TestModelConfig_RequestTimeoutParsing(t *testing.T) {
jsonData := `{
"model_name": "slow-local",
"model": "openai/local-model",
"api_base": "http://localhost:11434/v1",
"request_timeout": 300
}`

var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}

if cfg.RequestTimeout != 300 {
t.Fatalf("RequestTimeout = %d, want 300", cfg.RequestTimeout)
}
}

func TestModelConfig_RequestTimeoutDefaultZeroValue(t *testing.T) {
jsonData := `{
"model_name": "default-timeout",
"model": "openai/gpt-4o",
"api_key": "test-key"
}`

var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}

if cfg.RequestTimeout != 0 {
t.Fatalf("RequestTimeout = %d, want 0", cfg.RequestTimeout)
}
}
24 changes: 21 additions & 3 deletions pkg/providers/factory_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,13 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
if apiBase == "" {
apiBase = getDefaultAPIBase(protocol)
}
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
return NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(
cfg.APIKey,
apiBase,
cfg.Proxy,
cfg.MaxTokensField,
cfg.RequestTimeout,
), modelID, nil

case "openrouter", "groq", "zhipu", "gemini", "nvidia",
"ollama", "moonshot", "shengsuanyun", "deepseek", "cerebras",
Expand All @@ -97,7 +103,13 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
if apiBase == "" {
apiBase = getDefaultAPIBase(protocol)
}
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
return NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(
cfg.APIKey,
apiBase,
cfg.Proxy,
cfg.MaxTokensField,
cfg.RequestTimeout,
), modelID, nil

case "anthropic":
if cfg.AuthMethod == "oauth" || cfg.AuthMethod == "token" {
Expand All @@ -116,7 +128,13 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
if cfg.APIKey == "" {
return nil, "", fmt.Errorf("api_key is required for anthropic protocol (model: %s)", cfg.Model)
}
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
return NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(
cfg.APIKey,
apiBase,
cfg.Proxy,
cfg.MaxTokensField,
cfg.RequestTimeout,
), modelID, nil

case "antigravity":
return NewAntigravityProvider(), modelID, nil
Expand Down
43 changes: 43 additions & 0 deletions pkg/providers/factory_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
package providers

import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/sipeed/picoclaw/pkg/config"
)
Expand Down Expand Up @@ -247,3 +251,42 @@ func TestCreateProviderFromConfig_EmptyModel(t *testing.T) {
t.Fatal("CreateProviderFromConfig() expected error for empty model")
}
}

func TestCreateProviderFromConfig_RequestTimeoutPropagation(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(1500 * time.Millisecond)
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"ok"},"finish_reason":"stop"}]}`))
}))
defer server.Close()

cfg := &config.ModelConfig{
ModelName: "test-timeout",
Model: "openai/gpt-4o",
APIBase: server.URL,
RequestTimeout: 1,
}

provider, modelID, err := CreateProviderFromConfig(cfg)
if err != nil {
t.Fatalf("CreateProviderFromConfig() error = %v", err)
}
if modelID != "gpt-4o" {
t.Fatalf("modelID = %q, want %q", modelID, "gpt-4o")
}

_, err = provider.Chat(
t.Context(),
[]Message{{Role: "user", Content: "hi"}},
nil,
modelID,
nil,
)
if err == nil {
t.Fatal("Chat() expected timeout error, got nil")
}
errMsg := err.Error()
if !strings.Contains(errMsg, "context deadline exceeded") && !strings.Contains(errMsg, "Client.Timeout exceeded") {
t.Fatalf("Chat() error = %q, want timeout-related error", errMsg)
}
}
15 changes: 14 additions & 1 deletion pkg/providers/http_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,21 @@ func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider {
}

func NewHTTPProviderWithMaxTokensField(apiKey, apiBase, proxy, maxTokensField string) *HTTPProvider {
return NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(apiKey, apiBase, proxy, maxTokensField, 0)
}

func NewHTTPProviderWithMaxTokensFieldAndRequestTimeout(
apiKey, apiBase, proxy, maxTokensField string,
requestTimeoutSeconds int,
) *HTTPProvider {
return &HTTPProvider{
delegate: openai_compat.NewProviderWithMaxTokensField(apiKey, apiBase, proxy, maxTokensField),
delegate: openai_compat.NewProviderWithMaxTokensFieldAndTimeout(
apiKey,
apiBase,
proxy,
maxTokensField,
requestTimeoutSeconds,
),
}
}

Expand Down
16 changes: 15 additions & 1 deletion pkg/providers/openai_compat/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,27 @@ type Provider struct {
httpClient *http.Client
}

const defaultRequestTimeout = 120 * time.Second

func NewProvider(apiKey, apiBase, proxy string) *Provider {
return NewProviderWithMaxTokensField(apiKey, apiBase, proxy, "")
}

func NewProviderWithMaxTokensField(apiKey, apiBase, proxy, maxTokensField string) *Provider {
return NewProviderWithMaxTokensFieldAndTimeout(apiKey, apiBase, proxy, maxTokensField, 0)
}

func NewProviderWithMaxTokensFieldAndTimeout(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think more idiomatic go would use the Functional Options pattern to configure the behaviour of a new struct, as it would make adding options easier.

This can be addressed as a future improvement and should not block merging this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, I'm wishing for something like the following, please consider below to be pseudo code.

  func WithMaxTokensField(field string) Option {
      return func(p *Provider) {
          p.maxTokensField = field
      }
  }

  func WithRequestTimeout(timeout time.Duration) Option {
      return func(p *Provider) {
          p.httpClient.Timeout = timeout
      }
  }

apiKey, apiBase, proxy, maxTokensField string,
requestTimeoutSeconds int,
) *Provider {
timeout := defaultRequestTimeout
if requestTimeoutSeconds > 0 {
timeout = time.Duration(requestTimeoutSeconds) * time.Second
}

client := &http.Client{
Timeout: 120 * time.Second,
Timeout: timeout,
}

if proxy != "" {
Expand Down
15 changes: 15 additions & 0 deletions pkg/providers/openai_compat/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http/httptest"
"net/url"
"testing"
"time"
)

func TestProviderChat_UsesMaxCompletionTokensForGLM(t *testing.T) {
Expand Down Expand Up @@ -325,3 +326,17 @@ func TestNormalizeModel_UsesAPIBase(t *testing.T) {
t.Fatalf("normalizeModel(openrouter) = %q, want %q", got, "openrouter/auto")
}
}

func TestProvider_RequestTimeoutDefault(t *testing.T) {
p := NewProviderWithMaxTokensFieldAndTimeout("key", "https://example.com/v1", "", "", 0)
if p.httpClient.Timeout != defaultRequestTimeout {
t.Fatalf("http timeout = %v, want %v", p.httpClient.Timeout, defaultRequestTimeout)
}
}

func TestProvider_RequestTimeoutOverride(t *testing.T) {
p := NewProviderWithMaxTokensFieldAndTimeout("key", "https://example.com/v1", "", "", 300)
if p.httpClient.Timeout != 300*time.Second {
t.Fatalf("http timeout = %v, want %v", p.httpClient.Timeout, 300*time.Second)
}
}