diff --git a/docs/changes/unreleased/1501-crew-pinned-checker-gap.md b/docs/changes/unreleased/1501-crew-pinned-checker-gap.md new file mode 100644 index 0000000000..644747ef16 --- /dev/null +++ b/docs/changes/unreleased/1501-crew-pinned-checker-gap.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: the /crew panel's weak-checker warning counts a pinned checker +pr: 1501 +surface: [chat, docs] +invalidates: + - "The `/crew` panel's `no strong checker among the models you allow` warning was asked of the allowed models only. A strong checker pinned over a weak set left the warning on screen, and a weak checker pinned over a strong set had none. A pinned checker is now the one asked: a strong pin clears the warning, and a weak pin reads `checker pinned to · open-ended work will be checked weakly`." +--- diff --git a/internal/config/crew.go b/internal/config/crew.go index 250e491d0f..41fc87d2fe 100644 --- a/internal/config/crew.go +++ b/internal/config/crew.go @@ -1066,9 +1066,19 @@ func RestoreCrewState(profileDir string, state CrewState) error { } // CrewGapsAt names what the allowed models leave uncovered, for the panel's -// one-line warning. +// one-line warning. A pinned seat is judged by its pin alone, read from the +// catalog whether or not a route to it is healthy right now, because the pin +// is what the seat runs. func CrewGapsAt(profileDir string) []crewroute.Gap { - return crewroute.Gaps(CrewCandidatesAt(profileDir)) + pins := map[crewroute.Seat]crewroute.Model{} + for seat, pin := range CrewPinsAt(profileDir) { + model, known := crewCatalogModel(pin.Model) + if !known { + model = crewroute.Model{ID: pin.Model} + } + pins[seat] = model + } + return crewroute.Gaps(CrewCandidatesAt(profileDir), pins) } // ── one task's crew ───────────────────────────────────────────────────────── diff --git a/internal/config/crewhealth_test.go b/internal/config/crewhealth_test.go index dd2fd3da41..6fc772f9c8 100644 --- a/internal/config/crewhealth_test.go +++ b/internal/config/crewhealth_test.go @@ -96,6 +96,49 @@ func TestAForbiddenRouteIsNotRoutedToAgain(t *testing.T) { } } +// THE PANEL'S GAP COUNTS A PINNED CHECKER. A strong checker pinned over a set +// whose only other model is weak leaves no warning — even while the pin's route +// is quarantined and the candidates no longer carry it — and a weak one pinned +// over a strong set is warned about by name. +func TestTheGapCountsAPinnedChecker(t *testing.T) { + dir := crewProfile(t) + var history []router.CrewRouteOutcome + withRouteHistory(t, &history) + if err := SetCrewAllowed(dir, "deepseek-v4-flash, kimi-k3"); err != nil { + t.Fatal(err) + } + if gaps := CrewGapsAt(dir); len(gaps) != 0 { + t.Fatalf("a set with kimi-k3 in it: gaps %+v", gaps) + } + if err := SetCrewPin(dir, crewroute.Checker, "moonshotai/kimi-k3"); err != nil { + t.Fatal(err) + } + history = append(history, router.CrewRouteOutcome{At: time.Now(), Seat: "checker", Send: "moonshotai/kimi-k3", Provider: "openrouter", Kind: "forbidden"}) + for _, c := range CrewCandidatesAt(dir) { + if crewroute.Lineage(c.Model.ID) == crewroute.Lineage("moonshotai/kimi-k3") && len(c.Routes) > 0 { + t.Fatalf("the quarantined pin is still a candidate: %+v", c) + } + } + if gaps := CrewGapsAt(dir); len(gaps) != 0 { + t.Errorf("kimi-k3 pinned as checker: gaps %+v, want none", gaps) + } + history = nil + if err := SetCrewPin(dir, crewroute.Checker, "deepseek/deepseek-v4-flash"); err != nil { + t.Fatal(err) + } + gaps := CrewGapsAt(dir) + want := "checker pinned to deepseek-v4-flash · open-ended work will be checked weakly" + if len(gaps) != 1 || gaps[0].Line != want { + t.Errorf("v4-flash pinned as checker: gaps %+v, want %q", gaps, want) + } + if err := ClearCrewPin(dir, crewroute.Checker); err != nil { + t.Fatal(err) + } + if gaps := CrewGapsAt(dir); len(gaps) != 0 { + t.Errorf("the checker unpinned again: gaps %+v", gaps) + } +} + // A ZERO-CREDIT ACCOUNT, end to end: a paid call says payment → the next // task is still routed on the paid route, which its first call probes → the // seat's rescue is the free pool → with that pool at its limit and the chat diff --git a/internal/crewroute/route.go b/internal/crewroute/route.go index 2deb43cd93..3a6af5229b 100644 --- a/internal/crewroute/route.go +++ b/internal/crewroute/route.go @@ -1246,16 +1246,38 @@ type Gap struct { // Gaps names what the allowed models cannot cover. Today that is one thing, // the seat the routing rule depends on: open-ended work with no strong checker -// among the models allowed — none credible whose ability reaches the middle -// the open-ended link is centred on. -func Gaps(candidates []Candidate) []Gap { +// — none credible whose ability reaches the middle the open-ended link is +// centred on. +// +// A PINNED SEAT IS THE SEAT. A pin always runs, so a pinned checker is the only +// model the question is asked of: a strong pin leaves no gap however weak the +// rest of the allowed models are, and a weak pin is a gap however strong they +// are, said with the pin's name because the pin is what a person would change. +// pins carry each pinned seat's model as the catalog reads it, or only its id +// when the catalog does not carry it; a candidate of the same lineage is read +// in its place, the way [pinned] reads it. An unpinned checker is asked of +// every model allowed, as before. +func Gaps(candidates []Candidate, pins map[Seat]Model) []Gap { t := forDecision(candidates, nil) strong := t.linkOf(OpenEnded).URef - for _, c := range candidates { - if !seatable(Checker, c) || !t.credible(c.Model) { - continue + isStrong := func(m Model) bool { + a := t.abilityOf(m) + return t.credibleAt(a) && a.U+math.Sqrt(a.VarU) >= strong + } + if pin, ok := pins[Checker]; ok && strings.TrimSpace(pin.ID) != "" { + for _, c := range candidates { + if Lineage(c.Model.ID) == Lineage(pin.ID) { + pin = c.Model + break + } + } + if isStrong(pin) { + return nil } - if a := t.abilityOf(c.Model); a.U+math.Sqrt(a.VarU) >= strong { + return []Gap{{Class: OpenEnded, Seat: Checker, Line: "checker pinned to " + ShortModel(pin.ID) + " · open-ended work will be checked weakly"}} + } + for _, c := range candidates { + if seatable(Checker, c) && isStrong(c.Model) { return nil } } diff --git a/internal/crewroute/route_test.go b/internal/crewroute/route_test.go index 6fa4ee603c..f398dc9e90 100644 --- a/internal/crewroute/route_test.go +++ b/internal/crewroute/route_test.go @@ -516,15 +516,41 @@ func TestPaceGrowsAsTheCapNears(t *testing.T) { } func TestGapsNameAMissingStrongChecker(t *testing.T) { - if gaps := Gaps(catalogCandidates()); len(gaps) != 0 { + if gaps := Gaps(catalogCandidates(), nil); len(gaps) != 0 { t.Errorf("a set with a strong checker has gaps %+v", gaps) } - gaps := Gaps([]Candidate{candidateOf(v4Flash)}) + gaps := Gaps([]Candidate{candidateOf(v4Flash)}, nil) if len(gaps) != 1 || gaps[0].Seat != Checker || gaps[0].Class != OpenEnded { t.Errorf("v4-flash alone: gaps %+v, want the open-ended checker", gaps) } } +// A PINNED CHECKER IS THE CHECKER. The gap is asked of the pin alone: a strong +// pin closes it over a weak set, a weak pin opens it over a strong one and names +// itself, and a pin on another seat changes nothing. +func TestGapsCountAPinnedChecker(t *testing.T) { + weakSet := []Candidate{candidateOf(v4Flash)} + if gaps := Gaps(weakSet, map[Seat]Model{Checker: kimiK3}); len(gaps) != 0 { + t.Errorf("a strong pinned checker over a weak set: gaps %+v, want none", gaps) + } + // The pin is read from the candidates when they carry it, and from the + // figures it came with when they do not. + if gaps := Gaps(append(weakSet, candidateOf(kimiK3)), map[Seat]Model{Checker: {ID: kimiK3.ID}}); len(gaps) != 0 { + t.Errorf("a strong pinned checker the candidates carry: gaps %+v, want none", gaps) + } + gaps := Gaps(catalogCandidates(), map[Seat]Model{Checker: v4Flash}) + want := "checker pinned to deepseek-v4-flash · open-ended work will be checked weakly" + if len(gaps) != 1 || gaps[0].Seat != Checker || gaps[0].Class != OpenEnded || gaps[0].Line != want { + t.Errorf("a weak pinned checker over a strong set: gaps %+v, want %q", gaps, want) + } + if gaps := Gaps(catalogCandidates(), map[Seat]Model{Worker: v4Flash, Planner: v4Flash}); len(gaps) != 0 { + t.Errorf("an unpinned checker over a strong set, other seats pinned weak: gaps %+v", gaps) + } + gaps = Gaps(weakSet, map[Seat]Model{Worker: kimiK3}) + if len(gaps) != 1 || !strings.HasPrefix(gaps[0].Line, "no strong checker among the models you allow") { + t.Errorf("an unpinned checker over a weak set, the worker pinned strong: gaps %+v", gaps) + } +} func TestTheDecisionLine(t *testing.T) { cands := catalogCandidates() d, _ := Decide(Request{Class: OpenEnded, Candidates: cands, Pins: map[Seat]Pin{Checker: {Model: "moonshotai/kimi-k3"}}}) diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index a75f0bfff7..41b87fcf38 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -568,7 +568,12 @@ pinned seat whose only provider is off says `provider off` on its row. With nothing connected the panel says `no providers connected — /connect adds one`; a pin whose provider is not connected says `unavailable`. A rule that leaves open-ended work -without a strong checker says so under the rows. +without a strong checker says so under the rows: +`no strong checker among the models you allow · open-ended work will be checked weakly`. +**A pinned checker is the checker**, so the warning asks about the pin alone: a strong pin +clears it whatever else is allowed, and a weak one names itself — +`checker pinned to deepseek-v4-flash · open-ended work will be checked weakly` — even when a +strong model is allowed beside it. Every change is saved the moment you make it, and the next task uses it with no relaunch. The changed row wears a tick `✓`, and for five seconds the bottom edge offers `z undo`, diff --git a/internal/tui3/crewsnap_test.go b/internal/tui3/crewsnap_test.go index 1be50a0083..691ff64a44 100644 --- a/internal/tui3/crewsnap_test.go +++ b/internal/tui3/crewsnap_test.go @@ -50,6 +50,28 @@ func TestCrewSnapshotPanel(t *testing.T) { ╰─ enter change · esc close · ? keys ──────────────────────────────────────────────────────────────╯`) } +// A WEAK PINNED CHECKER IS WARNED ABOUT BY NAME, under the rows, however strong +// the other allowed models are: the pin is what runs. +func TestCrewSnapshotWeakPinnedChecker(t *testing.T) { + a, dir := crewLab(t) + if err := config.SetCrewPin(dir, crewroute.Checker, "deepseek/deepseek-v4-flash"); err != nil { + t.Fatal(err) + } + typeLine(t, a, "/crew") + crewSnap(t, a, "weak pinned checker", ` +╭─ crew ───────────────────────────────────────────────────────────────────────────────────── esc ─╮ +│› worker auto · likely glm-5.3-flash │ +│ planner auto · likely glm-5.3-flash │ +│ checker {pin} deepseek-v4-flash │ +│ │ +│ models ‹ all › (4) │ +│ providers {tick} openrouter + │ +│ cap per task $5 · crew daily cap none │ +│ the daily limit, $500, still covers everything codeaf spends · /budget │ +│ checker pinned to deepseek-v4-flash · open-ended work will be checked weakly │ +╰─ enter change · esc close · ? keys ──────────────────────────────────────────────────────────────╯`) +} + func TestCrewSnapshotSeatList(t *testing.T) { a, _ := crewLab(t) typeLine(t, a, "/crew")