Skip to content

Commit d17a2c8

Browse files
committed
refactor(provider): tighten a handful of helpers
Five small simplifications, no behaviour change. All touched functions remain at 100% line coverage and overall package coverage nudges from 96.9% to 98.2%. * mergeCloneOptions: apply every Opt to a single ModelOptions accumulator and read the final effective MaxTokens / NoThinking once, instead of allocating a fresh tempOpts per option and re-checking inside the loop. Later-opt-wins semantics are now an obvious property of the accumulator rather than a property of "the last loop iteration that set the field". * resolveEffectiveProvider: replace the if/else fallback to "openai" with cmp.Or(cfg.Provider, "openai"). * isOpenAICompatibleProvider: drop the default: arm so that the alias fallthrough sits at function scope, removing one indentation level and a redundant return false. * resolveProviderType: drop the cfg.ProviderOpts != nil guard. Reading from a nil map is well-defined and yields the zero value, which the existing type-assertion already handles. * mergeFromProviderConfig: reuse setProviderOptIfAbsent for the ProviderOpts merge loop instead of inlining the lazy-init + has-key dance, which already exists in that helper. Assisted-By: docker-agent
1 parent 304f4e8 commit d17a2c8

2 files changed

Lines changed: 20 additions & 32 deletions

File tree

pkg/model/provider/clone.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,19 @@ func mergeCloneOptions(cfg base.Config, opts []options.Opt) (latest.ModelConfig,
4242
baseOpts := options.FromModelOptions(cfg.ModelOptions)
4343
mergedOpts := append(baseOpts, opts...)
4444

45-
// Apply max_tokens override if present in options
46-
// We need to apply it to the ModelConfig itself since that's what providers use
47-
// Only update MaxTokens if an option explicitly sets it (non-zero value)
48-
modelConfig := cfg.ModelConfig
45+
// Apply every option to a single accumulator so we can read the final
46+
// effective values directly. "Later opt wins" semantics fall out naturally.
47+
var merged options.ModelOptions
4948
for _, opt := range mergedOpts {
50-
tempOpts := &options.ModelOptions{}
51-
opt(tempOpts)
52-
if mt := tempOpts.MaxTokens(); mt != 0 {
53-
modelConfig.MaxTokens = &mt
54-
}
55-
if tempOpts.NoThinking() {
56-
modelConfig.ThinkingBudget = nil
57-
}
49+
opt(&merged)
5850
}
5951

52+
modelConfig := cfg.ModelConfig
53+
if mt := merged.MaxTokens(); mt != 0 {
54+
modelConfig.MaxTokens = &mt
55+
}
56+
if merged.NoThinking() {
57+
modelConfig.ThinkingBudget = nil
58+
}
6059
return modelConfig, mergedOpts
6160
}

pkg/model/provider/defaults.go

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package provider
22

33
import (
4+
"cmp"
45
"log/slog"
56
"maps"
67
"strings"
@@ -14,11 +15,10 @@ import (
1415

1516
// resolveProviderType determines the effective API type for a config.
1617
// Priority: ProviderOpts["api_type"] > built-in alias > provider name.
18+
// Reading from a nil ProviderOpts map is safe and yields the zero value.
1719
func resolveProviderType(cfg *latest.ModelConfig) string {
18-
if cfg.ProviderOpts != nil {
19-
if apiType, ok := cfg.ProviderOpts["api_type"].(string); ok && apiType != "" {
20-
return apiType
21-
}
20+
if apiType, ok := cfg.ProviderOpts["api_type"].(string); ok && apiType != "" {
21+
return apiType
2222
}
2323
if alias, exists := LookupAlias(cfg.Provider); exists && alias.APIType != "" {
2424
return alias.APIType
@@ -29,24 +29,18 @@ func resolveProviderType(cfg *latest.ModelConfig) string {
2929
// resolveEffectiveProvider returns the effective provider type for a ProviderConfig.
3030
// If Provider is explicitly set, returns that. Otherwise returns "openai" (backward compat).
3131
func resolveEffectiveProvider(cfg latest.ProviderConfig) string {
32-
if cfg.Provider != "" {
33-
return cfg.Provider
34-
}
35-
return "openai"
32+
return cmp.Or(cfg.Provider, "openai")
3633
}
3734

3835
// isOpenAICompatibleProvider returns true if the provider type uses the OpenAI API protocol.
3936
func isOpenAICompatibleProvider(providerType string) bool {
4037
switch providerType {
4138
case "openai", "openai_chatcompletions", "openai_responses":
4239
return true
43-
default:
44-
// Check if it's an alias that maps to openai
45-
if alias, exists := LookupAlias(providerType); exists {
46-
return alias.APIType == "openai"
47-
}
48-
return false
4940
}
41+
// Otherwise, the type is OpenAI-compatible iff it's an alias that maps to OpenAI.
42+
alias, exists := LookupAlias(providerType)
43+
return exists && alias.APIType == "openai"
5044
}
5145

5246
// ---------------------------------------------------------------------------
@@ -114,12 +108,7 @@ func mergeFromProviderConfig(dst *latest.ModelConfig, src latest.ProviderConfig)
114108

115109
// Merge provider_opts from provider config (model opts take precedence).
116110
for k, v := range src.ProviderOpts {
117-
if dst.ProviderOpts == nil {
118-
dst.ProviderOpts = make(map[string]any)
119-
}
120-
if _, has := dst.ProviderOpts[k]; !has {
121-
dst.ProviderOpts[k] = v
122-
}
111+
setProviderOptIfAbsent(dst, k, v)
123112
}
124113

125114
// Set api_type in ProviderOpts if not already set.

0 commit comments

Comments
 (0)