From d01a874c177b2723d6ee31782cb5bb9304f5762e Mon Sep 17 00:00:00 2001 From: rfshubert Date: Sun, 22 Feb 2026 10:43:37 -0300 Subject: [PATCH] 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. --- cmd/picoclaw/cmd_gateway.go | 11 --------- cmd/picoclaw/stt_resolution.go | 5 +++- cmd/picoclaw/stt_resolution_test.go | 36 +++++++++++++++++------------ pkg/providers/factory_provider.go | 9 ++++---- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/cmd/picoclaw/cmd_gateway.go b/cmd/picoclaw/cmd_gateway.go index c138f837e..97fe69f4f 100644 --- a/cmd/picoclaw/cmd_gateway.go +++ b/cmd/picoclaw/cmd_gateway.go @@ -241,14 +241,3 @@ func setupCronTool( 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 "" - } -} diff --git a/cmd/picoclaw/stt_resolution.go b/cmd/picoclaw/stt_resolution.go index cc70d71b5..6663e7d14 100644 --- a/cmd/picoclaw/stt_resolution.go +++ b/cmd/picoclaw/stt_resolution.go @@ -20,7 +20,10 @@ func resolveSTTTranscriber(cfg *config.Config) voice.Transcriber { protocol, modelID := providers.ExtractProtocol(mc.Model) apiBase := mc.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) } diff --git a/cmd/picoclaw/stt_resolution_test.go b/cmd/picoclaw/stt_resolution_test.go index 3f3f16858..59a1bb32d 100644 --- a/cmd/picoclaw/stt_resolution_test.go +++ b/cmd/picoclaw/stt_resolution_test.go @@ -7,22 +7,28 @@ import ( "github.com/sipeed/picoclaw/pkg/voice" ) -func TestGetDefaultSTTBase(t *testing.T) { - tests := []struct { - protocol string - expected string - }{ - {"openai", "https://api.openai.com/v1"}, - {"groq", "https://api.groq.com/openai/v1"}, - {"unknown", ""}, - {"", ""}, +func TestResolveSTTTranscriber_UnknownProtocolSkipped(t *testing.T) { + // stt_model points to an entry with an unknown protocol and no api_base. + // The unknown protocol resolves to an empty API base, so this entry must be + // skipped (continue) and the function should fall through to return nil. + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + 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) { - if got := getDefaultSTTBase(tt.protocol); got != tt.expected { - t.Errorf("getDefaultSTTBase(%q) = %q, want %q", tt.protocol, got, tt.expected) - } - }) + + tr := resolveSTTTranscriber(cfg) + if tr != nil { + t.Error("expected nil - unknown protocol with no api_base should be skipped") } } diff --git a/pkg/providers/factory_provider.go b/pkg/providers/factory_provider.go index 74fe8a36c..9218108a3 100644 --- a/pkg/providers/factory_provider.go +++ b/pkg/providers/factory_provider.go @@ -82,7 +82,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err } apiBase := cfg.APIBase if apiBase == "" { - apiBase = getDefaultAPIBase(protocol) + apiBase = GetDefaultAPIBase(protocol) } 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 if apiBase == "" { - apiBase = getDefaultAPIBase(protocol) + apiBase = GetDefaultAPIBase(protocol) } 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. -func getDefaultAPIBase(protocol string) string { +// GetDefaultAPIBase returns the default API base URL for a given protocol. +// Returns an empty string if the protocol is unknown. +func GetDefaultAPIBase(protocol string) string { switch protocol { case "openai": return "https://api.openai.com/v1"