From 559ce021479bd315832ded56f8d26516ddfd82d2 Mon Sep 17 00:00:00 2001 From: yetone Date: Tue, 29 Sep 2026 18:53:57 +0800 Subject: [PATCH] =?UTF-8?q?models:=20a=20model's=20own=20name=20given=20by?= =?UTF-8?q?=20the=20user=20keeps=20its=20provider=20after=20it=20too=20(#2?= =?UTF-8?q?03=20uclort:=20=E5=A6=82=E6=9E=9C=E5=9C=A8=E4=BE=9B=E5=BA=94?= =?UTF-8?q?=E5=95=86=E6=A8=A1=E5=9E=8B=E9=A1=B5=E8=87=AA=E5=AE=9A=E4=B9=89?= =?UTF-8?q?=E6=A8=A1=E5=9E=8B=E5=90=8D=E7=A7=B0=EF=BC=8C=E5=9C=A8=20Codex?= =?UTF-8?q?=20=E4=B8=AD=E5=B0=B1=E4=B8=8D=E4=BC=9A=E6=98=BE=E7=A4=BA?= =?UTF-8?q?=E4=BE=9B=E5=BA=94=E5=95=86=E4=BA=86)=20=E2=80=94=20Entry.Label?= =?UTF-8?q?()=20showed=20"name=20=C2=B7=20provider"=20only=20for=20a=20mod?= =?UTF-8?q?el's=20own=20(vendor's=20or=20models.dev's)=20name;=20a=20name?= =?UTF-8?q?=20the=20user=20gave=20it=20(provider's=20model=20page=20?= =?UTF-8?q?=E2=86=92=20Names=20&=20levels)=20dropped=20the=20provider=20en?= =?UTF-8?q?tirely,=20so=20a=20renamed=20model=20in=20Codex's=20picker=20(a?= =?UTF-8?q?nd=20Pi's,=20ZCode's,=20OMP's,=20magpie's=20own=20gateway=20lis?= =?UTF-8?q?t=20=E2=80=94=20everything=20magpieModels/Label=20feeds)=20coul?= =?UTF-8?q?d=20no=20longer=20be=20told=20apart=20by=20vendor=20at=20a=20gl?= =?UTF-8?q?ance.=20Label()=20now=20appends=20"=20=C2=B7=20provider"=20(or?= =?UTF-8?q?=20"routing=20group")=20to=20a=20custom=20name=20too,=20the=20s?= =?UTF-8?q?ame=20as=20it=20does=20for=20the=20vendor's=20own,=20and=20only?= =?UTF-8?q?=20leaves=20it=20off=20when=20the=20name=20already=20contains?= =?UTF-8?q?=20the=20provider's=20(case-insensitively),=20so=20a=20name=20l?= =?UTF-8?q?ike=20"Sol=20on=20A"=20isn't=20doubled=20to=20"Sol=20on=20A=20?= =?UTF-8?q?=C2=B7=20A".=20No=20settings=20toggle=20added:=20the=20fix=20ma?= =?UTF-8?q?kes=20the=20provider=20suffix=20unconditional=20(what=20the=20i?= =?UTF-8?q?ssue=20actually=20asked=20for),=20so=20a=20preference=20to=20hi?= =?UTF-8?q?de/show=20it=20would=20only=20reintroduce=20the=20gap;=20there'?= =?UTF-8?q?s=20also=20no=20existing=20"model=20list"=20preferences=20surfa?= =?UTF-8?q?ce=20to=20hang=20it=20on=20cheaply=20=E2=80=94=20Names=20&=20le?= =?UTF-8?q?vels=20only=20holds=20the=20name=20and=20reasoning-level=20pick?= =?UTF-8?q?ers.=20Tested=20with=20TestModelName=20(internal/provider/model?= =?UTF-8?q?prefs=5Ftest.go,=20sandbox=20HOME):=20a=20custom=20name=20now?= =?UTF-8?q?=20reads=20"My=20Sol=20=C2=B7=20A"=20in=20Label()=20and=20in=20?= =?UTF-8?q?CodexListed(),=20and=20"Sol=20on=20A"=20(already=20containing?= =?UTF-8?q?=20the=20provider)=20is=20left=20alone;=20and=20TestModelNameAn?= =?UTF-8?q?dLevelsReachAgentFiles=20(internal/agent/modelprefs=5Ftest.go,?= =?UTF-8?q?=20sandbox=20HOME)=20checks=20the=20generated=20Codex=20model?= =?UTF-8?q?=5Fcatalog=5Fjson=20and=20Pi=20models.json=20entries=20carry=20?= =?UTF-8?q?"GLM=20=C2=B7=20Relay"=20for=20a=20custom=20name=20and=20don't?= =?UTF-8?q?=20double=20it=20to=20"GLM=20via=20Relay=20=C2=B7=20Relay"=20wh?= =?UTF-8?q?en=20the=20name=20already=20says=20Relay.=20go=20build=20-tags?= =?UTF-8?q?=20nogui=20(and=20GOOS=3Dlinux,=20GOOS=3Dwindows),=20go=20test?= =?UTF-8?q?=20-tags=20nogui=20-count=3D1=20./...=20pass=20except=20TestUse?= =?UTF-8?q?rPath=20and=20TestClaudeAccountNamedAsItsLogin,=20which=20are?= =?UTF-8?q?=20load-flaky=20(pass=20alone,=20unrelated=20to=20this=20change?= =?UTF-8?q?)=20same=20as=20the=20pre-existing=20rtk.go=20gofmt=20/=20TestC?= =?UTF-8?q?onfiguredModelsAdvertiseImageInput=20flakiness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/agent/modelprefs_test.go | 15 +++++++++++++-- internal/provider/modelprefs.go | 26 +++++++++++++++----------- internal/provider/modelprefs_test.go | 16 +++++++++++++--- 3 files changed, 41 insertions(+), 16 deletions(-) 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) }