fix: deduplicate getDefaultAPIBase and add empty apiBase validation
Export GetDefaultAPIBase from pkg/providers and reuse it in STT resolution instead of the duplicated getDefaultSTTBase. Skip model configs with unknown protocols that resolve to an empty API base. Resolves Copilot review comments on PR #635.
This commit is contained in:
parent
05219a38e3
commit
d01a874c17
4 changed files with 30 additions and 31 deletions
|
|
@ -241,14 +241,3 @@ func setupCronTool(
|
||||||
|
|
||||||
return cronService
|
return cronService
|
||||||
}
|
}
|
||||||
|
|
||||||
func getDefaultSTTBase(protocol string) string {
|
|
||||||
switch protocol {
|
|
||||||
case "openai":
|
|
||||||
return "https://api.openai.com/v1"
|
|
||||||
case "groq":
|
|
||||||
return "https://api.groq.com/openai/v1"
|
|
||||||
default:
|
|
||||||
return ""
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -20,7 +20,10 @@ func resolveSTTTranscriber(cfg *config.Config) voice.Transcriber {
|
||||||
protocol, modelID := providers.ExtractProtocol(mc.Model)
|
protocol, modelID := providers.ExtractProtocol(mc.Model)
|
||||||
apiBase := mc.APIBase
|
apiBase := mc.APIBase
|
||||||
if apiBase == "" {
|
if apiBase == "" {
|
||||||
apiBase = getDefaultSTTBase(protocol)
|
apiBase = providers.GetDefaultAPIBase(protocol)
|
||||||
|
}
|
||||||
|
if apiBase == "" {
|
||||||
|
continue // unknown protocol, skip this entry
|
||||||
}
|
}
|
||||||
return voice.NewOpenAICompatTranscriber(mc.APIKey, apiBase, modelID)
|
return voice.NewOpenAICompatTranscriber(mc.APIKey, apiBase, modelID)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,22 +7,28 @@ import (
|
||||||
"github.com/sipeed/picoclaw/pkg/voice"
|
"github.com/sipeed/picoclaw/pkg/voice"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestGetDefaultSTTBase(t *testing.T) {
|
func TestResolveSTTTranscriber_UnknownProtocolSkipped(t *testing.T) {
|
||||||
tests := []struct {
|
// stt_model points to an entry with an unknown protocol and no api_base.
|
||||||
protocol string
|
// The unknown protocol resolves to an empty API base, so this entry must be
|
||||||
expected string
|
// skipped (continue) and the function should fall through to return nil.
|
||||||
}{
|
cfg := &config.Config{
|
||||||
{"openai", "https://api.openai.com/v1"},
|
Agents: config.AgentsConfig{
|
||||||
{"groq", "https://api.groq.com/openai/v1"},
|
Defaults: config.AgentDefaults{
|
||||||
{"unknown", ""},
|
STTModel: "whisper",
|
||||||
{"", ""},
|
},
|
||||||
|
},
|
||||||
|
ModelList: []config.ModelConfig{
|
||||||
|
{
|
||||||
|
ModelName: "whisper",
|
||||||
|
Model: "unknownprotocol/whisper-1",
|
||||||
|
APIKey: "sk-test",
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
|
||||||
t.Run(tt.protocol, func(t *testing.T) {
|
tr := resolveSTTTranscriber(cfg)
|
||||||
if got := getDefaultSTTBase(tt.protocol); got != tt.expected {
|
if tr != nil {
|
||||||
t.Errorf("getDefaultSTTBase(%q) = %q, want %q", tt.protocol, got, tt.expected)
|
t.Error("expected nil - unknown protocol with no api_base should be skipped")
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -82,7 +82,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
||||||
}
|
}
|
||||||
apiBase := cfg.APIBase
|
apiBase := cfg.APIBase
|
||||||
if apiBase == "" {
|
if apiBase == "" {
|
||||||
apiBase = getDefaultAPIBase(protocol)
|
apiBase = GetDefaultAPIBase(protocol)
|
||||||
}
|
}
|
||||||
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
|
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
|
||||||
|
|
||||||
|
|
@ -95,7 +95,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
||||||
}
|
}
|
||||||
apiBase := cfg.APIBase
|
apiBase := cfg.APIBase
|
||||||
if apiBase == "" {
|
if apiBase == "" {
|
||||||
apiBase = getDefaultAPIBase(protocol)
|
apiBase = GetDefaultAPIBase(protocol)
|
||||||
}
|
}
|
||||||
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
|
return NewHTTPProviderWithMaxTokensField(cfg.APIKey, apiBase, cfg.Proxy, cfg.MaxTokensField), modelID, nil
|
||||||
|
|
||||||
|
|
@ -155,8 +155,9 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// getDefaultAPIBase returns the default API base URL for a given protocol.
|
// GetDefaultAPIBase returns the default API base URL for a given protocol.
|
||||||
func getDefaultAPIBase(protocol string) string {
|
// Returns an empty string if the protocol is unknown.
|
||||||
|
func GetDefaultAPIBase(protocol string) string {
|
||||||
switch protocol {
|
switch protocol {
|
||||||
case "openai":
|
case "openai":
|
||||||
return "https://api.openai.com/v1"
|
return "https://api.openai.com/v1"
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue