Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
63 changes: 39 additions & 24 deletions cmd/opencodereview/config_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,17 +302,18 @@ func deleteCustomProvider(cfg *Config, name string) (bool, error) {

// ProviderEntry holds per-provider configuration in the providers map.
type ProviderEntry struct {
APIKey string `json:"api_key,omitempty"`
APIKeyCmd string `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
URL string `json:"url,omitempty"`
Protocol string `json:"protocol,omitempty"`
Model string `json:"model,omitempty"`
Models []string `json:"models,omitempty"`
AuthHeader string `json:"auth_header,omitempty"`
TimeoutSec int `json:"timeout_sec,omitempty"` // per-request HTTP timeout in seconds
ExtraBody map[string]any `json:"extra_body,omitempty"`
ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
RetryCodes []int `json:"retry_codes,omitempty"`
APIKey string `json:"api_key,omitempty"`
APIKeyCmd string `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
URL string `json:"url,omitempty"`
Protocol string `json:"protocol,omitempty"`
Model string `json:"model,omitempty"`
Models []string `json:"models,omitempty"`
AuthHeader string `json:"auth_header,omitempty"`
TimeoutSec int `json:"timeout_sec,omitempty"` // per-request HTTP timeout in seconds
ExtraBody map[string]any `json:"extra_body,omitempty"`
ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
RetryCodes []int `json:"retry_codes,omitempty"`
PromptCaching *bool `json:"prompt_caching,omitempty"`

// AWSProfile and AWSRegion pin the credentials and region for providers that
// authenticate from the AWS chain (bedrock). Both are optional — without
Expand Down Expand Up @@ -362,17 +363,18 @@ type Config struct {
}

type LlmConfig struct {
URL string `json:"url,omitempty"`
AuthToken string `json:"auth_token,omitempty"`
AuthTokenCmd string `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
AuthHeader string `json:"auth_header,omitempty"`
Model string `json:"model,omitempty"`
Protocol string `json:"protocol,omitempty"` // canonical protocol name; takes priority over UseAnthropic
UseAnthropic *bool `json:"use_anthropic,omitempty"` // nil = default true; false = OpenAI protocol (legacy fallback)
TimeoutSec int `json:"timeout_sec,omitempty"` // per-request HTTP timeout in seconds
ExtraBody map[string]any `json:"extra_body,omitempty"`
ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
RetryCodes []int `json:"retry_codes,omitempty"`
URL string `json:"url,omitempty"`
AuthToken string `json:"auth_token,omitempty"`
AuthTokenCmd string `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
AuthHeader string `json:"auth_header,omitempty"`
Model string `json:"model,omitempty"`
Protocol string `json:"protocol,omitempty"` // canonical protocol name; takes priority over UseAnthropic
UseAnthropic *bool `json:"use_anthropic,omitempty"` // nil = default true; false = OpenAI protocol (legacy fallback)
TimeoutSec int `json:"timeout_sec,omitempty"` // per-request HTTP timeout in seconds
ExtraBody map[string]any `json:"extra_body,omitempty"`
ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
RetryCodes []int `json:"retry_codes,omitempty"`
PromptCaching *bool `json:"prompt_caching,omitempty"`

unknownJSONFields map[string]json.RawMessage
}
Expand Down Expand Up @@ -620,6 +622,7 @@ var supportedConfigKeys = []string{
"llm.extra_body",
"llm.extra_headers",
"llm.retry_codes",
"llm.prompt_caching",
"language",
"telemetry.enabled",
"telemetry.exporter",
Expand Down Expand Up @@ -792,8 +795,14 @@ func setConfigValue(cfg *Config, key, value string) error {
fmt.Fprintf(os.Stderr, "[ocr] WARNING: %s\n", w)
}
cfg.Llm.RetryCodes = codes
case "llm.prompt_caching", "llm.PromptCaching":
b, err := strconv.ParseBool(value)
if err != nil {
return fmt.Errorf("invalid boolean for llm.prompt_caching: %w", err)
}
cfg.Llm.PromptCaching = &b
default:
return fmt.Errorf("unknown config key: %s\nSupported keys: %s\nProvider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, aws_region, aws_profile\nProtocol values: anthropic, anthropic-bedrock, openai, openai-responses\nMCP server fields: type, command, args, env, url, headers, tools, setup", key, strings.Join(supportedConfigKeys, ", "))
return fmt.Errorf("unknown config key: %s\nSupported keys: %s\nProvider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, prompt_caching, aws_region, aws_profile\nProtocol values: anthropic, anthropic-bedrock, openai, openai-responses\nMCP server fields: type, command, args, env, url, headers, tools, setup", key, strings.Join(supportedConfigKeys, ", "))
}
return nil
}
Expand Down Expand Up @@ -862,6 +871,12 @@ func applyProviderField(providerName string, entry *ProviderEntry, field, key, v
fmt.Fprintf(os.Stderr, "[ocr] WARNING: %s\n", w)
}
entry.RetryCodes = codes
case "prompt_caching":
b, err := strconv.ParseBool(value)
if err != nil {
return fmt.Errorf("invalid boolean for %s: %w", key, err)
}
entry.PromptCaching = &b
case "timeout_sec":
timeout, err := parseTimeoutSeconds(value)
if err != nil {
Expand All @@ -882,7 +897,7 @@ func applyProviderField(providerName string, entry *ProviderEntry, field, key, v
entry.AWSProfile = normalized
}
default:
return fmt.Errorf("unknown provider field %q: supported fields are api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, aws_region, aws_profile", field)
return fmt.Errorf("unknown provider field %q: supported fields are api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, prompt_caching, aws_region, aws_profile", field)
}
return nil
}
Expand Down
33 changes: 31 additions & 2 deletions cmd/opencodereview/config_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,35 @@ func TestSetConfigValueLlmExtraBodyInvalid(t *testing.T) {
}
}

func TestSetConfigValuePromptCaching(t *testing.T) {
t.Run("global setting", func(t *testing.T) {
cfg := &Config{}
if err := setConfigValue(cfg, "llm.prompt_caching", "false"); err != nil {
t.Fatalf("setConfigValue: %v", err)
}
if cfg.Llm.PromptCaching == nil || *cfg.Llm.PromptCaching {
t.Fatalf("PromptCaching = %v, want false", cfg.Llm.PromptCaching)
}
})

t.Run("provider override", func(t *testing.T) {
cfg := &Config{}
if err := setConfigValue(cfg, "providers.anthropic.prompt_caching", "true"); err != nil {
t.Fatalf("setConfigValue: %v", err)
}
got := cfg.Providers["anthropic"].PromptCaching
if got == nil || !*got {
t.Fatalf("PromptCaching = %v, want true", got)
}
})

t.Run("invalid boolean", func(t *testing.T) {
if err := setConfigValue(&Config{}, "llm.prompt_caching", "sometimes"); err == nil {
t.Fatal("expected invalid boolean error")
}
})
}

func TestSetConfigValueUnknownKey(t *testing.T) {
cfg := &Config{}
if err := setConfigValue(cfg, "unknown.key", "val"); err == nil {
Expand All @@ -1140,8 +1169,8 @@ func TestSetConfigValueUnknownKeyMessage(t *testing.T) {
t.Fatal("expected error for unknown key")
}
want := "unknown config key: bogus.key\n" +
"Supported keys: provider, model, max_tokens, effort, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_token_cmd, llm.auth_header, llm.model, llm.timeout_sec, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, llm.retry_codes, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\n" +
"Provider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, aws_region, aws_profile\n" +
"Supported keys: provider, model, max_tokens, effort, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_token_cmd, llm.auth_header, llm.model, llm.timeout_sec, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, llm.retry_codes, llm.prompt_caching, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\n" +
"Provider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, timeout_sec, extra_body, extra_headers, retry_codes, prompt_caching, aws_region, aws_profile\n" +
"Protocol values: anthropic, anthropic-bedrock, openai, openai-responses\n" +
"MCP server fields: type, command, args, env, url, headers, tools, setup"
if err.Error() != want {
Expand Down
31 changes: 20 additions & 11 deletions cmd/opencodereview/provider_tui.go
Original file line number Diff line number Diff line change
Expand Up @@ -1298,17 +1298,18 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) {
// original's slice or map fields.
func cloneProviderEntry(v ProviderEntry) ProviderEntry {
out := ProviderEntry{
APIKey: v.APIKey,
APIKeyCmd: v.APIKeyCmd,
URL: v.URL,
Protocol: v.Protocol,
Model: v.Model,
Models: append([]string(nil), v.Models...),
AuthHeader: v.AuthHeader,
TimeoutSec: v.TimeoutSec,
RetryCodes: append([]int(nil), v.RetryCodes...),
AWSProfile: v.AWSProfile,
AWSRegion: v.AWSRegion,
APIKey: v.APIKey,
APIKeyCmd: v.APIKeyCmd,
URL: v.URL,
Protocol: v.Protocol,
Model: v.Model,
Models: append([]string(nil), v.Models...),
AuthHeader: v.AuthHeader,
TimeoutSec: v.TimeoutSec,
RetryCodes: append([]int(nil), v.RetryCodes...),
PromptCaching: cloneBoolPointer(v.PromptCaching),
AWSProfile: v.AWSProfile,
AWSRegion: v.AWSRegion,
}
out.unknownJSONFields = cloneUnknownJSONFields(v.unknownJSONFields)
if v.ExtraBody != nil {
Expand All @@ -1326,6 +1327,14 @@ func cloneProviderEntry(v ProviderEntry) ProviderEntry {
return out
}

func cloneBoolPointer(v *bool) *bool {
if v == nil {
return nil
}
b := *v
return &b
}

func cloneUnknownJSONFields(src map[string]json.RawMessage) map[string]json.RawMessage {
if src == nil {
return nil
Expand Down
18 changes: 18 additions & 0 deletions cmd/opencodereview/provider_tui_customform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,24 @@ func TestCustomFormEndsAtProtocolForBedrock(t *testing.T) {
})
}

func TestApplyCreateCustomProvider_AnthropicInheritsPromptCaching(t *testing.T) {
path := filepath.Join(t.TempDir(), "config.json")
m := newProviderTUI(&Config{}, path)
m.activeTab = tabCustom
m.creatingCustom = true
m.cpProtocolIdx = cpProtocolIndex(llm.ProtocolAnthropic)
m.cpNameInput.SetValue("anthropic-custom")
m.cpURLInput.SetValue("https://api.example.com")
m.apiKeyInput.SetValue("test-key")
m.cpAuthInput.SetValue("authorization")

out, _ := m.applyCreateCustomProvider()
got := out.(providerTUIModel).existingCfg.CustomProviders["anthropic-custom"]
if got.PromptCaching != nil {
t.Fatalf("PromptCaching = %v, want nil so the global setting is inherited", got.PromptCaching)
}
}

// TestUpdateCustomProviderForm_ProtocolNav covers up/down protocol selection.
func TestUpdateCustomProviderForm_ProtocolNav(t *testing.T) {
m := newProviderTUI(&Config{}, "")
Expand Down
28 changes: 15 additions & 13 deletions cmd/opencodereview/provider_tui_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,20 +281,22 @@ func TestCloneProviderEntry_NilExtraBody(t *testing.T) {
// omission. It catches a dropped field, not an aliased one -- DeepEqual
// compares values, not identity; the sibling tests above cover aliasing.
func TestCloneProviderEntry_CopiesEveryField(t *testing.T) {
promptCaching := true
orig := ProviderEntry{
APIKey: "key",
APIKeyCmd: "op read op://dev/x/api-key",
URL: "http://localhost",
Protocol: "openai",
Model: "gpt-4",
Models: []string{"gpt-4"},
AuthHeader: "Authorization",
TimeoutSec: 45,
RetryCodes: []int{403},
ExtraBody: map[string]any{"temperature": 0.7},
ExtraHeaders: map[string]string{"X-Trace": "on"},
AWSRegion: "us-west-2",
AWSProfile: "example-profile",
APIKey: "key",
APIKeyCmd: "op read op://dev/x/api-key",
URL: "http://localhost",
Protocol: "openai",
Model: "gpt-4",
Models: []string{"gpt-4"},
AuthHeader: "Authorization",
TimeoutSec: 45,
RetryCodes: []int{403},
PromptCaching: &promptCaching,
ExtraBody: map[string]any{"temperature": 0.7},
ExtraHeaders: map[string]string{"X-Trace": "on"},
AWSRegion: "us-west-2",
AWSProfile: "example-profile",
unknownJSONFields: map[string]json.RawMessage{
"future_provider": json.RawMessage(`{"value":"preserve-me"}`),
},
Expand Down
42 changes: 27 additions & 15 deletions internal/llm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,14 +398,17 @@ type FunctionDef struct {

// ClientConfig holds configuration for connecting to an LLM service.
type ClientConfig struct {
URL string // Full API endpoint URL
APIKey string // Bearer token / API key
Model string // Default model override
AuthHeader string // Auth header name: "x-api-key", "authorization", or empty for protocol default
Timeout time.Duration // Request timeout
ExtraBody map[string]any // Vendor-specific fields merged into every request body
ExtraHeaders map[string]string // Extra HTTP headers sent with every request
RetryCodes []int // Additional HTTP status codes that trigger retry
URL string // Full API endpoint URL
APIKey string // Bearer token / API key
Model string // Default model override
AuthHeader string // Auth header name: "x-api-key", "authorization", or empty for protocol default
Timeout time.Duration // Request timeout
ExtraBody map[string]any // Vendor-specific fields merged into every request body
ExtraHeaders map[string]string // Extra HTTP headers sent with every request
RetryCodes []int // Additional HTTP status codes that trigger retry
// PromptCaching controls Anthropic cache_control breakpoints. Nil preserves
// the historical default of enabling prompt caching.
PromptCaching *bool
RequiresStreaming bool
RejectsSamplingParams bool
ChatGPTPlan bool
Expand Down Expand Up @@ -493,14 +496,15 @@ func NewLLMClient(ep ResolvedEndpoint, collector *RetryCollector, raw *RawHolder
ExtraBody: ep.ExtraBody,
ExtraHeaders: ep.ExtraHeaders,
RetryCodes: ep.RetryCodes,
RequiresStreaming: ep.RequiresStreaming,
RejectsSamplingParams: ep.RejectsSamplingParams,
ChatGPTPlan: ep.ChatGPTPlan,
ChatGPTAccount: ep.ChatGPTAccount,
PromptCaching: ep.PromptCaching,
retryCollector: collector,
rawHolder: raw,
AWSProfile: ep.AWSProfile,
AWSRegion: ep.AWSRegion,
RequiresStreaming: ep.RequiresStreaming,
RejectsSamplingParams: ep.RejectsSamplingParams,
ChatGPTPlan: ep.ChatGPTPlan,
ChatGPTAccount: ep.ChatGPTAccount,
}
switch ep.Protocol {
case ProtocolAnthropic:
Expand Down Expand Up @@ -1635,11 +1639,15 @@ func (c *AnthropicClient) buildAnthropicParams(model string, req ChatRequest) (a
}

if len(systemBlocks) > 0 {
systemBlocks[len(systemBlocks)-1].CacheControl = anthropic.NewCacheControlEphemeralParam()
if c.promptCachingEnabled() {
systemBlocks[len(systemBlocks)-1].CacheControl = anthropic.NewCacheControlEphemeralParam()
}
params.System = systemBlocks
}
if len(tools) > 0 {
tools[len(tools)-1].OfTool.CacheControl = anthropic.NewCacheControlEphemeralParam()
if c.promptCachingEnabled() {
tools[len(tools)-1].OfTool.CacheControl = anthropic.NewCacheControlEphemeralParam()
}
params.Tools = tools
if req.ToolChoice == "required" {
params.ToolChoice = anthropic.ToolChoiceUnionParam{
Expand All @@ -1649,7 +1657,7 @@ func (c *AnthropicClient) buildAnthropicParams(model string, req ChatRequest) (a
}
// Dynamic breakpoint on the latest message so multi-turn history is
// cached incrementally: read the full previous prefix, write only the delta.
if len(messages) > 0 {
if c.promptCachingEnabled() && len(messages) > 0 {
last := &messages[len(messages)-1]
if n := len(last.Content); n > 0 {
lastIdx := n - 1
Expand All @@ -1670,6 +1678,10 @@ func (c *AnthropicClient) buildAnthropicParams(model string, req ChatRequest) (a
return params, nil
}

func (c *AnthropicClient) promptCachingEnabled() bool {
return c.cfg.PromptCaching == nil || *c.cfg.PromptCaching
}

// cloneContentBlockParam deep-copies a content block via JSON round trip.
func cloneContentBlockParam(b anthropic.ContentBlockParamUnion) (anthropic.ContentBlockParamUnion, error) {
raw, err := json.Marshal(b)
Expand Down
32 changes: 32 additions & 0 deletions internal/llm/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,38 @@ func TestBuildAnthropicParams_CacheControl(t *testing.T) {
})
}

func TestBuildAnthropicParams_PromptCachingDisabled(t *testing.T) {
disabled := false
client := NewAnthropicClient(ClientConfig{URL: "https://api.anthropic.com", PromptCaching: &disabled})
params, err := client.buildAnthropicParams("claude-sonnet-4-20250514", ChatRequest{
Messages: []Message{
{Role: "system", Content: "Review this code."},
{Role: "user", Content: "Hello"},
},
Tools: []ToolDef{{Type: "function", Function: FunctionDef{Name: "tool", Parameters: map[string]any{"type": "object"}}}},
})
if err != nil {
t.Fatalf("buildAnthropicParams: %v", err)
}
if len(params.System) != 1 {
t.Fatalf("expected one system block, got %d", len(params.System))
}
if params.System[0].CacheControl.Type != "" {
t.Errorf("system CacheControl.Type = %q, want empty", params.System[0].CacheControl.Type)
}
if len(params.Tools) != 1 || params.Tools[0].OfTool == nil {
t.Fatalf("expected one tool block, got %#v", params.Tools)
}
if params.Tools[0].OfTool.CacheControl.Type != "" {
t.Errorf("tool CacheControl.Type = %q, want empty", params.Tools[0].OfTool.CacheControl.Type)
}
lastMessage := params.Messages[len(params.Messages)-1]
lastBlock := lastMessage.Content[len(lastMessage.Content)-1]
if lastBlock.OfText.CacheControl.Type != "" {
t.Errorf("message CacheControl.Type = %q, want empty", lastBlock.OfText.CacheControl.Type)
}
}

func TestBuildAnthropicParams_CacheControl_NoTools(t *testing.T) {
client := NewAnthropicClient(ClientConfig{URL: "https://api.anthropic.com"})

Expand Down
Loading
Loading