diff --git a/internal/agent/modelprefs_test.go b/internal/agent/modelprefs_test.go index 3e2af49c..6c7baa52 100644 --- a/internal/agent/modelprefs_test.go +++ b/internal/agent/modelprefs_test.go @@ -46,7 +46,10 @@ func TestModelNameAndLevelsReachAgentFiles(t *testing.T) { t.Fatal(err) } ms := pi.Providers["magpie"].Models - if len(ms) != 1 || ms[0]["id"] != "relay/glm-4.6" || ms[0]["name"] != "GLM" { + // a custom name still carries its provider, the same as GLM-4.6's own + // name would, so a picker full of renamed models can still be told + // apart by vendor + if len(ms) != 1 || ms[0]["id"] != "relay/glm-4.6" || ms[0]["name"] != "GLM · Relay" { t.Fatalf("pi models.json: %v", ms) } var cx struct { @@ -61,7 +64,7 @@ func TestModelNameAndLevelsReachAgentFiles(t *testing.T) { if err := json.Unmarshal([]byte(readFile(codexCat)), &cx); err != nil { t.Fatal(err) } - if len(cx.Models) != 1 || cx.Models[0].Slug != "relay/glm-4.6" || cx.Models[0].Name != "GLM" { + if len(cx.Models) != 1 || cx.Models[0].Slug != "relay/glm-4.6" || cx.Models[0].Name != "GLM · Relay" { t.Fatalf("codex catalog: %+v", cx.Models) } var levels []string @@ -72,6 +75,14 @@ func TestModelNameAndLevelsReachAgentFiles(t *testing.T) { t.Fatalf("codex levels: %v", levels) } + // unless the custom name already says the provider: it isn't repeated + if err := provider.SetModelName("relay/glm-4.6", "GLM via Relay"); err != nil { + t.Fatal(err) + } + if got := readFile(codexCat); !strings.Contains(got, `"GLM via Relay"`) || strings.Contains(got, `"GLM via Relay · Relay"`) { + t.Fatalf("codex catalog, provider repeated:\n%s", got) + } + if err := provider.SetModelName("relay/glm-4.6", ""); err != nil { t.Fatal(err) } diff --git a/internal/provider/modelprefs.go b/internal/provider/modelprefs.go index d8dfba9c..8b6f7fdf 100644 --- a/internal/provider/modelprefs.go +++ b/internal/provider/modelprefs.go @@ -12,11 +12,14 @@ import ( // A model's name is the vendor's, or models.dev's, and agents are shown it // with the provider after it (Claude Opus 5.5 · Claude Code). The user can -// give one of a provider's models a name of their own instead (Opus 5.5): -// it is kept in settings' ModelNames by "/", apart -// from the vendor's list, so a Refresh, a restart or an upgrade leaves it, -// and the same model another provider serves keeps its own. Only the name -// changes: the model's id, and where requests for it go, stay as they were. +// give one of a provider's models a name of their own instead (Opus): it is +// kept in settings' ModelNames by "/", apart from the +// vendor's list, so a Refresh, a restart or an upgrade leaves it, and the +// same model another provider serves keeps its own. Only the name changes: +// the model's id, and where requests for it go, stay as they were. The +// provider still goes after it (Opus · Claude Code), same as the vendor's +// own name, so a picker full of renamed models can still be told apart by +// vendor; it's left off only when the name given already says it. // The user can also keep only some of the reasoning levels a model has // (low, medium and high of its six): settings' ModelEfforts, kept the same @@ -196,17 +199,18 @@ func renameKeys[V any](m map[string]V, from, to string) bool { return len(moved) > 0 } -// Label is how an agent's list names the entry: the name the user gave it, -// else its own with its provider's after it, or "routing group" for a -// group's. +// Label is how an agent's list names the entry: its name (the user's own, +// when they gave one, else the vendor's) with its provider's after it, or +// "routing group" for a group's — dropped only when a name of the user's +// own already carries the provider's, so it isn't said twice. func (e Entry) Label() string { - if e.Default != "" { - return e.Name - } by := e.Provider.Name if e.Group != "" { by = "routing group" } + if e.Default != "" && strings.Contains(strings.ToLower(e.Name), strings.ToLower(by)) { + return e.Name + } return e.Name + " · " + by } diff --git a/internal/provider/modelprefs_test.go b/internal/provider/modelprefs_test.go index 12dba2f0..674bee30 100644 --- a/internal/provider/modelprefs_test.go +++ b/internal/provider/modelprefs_test.go @@ -77,7 +77,9 @@ func TestModelName(t *testing.T) { t.Fatal(n, ok) } a, b := entry(t, "a/sol"), entry(t, "b/sol") - if a.Name != "My Sol" || a.Default != "Sol" || a.Label() != "My Sol" || a.Model != "sol" { + // a custom name still carries its provider, the same as the vendor's + // own name does, so the picker can still be told apart by vendor + if a.Name != "My Sol" || a.Default != "Sol" || a.Label() != "My Sol · A" || a.Model != "sol" { t.Fatalf("%+v %q", a, a.Label()) } if b.Name != "Sol" || b.Default != "" || b.Label() != "Sol · B" { @@ -93,10 +95,18 @@ func TestModelName(t *testing.T) { for _, m := range CodexListed() { listed = append(listed, m.ID+"="+m.Name) } - if !slices.Contains(listed, "a/sol=My Sol") || !slices.Contains(listed, "b/sol=Sol · B") { + if !slices.Contains(listed, "a/sol=My Sol · A") || !slices.Contains(listed, "b/sol=Sol · B") { t.Fatal(listed) } - if got := (Provider{ID: "a"}).ModelNames(); got["sol"] != "My Sol" || len(got) != 1 { + + // unless the custom name already says the provider: it isn't repeated + if err := SetModelName("a/sol", "Sol on A"); err != nil { + t.Fatal(err) + } + if a := entry(t, "a/sol"); a.Label() != "Sol on A" { + t.Fatalf("provider repeated: %+v %q", a, a.Label()) + } + if got := (Provider{ID: "a"}).ModelNames(); got["sol"] != "Sol on A" || len(got) != 1 { t.Fatal(got) }