fix(agent): resolve model names to full model strings before fallback

Previously, ResolveCandidates was using model_names (like "gemini-flash")
directly as model strings, without looking them up in model_list first.

This caused issues where:
1. model_name: "gemini-flash" → used as model: "gemini-flash" directly
2. ParseModelRef("gemini-flash", "") → Provider: "", Model: "gemini-flash"
3. Error showed: provider= model=gemini-flash (empty provider!)
4. The actual model string "antigravity/gemini-3-flash" was never used

The fix adds two helper functions:
1. resolveModelString(cfg, modelName) - looks up model_name in model_list
   and returns the full model string (e.g., "antigravity/gemini-3-flash")
2. resolveFallbackModelStrings(cfg, modelNames) - resolves multiple names

Now the agent instance creation flow:
1. Gets model_name (e.g., "gemini-flash") from config
2. Looks it up in model_list to get full model (e.g., "antigravity/gemini-3-flash")
3. Passes full model string to ResolveCandidates
4. ParseModelRef correctly parses: Provider="antigravity", Model="gemini-3-flash"
5. Fallback candidates have correct provider and model info

This also fixes nested protocol models like "openrouter/openrouter/free":
- Provider: "openrouter"
- Model: "openrouter/free" (everything after first /)

Tests added:
- TestResolveModelString: verifies model name lookup
- TestResolveFallbackModelStrings: verifies batch lookup
- TestNewAgentInstance_ModelResolution: integration test

Fixes the issue where model changes weren't being picked up after restart.
This commit is contained in:
Vishnuvardhan Reddy 2026-02-26 12:35:54 +00:00
parent 6254bd52ac
commit 6e48d37667
2 changed files with 193 additions and 2 deletions

View file

@ -88,9 +88,13 @@ func NewAgentInstance(
}
// Resolve fallback candidates
// First, look up model names in model_list to get full model strings
primaryModelString := resolveModelString(cfg, model)
fallbackModelStrings := resolveFallbackModelStrings(cfg, fallbacks)
modelCfg := providers.ModelConfig{
Primary: model,
Fallbacks: fallbacks,
Primary: primaryModelString,
Fallbacks: fallbackModelStrings,
}
candidates := providers.ResolveCandidates(modelCfg, defaults.Provider)
@ -156,3 +160,32 @@ func expandHome(path string) string {
}
return path
}
// resolveModelString looks up a model name in model_list and returns the full model string.
// If the model name already contains a "/" (like "openrouter/free"), it's returned as-is.
// If the model name is not found in model_list, it's returned as-is (for backward compatibility).
func resolveModelString(cfg *config.Config, modelName string) string {
// If it already looks like a full model string (protocol/model), return it as-is
if strings.Contains(modelName, "/") {
return modelName
}
// Look up in model_list
modelCfg, err := cfg.GetModelConfig(modelName)
if err != nil {
// Model not found in model_list, return as-is for backward compatibility
return modelName
}
// Return the full model string (e.g., "antigravity/gemini-3-flash")
return modelCfg.Model
}
// resolveFallbackModelStrings looks up multiple model names in model_list and returns their full model strings.
func resolveFallbackModelStrings(cfg *config.Config, modelNames []string) []string {
result := make([]string, 0, len(modelNames))
for _, name := range modelNames {
result = append(result, resolveModelString(cfg, name))
}
return result
}

View file

@ -0,0 +1,158 @@
package agent
import (
"testing"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestResolveModelString tests that model names are correctly resolved to full model strings
func TestResolveModelString(t *testing.T) {
tests := []struct {
name string
modelName string
modelListEntry *config.ModelConfig
expectedResult string
}{
{
name: "model name without slash - looks up in model_list",
modelName: "gemini-flash",
modelListEntry: &config.ModelConfig{Model: "antigravity/gemini-3-flash"},
expectedResult: "antigravity/gemini-3-flash",
},
{
name: "model name with slash - returned as-is",
modelName: "openrouter/free",
modelListEntry: nil,
expectedResult: "openrouter/free",
},
{
name: "model name not in model_list - returned as-is",
modelName: "unknown-model",
modelListEntry: nil,
expectedResult: "unknown-model",
},
{
name: "nested protocol model",
modelName: "openrouter-nested",
modelListEntry: &config.ModelConfig{Model: "openrouter/openrouter/free"},
expectedResult: "openrouter/openrouter/free",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg := &config.Config{}
if tt.modelListEntry != nil {
tt.modelListEntry.ModelName = tt.modelName
cfg.ModelList = []config.ModelConfig{*tt.modelListEntry}
}
result := resolveModelString(cfg, tt.modelName)
assert.Equal(t, tt.expectedResult, result)
})
}
}
// TestResolveFallbackModelStrings tests that multiple model names are resolved correctly
func TestResolveFallbackModelStrings(t *testing.T) {
cfg := &config.Config{
ModelList: []config.ModelConfig{
{ModelName: "gemini-flash", Model: "antigravity/gemini-3-flash"},
{ModelName: "openrouter-free", Model: "openrouter/free"},
{ModelName: "claude-sonnet", Model: "anthropic/claude-sonnet-4-20250514"},
},
}
modelNames := []string{"gemini-flash", "openrouter-free", "claude-sonnet"}
result := resolveFallbackModelStrings(cfg, modelNames)
expected := []string{
"antigravity/gemini-3-flash",
"openrouter/free",
"anthropic/claude-sonnet-4-20250514",
}
assert.Equal(t, expected, result)
}
// TestResolveFallbackModelStrings_MixedInput tests resolution with mixed input
func TestResolveFallbackModelStrings_MixedInput(t *testing.T) {
cfg := &config.Config{
ModelList: []config.ModelConfig{
{ModelName: "gemini-flash", Model: "antigravity/gemini-3-flash"},
},
}
// Mix of model names and full model strings
modelNames := []string{
"gemini-flash", // Should resolve to "antigravity/gemini-3-flash"
"openrouter/free", // Already a full string, kept as-is
"anthropic/claude-3-5", // Already a full string, kept as-is
}
result := resolveFallbackModelStrings(cfg, modelNames)
expected := []string{
"antigravity/gemini-3-flash",
"openrouter/free",
"anthropic/claude-3-5",
}
assert.Equal(t, expected, result)
}
// TestNewAgentInstance_ModelResolution is an integration test that verifies
// the full model resolution flow when creating an agent instance
func TestNewAgentInstance_ModelResolution(t *testing.T) {
cfg := &config.Config{
ModelList: []config.ModelConfig{
{
ModelName: "gemini-flash",
Model: "antigravity/gemini-3-flash",
APIKey: "test-key",
},
{
ModelName: "openrouter-free",
Model: "openrouter/free",
APIKey: "sk-or-test",
APIBase: "https://openrouter.ai/api/v1",
},
},
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
Workspace: t.TempDir(),
ModelName: "gemini-flash",
ModelFallbacks: []string{"openrouter-free"},
},
},
}
// We don't need an actual provider for this test since we're not calling Chat
// Use the existing mockProvider from mock_provider_test.go
provider := &mockProvider{}
instance := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, provider)
require.NotNil(t, instance)
// Verify the agent's Model field is still the model_name (not overwritten)
assert.Equal(t, "gemini-flash", instance.Model,
"Agent's Model field should remain as the model_list entry name")
// Verify the Candidates have been resolved to full model strings
require.Len(t, instance.Candidates, 2, "Should have 2 candidates (primary + fallback)")
// First candidate should be the resolved primary model
assert.Equal(t, "antigravity", instance.Candidates[0].Provider,
"Primary candidate provider should be 'antigravity'")
assert.Equal(t, "gemini-3-flash", instance.Candidates[0].Model,
"Primary candidate model should be 'gemini-3-flash' (not 'antigravity/gemini-3-flash')")
// Second candidate should be the resolved fallback model
assert.Equal(t, "openrouter", instance.Candidates[1].Provider,
"Fallback candidate provider should be 'openrouter'")
assert.Equal(t, "free", instance.Candidates[1].Model,
"Fallback candidate model should be 'free' (not 'openrouter/free')")
}