From 32320b9e04ffe5297a31f44c918211656cc46082 Mon Sep 17 00:00:00 2001 From: gluck Date: Wed, 30 Sep 2026 16:41:32 -0400 Subject: [PATCH] :models is a pit you choose in, and Enter sets the model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `:models` in the command box printed the table `figaro models` prints, with os.Stdout: round the pager's writer, onto the terminal. Nothing on it could be acted on. It is now an itemised live view (itemView, like the form and the queue), so the pit's one picker drives it: ^N/^P and j/k move, the wash and the marker show the choice, `y` yanks the model id, `/` searches, G and gg reach the ends of three hundred models. The subject's current model carries a ●, and each row carries its provider. ENTER SETS THE MODEL, and its provider with it, in one patch: system.model to the chosen id and system.provider to the provider that listed it. A model is only meaningful to its own provider, and choosing copilot's model while the board still said anthropic would send a name Anthropic has never heard of on the next turn. When the provider already matches, the store drops the unchanged key and the write is exactly `figaro set system.model `. The bar says what took; the ● moves. Activate hands off to a goroutine: it runs under the render lock and the set dials. `/` could not reach a provider: the heading is chrome and search walks selectable rows. The picker now searches a row's note as well as its text -- what a reader sees is what `/` finds -- which also makes the tab pit's descriptions searchable. The standalone `figaro models` shares the fetch and now writes to the package's stdout, so it can never paint over a pager again. Verified with the flake build in an isolated .#sandbox shell, real credentials: :models lists every configured provider's models, Enter on claude-opus-5-5 set it (figaro state agrees) and the next turn was answered by it; Enter on a copilot model set model and provider together and the next turn was answered through copilot; Enter back on anthropic restored both. --- internal/cli/command_router.go | 77 ++++++++++++++++ internal/cli/models_view.go | 147 +++++++++++++++++++++++++++++++ internal/cli/models_view_test.go | 123 ++++++++++++++++++++++++++ internal/cli/picker.go | 3 +- internal/cli/system.go | 34 ++----- 5 files changed, 358 insertions(+), 26 deletions(-) create mode 100644 internal/cli/models_view.go create mode 100644 internal/cli/models_view_test.go diff --git a/internal/cli/command_router.go b/internal/cli/command_router.go index 9b66d621..d8f9e4cc 100644 --- a/internal/cli/command_router.go +++ b/internal/cli/command_router.go @@ -20,7 +20,9 @@ package cli import ( "context" + "encoding/json" "fmt" + "github.com/jack-work/figaro/api/form" "github.com/jack-work/figaro/internal/cmdkit" "strings" "sync" @@ -103,6 +105,9 @@ var overlayVerbs = map[string]bool{ "kill": true, // it can kill the aria on screen, and must then move "import": true, // reads os.Stdin for `-` (portable.go) "login": true, // an interactive OAuth flow with its own prompts + // models is a PIT, not a table: a list you choose the subject's model + // in (models_view.go). As a routed verb it printed to os.Stdout. + "models": true, } // terminalSubverbs are verb+subverb pairs that take the terminal, which a bare @@ -232,11 +237,83 @@ func (in *interactiveInput) runOverlay(verb string, args []string) { in.commandAsync(func(ctx context.Context) (string, error) { return in.commandFork(ctx, args) }) + case "models": + in.openModels() default: in.note(fmt.Sprintf("%s: not available inside the pager (it opens a view of its own)", verb)) } } +// openModels hosts the models pit. Asking every provider dials, so the list is +// fetched off the input goroutine and installed under the render lock, as a +// live form view is (openLive). +func (in *interactiveInput) openModels() { + in.note("…models") + go func() { + models, warnings := fetchModels(in.loaded) + current := in.subjectModel() + view := newModelsView(models, current, in.chooseModel) + in.mu.Lock() + in.lt.tr.showLivePit("models", view, false) + in.mu.Unlock() + if len(models) == 0 && len(warnings) > 0 { + in.note("models: " + strings.Join(warnings, "; ")) + } + }() +} + +// subjectModel is the aria on screen's system.model, or "" when it cannot be +// read: the ● in the list is a courtesy, not a precondition. +func (in *interactiveInput) subjectModel() string { + cli := in.aria() + if cli == nil { + return "" + } + ctx, cancel := context.WithTimeout(context.Background(), commandTimeout) + defer cancel() + resp, err := cli.Form(ctx) + if err != nil { + return "" + } + raw, ok := resp.Snapshot.Get("system.model") + if !ok { + return "" + } + return valueSummary(raw) +} + +// chooseModel is Enter in the models pit: the subject's system.model becomes +// the chosen id, and system.provider the provider that listed it, in ONE +// patch. A model is only meaningful to its own provider; choosing copilot's +// gpt-5 while the board still said anthropic would send a name Anthropic has +// never heard of on the next turn. When the provider already matches, the +// write lands only the model (the store drops an unchanged key), which is +// exactly `figaro set system.model `. It runs on its own goroutine. +func (in *interactiveInput) chooseModel(c modelChoice) { + cli := in.aria() + id := in.currentID() + if cli == nil { + in.noteErr("models: no aria on screen to set") + return + } + ctx, cancel := context.WithTimeout(context.Background(), commandTimeout) + defer cancel() + set := map[string]json.RawMessage{"system.model": mustJSON(c.model)} + if c.provider != "" { + set["system.provider"] = mustJSON(c.provider) + } + if _, err := cli.Set(ctx, form.Build(form.Snapshot{}, set, nil), 0); err != nil { + in.noteErr("models: " + wireErrorText(err)) + return + } + in.note(fmt.Sprintf("model: %s (%s) on %s", c.model, c.provider, id)) +} + +func mustJSON(s string) json.RawMessage { + b, _ := json.Marshal(s) + return b +} + // --------------------------------------------------------------------------- // The router, run in-process. // --------------------------------------------------------------------------- diff --git a/internal/cli/models_view.go b/internal/cli/models_view.go new file mode 100644 index 00000000..69406b8a --- /dev/null +++ b/internal/cli/models_view.go @@ -0,0 +1,147 @@ +package cli + +// :models AS A PIT YOU CHOOSE IN. +// +// It was a table printed with os.Stdout: round the pager's writer, onto the +// terminal, and nothing on it could be acted on. It is now an itemised live +// view (itemView, pit.go), so the pit's one picker drives it exactly as it +// drives the queue, the form and the tab menu: ^N/^P and j/k move, the wash +// and the marker show the choice, `y` yanks the id, `/` searches. Enter is +// this view's own action, and it makes the chosen model the subject's. + +import ( + "context" + "fmt" + "strconv" + "strings" + "sync" + "time" + + "github.com/jack-work/figaro/internal/config" + "github.com/jack-work/figaro/internal/provider" + "github.com/mattn/go-runewidth" +) + +// modelChoice is what Enter chose: a model is only meaningful to its own +// provider, so the two travel together. +type modelChoice struct { + provider string + model string +} + +// modelsView is the pit's model: the list, which one the subject is on, and +// what to do when one is chosen. +type modelsView struct { + mu sync.Mutex + models []provider.ModelInfo + current string + // choose applies a choice. It dials, so Activate never calls it on the + // input goroutine: Activate runs under the render lock. + choose func(modelChoice) +} + +func newModelsView(models []provider.ModelInfo, current string, choose func(modelChoice)) *modelsView { + return &modelsView{models: models, current: current, choose: choose} +} + +// modelRowID addresses a row by position, because one model id can be served +// by two providers (a gateway re-exporting Anthropic's names) and the row, not +// the id, is what was chosen. +func modelRowID(i int) string { return "model:" + strconv.Itoa(i) } + +// Items renders one row per model, grouped by provider under a heading. The +// subject's current model carries ● so a reader knows where they are before +// choosing where to go. +func (v *modelsView) Items(width int) []pitRow { + v.mu.Lock() + defer v.mu.Unlock() + idW := 0 + for _, m := range v.models { + idW = max(idW, runewidth.StringWidth(m.ID)) + } + idW = min(idW, max(width/2, 16)) + var rows []pitRow + last := "" + for i, m := range v.models { + if m.Provider != last { + rows = append(rows, staticRow(m.Provider)) + last = m.Provider + } + mark := " " + if m.ID == v.current { + mark = "●" + } + text := fmt.Sprintf("%s %s %s", mark, padTo(clipHead(m.ID, idW), idW), modelDetail(m)) + // The provider rides every row as its note, not only the heading: `/` + // walks selectable rows, and a heading is chrome, so `/copilot` found + // nothing among three hundred models until the row said it. + rows = append(rows, pitRow{text: text, note: m.Provider, yank: m.ID, id: modelRowID(i)}) + } + if len(rows) == 0 { + rows = append(rows, staticRow("no models: no provider answered (figaro login )")) + } + return rows +} + +// modelDetail is the row's second column: the name when it says more than the +// id, and the context window and output cap when the provider reported them. +func modelDetail(m provider.ModelInfo) string { + var parts []string + if m.Name != "" && !strings.EqualFold(m.Name, m.ID) { + parts = append(parts, m.Name) + } + if m.ContextWindow > 0 { + parts = append(parts, dashCount(m.ContextWindow)+" ctx") + } + if m.MaxTokens > 0 { + parts = append(parts, dashCount(m.MaxTokens)+" out") + } + return strings.Join(parts, " · ") +} + +// Activate is Enter: choose the model. It HANDS OFF -- it runs under the +// render lock, and the set it causes dials the daemon. +func (v *modelsView) Activate(id string) { + n, err := strconv.Atoi(strings.TrimPrefix(id, "model:")) + v.mu.Lock() + if err != nil || n < 0 || n >= len(v.models) || v.choose == nil { + v.mu.Unlock() + return + } + m := v.models[n] + v.current = m.ID // the mark moves now; the bar says whether it took + choose := v.choose + v.mu.Unlock() + go choose(modelChoice{provider: m.Provider, model: m.ID}) +} + +// Close: the list is a snapshot, holding nothing. +func (v *modelsView) Close() {} + +// fetchModels asks every configured provider for its models, in the order +// `figaro models` lists them. A provider that fails is skipped and reported, +// as the table did. +func fetchModels(loaded *config.Loaded) ([]provider.ModelInfo, []string) { + ensureHush() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + names := loaded.ListProviders() + if len(names) == 0 { + names = KnownProviders() + } + var out []provider.ModelInfo + var warnings []string + for _, name := range names { + prov, _ := buildProvider(loaded, name) + if prov == nil { + continue + } + models, err := prov.Models(ctx) + if err != nil { + warnings = append(warnings, name+": "+err.Error()) + continue + } + out = append(out, models...) + } + return out, warnings +} diff --git a/internal/cli/models_view_test.go b/internal/cli/models_view_test.go new file mode 100644 index 00000000..160e8afb --- /dev/null +++ b/internal/cli/models_view_test.go @@ -0,0 +1,123 @@ +package cli + +import ( + "strings" + "sync" + "testing" + "time" + + "github.com/jack-work/figaro/internal/provider" +) + +// :models IS A PIT YOU CHOOSE IN. It used to be a table printed to os.Stdout +// -- round the pager, onto the terminal -- that nothing could act on. Now it is +// an itemised view: the picker drives it (^N/^P, j/k, the wash, the marker), +// `y` yanks the id, and Enter makes the chosen model the subject's. + +func modelsFixture() []provider.ModelInfo { + return []provider.ModelInfo{ + {Provider: "anthropic", ID: "claude-opus-5", Name: "Claude Opus 5", ContextWindow: 1_000_000, MaxTokens: 32000}, + {Provider: "anthropic", ID: "claude-sonnet-4-5", Name: "Claude Sonnet 4.5", ContextWindow: 200_000, MaxTokens: 64000}, + {Provider: "copilot", ID: "gpt-5", Name: "GPT-5", ContextWindow: 400_000}, + } +} + +func TestModelsViewIsAListYouChooseIn(t *testing.T) { + v := newModelsView(modelsFixture(), "claude-opus-5", nil) + rows := v.Items(100) + selectable := 0 + for _, r := range rows { + if r.selectable() { + selectable++ + if r.yank == "" || r.id == "" { + t.Fatalf("a model row must be yankable and addressable: %+v", r) + } + } + } + if selectable != 3 { + t.Fatalf("%d selectable rows, want one per model", selectable) + } + var current string + for _, r := range rows { + if strings.Contains(r.text, "claude-opus-5") { + current = r.text + } + } + if !strings.Contains(current, "●") { + t.Fatalf("the subject's current model must be marked: %q", current) + } + // `y` copies the bare id, which is what `set system.model` takes. + for _, r := range rows { + if r.id != "" && strings.Contains(r.text, "gpt-5") && r.yank != "gpt-5" { + t.Fatalf("yank = %q, want the model id", r.yank) + } + } +} + +// ENTER SETS system.model, and system.provider with it: a model is only +// meaningful to its own provider, and choosing copilot's gpt-5 while the board +// still says anthropic would send a model name Anthropic has never heard of on +// the next turn. +func TestModelsViewEnterSetsTheModelAndItsProvider(t *testing.T) { + var mu sync.Mutex + var got []modelChoice + done := make(chan struct{}, 1) + v := newModelsView(modelsFixture(), "claude-opus-5", func(c modelChoice) { + mu.Lock() + got = append(got, c) + mu.Unlock() + done <- struct{}{} + }) + var gpt string + for _, r := range v.Items(100) { + if strings.Contains(r.text, "gpt-5") { + gpt = r.id + } + } + v.Activate(gpt) + <-done + mu.Lock() + defer mu.Unlock() + if len(got) != 1 || got[0].model != "gpt-5" || got[0].provider != "copilot" { + t.Fatalf("Enter chose %+v, want gpt-5 on copilot", got) + } +} + +// Activate runs under the render lock: it must hand off, never call the set +// itself (which dials) on the input goroutine. +func TestModelsViewActivateDoesNotBlock(t *testing.T) { + block := make(chan struct{}) + v := newModelsView(modelsFixture(), "", func(modelChoice) { <-block }) + defer close(block) + returned := make(chan struct{}) + go func() { + for _, r := range v.Items(100) { + if r.id != "" { + v.Activate(r.id) + break + } + } + close(returned) + }() + select { + case <-returned: + case <-timeAfter(): + t.Fatal("Activate blocked on the set: it runs under the render lock and must hand off") + } +} + +func timeAfter() <-chan time.Time { return time.After(2 * time.Second) } + +// `/` walks selectable rows; the provider heading is chrome. So each row must +// carry its provider for `/copilot` to land anywhere. +func TestModelsViewRowsCarryTheirProvider(t *testing.T) { + v := newModelsView(modelsFixture(), "", nil) + p := newPicker(v.Items(100)) + if !p.find("copilot", 1) { + t.Fatal("/copilot found no row") + } + row, _ := p.selected() + if row.yank != "gpt-5" { + t.Fatalf("/copilot landed on %q", row.yank) + } +} diff --git a/internal/cli/picker.go b/internal/cli/picker.go index 54d354a6..c9b3c0f9 100644 --- a/internal/cli/picker.go +++ b/internal/cli/picker.go @@ -128,7 +128,8 @@ func (p *picker) find(q string, dir int) bool { if !p.rows[n].selectable() { continue } - if searchContains(p.rows[n].text, q) { + // The note is drawn, so it is searched: a row is what a reader SEES. + if searchContains(p.rows[n].text, q) || searchContains(p.rows[n].note, q) { p.cursor = n p.follow() return true diff --git a/internal/cli/system.go b/internal/cli/system.go index db77fdfd..0a2981a0 100644 --- a/internal/cli/system.go +++ b/internal/cli/system.go @@ -121,33 +121,17 @@ func dashCount(n int) string { // runModels lists provider models. func runModels(loaded *config.Loaded) { - ensureHush() - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - providerNames := loaded.ListProviders() - if len(providerNames) == 0 { - // Fall back to the providers the factory knows how to build. - providerNames = KnownProviders() + models, warnings := fetchModels(loaded) + for _, w := range warnings { + fmt.Fprintf(stderrw, "warning: %s\n", w) } - - w := tabwriter.NewWriter(os.Stdout, 0, 4, 2, ' ', 0) + // The package's writer, never os.Stdout: the pager captures this one, and + // a table printed round it lands on the terminal over the status bar. + w := tabwriter.NewWriter(stdout, 0, 4, 2, ' ', 0) fmt.Fprintf(w, "PROVIDER\tMODEL ID\tNAME\tCONTEXT\tMAX OUT\n") - - for _, name := range providerNames { - prov, _ := buildProvider(loaded, name) - if prov == nil { - continue - } - models, err := prov.Models(ctx) - if err != nil { - fmt.Fprintf(stderrw, "warning: %s: %s\n", name, err) - continue - } - for _, m := range models { - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", - m.Provider, m.ID, m.Name, dashCount(m.ContextWindow), dashCount(m.MaxTokens)) - } + for _, m := range models { + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", + m.Provider, m.ID, m.Name, dashCount(m.ContextWindow), dashCount(m.MaxTokens)) } w.Flush() }