diff --git a/docs/GUIDE.md b/docs/GUIDE.md index f6876244e3..407623250e 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -326,9 +326,9 @@ The second page is `Daily limit` and `Chat model`. The chat model resolves from `--model`, then saved `model.talk`, then `CODEAF_MODEL`, -then `~deepseek/deepseek-v4-flash-latest`. The last value is a floating alias. Besides -OpenRouter, the connection screen supports DeepSeek, Z.ai, Moonshot, MiniMax, Alibaba -Qwen, Codex through a ChatGPT plan, Ollama, and a custom OpenAI-compatible provider. +then `~deepseek/deepseek-v4-flash-latest`. The last value is a floating alias. The connection +screen shows every built-in provider in one flat list, including a custom +OpenAI-compatible API option. The same supported providers can be managed without opening the chat with `codeaf connect` and `codeaf disconnect`; a qualified slug such as `qwen/` selects its provider. diff --git a/docs/changes/unreleased/1795-provider-list-extensible.md b/docs/changes/unreleased/1795-provider-list-extensible.md new file mode 100644 index 0000000000..52b8110d8a --- /dev/null +++ b/docs/changes/unreleased/1795-provider-list-extensible.md @@ -0,0 +1,15 @@ +--- +kind: fixed +title: Built-in provider lists accept appended catalog rows +pr: 1795 +surface: [chat, docs] +invalidates: + - "The provider chooser kept its key hint even when it pushed OpenRouter off a short terminal. The hint now yields before any provider row, so a tenth entry fits at 40x10." + - "Provider-menu tests pinned the catalog size or assumed Custom API was last. Counts now follow the catalog, appended sources preserve the established order, and custom-row tests select by identity." + - "The manual repeated numeric built-in provider counts and complete provider lists. Those passages and the guide now describe every built-in provider without a fixed count, and a manual guard rejects count claims." +--- + +The built-in catalog is unchanged. A throwaway source appended to Vendored() +passes the full tui3 and manual suites, and the short chooser shows every row +without its footer. The separate provider addition can land without changing +these setup tests or manual passages. diff --git a/internal/manual/chat/commands.md b/internal/manual/chat/commands.md index 62793aa19d..540c986564 100644 --- a/internal/manual/chat/commands.md +++ b/internal/manual/chat/commands.md @@ -1729,7 +1729,7 @@ status sheet. Change that machine's profile there. ## /connect — your connected accounts `/connect` (or `/connections`) opens the connect panel. Its pinned `providers` group -holds the six built-in model providers plus every one already connected; the account +holds every built-in model provider plus every one already connected; the account catalog groups follow it. The Codex row says `browser`; enter opens the sign-in road and the waiting card keeps the address available to copy. The other listed providers say what they need. Pick a row and connect it. There is no argument form. **Custom OpenAI-compatible API** connects a custom provider: it asks for a diff --git a/internal/manual/chat/getting-started.md b/internal/manual/chat/getting-started.md index 0f7b8ffbbf..4a5a712a4d 100644 --- a/internal/manual/chat/getting-started.md +++ b/internal/manual/chat/getting-started.md @@ -14,9 +14,8 @@ terminal* page, under *How do I install or update codeaf*. ## Getting started — first time setup, what happens the first time I run codeaf The first time `codeaf` opens a new local conversation with no provider connected, -setup begins with **choose a model provider**. Its one flat list shows all nine options: -**OpenRouter**, **Ollama**, **Codex**, **DeepSeek**, **Z.ai**, **Moonshot**, **MiniMax**, -**Alibaba Qwen**, and **Custom OpenAI-compatible API**. There is no scrolling viewport +setup begins with **choose a model provider**. Its one flat list shows every built-in provider. +On a short terminal the key hint yields before any provider row. There is no scrolling viewport or page counter. Arrow keys or the mouse wheel move the selection; `home` and `end` jump to either end. Press `enter` or click a row to choose it. No key is collected on the chooser. diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index e773b67333..ce26e86d46 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -279,7 +279,7 @@ key removes its models from subsequent lists. A cold catalog never adds built-in This applies to `/model`, task rooms, settings slots and roles, Home drafts and the task composer. Each slot still keeps only models with the capabilities it needs. A provider that cannot list models has a non-selectable notice; `+ add a provider` opens connection -setup and is not a model. That menu includes all nine initial provider options, including +setup and is not a model. That menu includes every built-in provider from initial setup, including OpenRouter, and labels each `connected` or `not connected`. You can add OpenRouter after Ollama without repeating onboarding; both catalogs then appear together. `ctrl+r` refreshes the known lists. diff --git a/internal/manual/chat/services.md b/internal/manual/chat/services.md index 109346d4df..4c94381242 100644 --- a/internal/manual/chat/services.md +++ b/internal/manual/chat/services.md @@ -7,8 +7,8 @@ something else again: long-running background processes, covered by their own pa ## Add a key — connect a provider, add an api key, use a different provider An api key for another provider, or another model provider, is added here. Open `/connect` or -`/connections`. The `providers` group includes all nine setup options: OpenRouter, Ollama, -Codex, DeepSeek, Z.ai, Moonshot, MiniMax, Alibaba Qwen and **Custom OpenAI-compatible API**. +`/connections`. The `providers` group includes every built-in provider from setup, +including **Custom OpenAI-compatible API**. Connected providers appear first, followed by the remaining options and any saved custom instances. Once a custom provider is connected, a `+ add a provider` row. Codex says `browser`; it signs in a ChatGPT plan instead of asking for an API key. OpenRouter offers the same browser/key @@ -26,8 +26,8 @@ provider in that order; with only the default provider, the picker remains ungro ## Add OpenRouter after Ollama — add another provider from the model menu -Open `/model` and choose `+ add a provider`. Its provider list shows the same nine -options as initial setup, with `connected` or `not connected` beside each. Local servers +Open `/model` and choose `+ add a provider`. Its flat list shows every built-in +provider from initial setup, with `connected` or `not connected` beside each. Local servers found on this machine appear separately. An unconnected row starts its connection; a connected row says `enter manages` and opens `/connect` focused on that provider. The custom API option says `connected · add another` when a custom connection exists; diff --git a/internal/manual/chat/starting-codeaf.md b/internal/manual/chat/starting-codeaf.md index 9db6c071d8..be5809697a 100644 --- a/internal/manual/chat/starting-codeaf.md +++ b/internal/manual/chat/starting-codeaf.md @@ -65,8 +65,8 @@ that is a different program and nothing here talks to it. | `codeaf chat --host devbox` | the chat here, the work on another machine | **The very first launch on a machine with nothing configured** begins with **choose a -model provider**. One flat list shows all nine options: OpenRouter, Ollama, Codex, DeepSeek, -Z.ai, Moonshot, MiniMax, Alibaba Qwen and the custom API connection. +model provider**. One flat list shows every built-in provider. +On a short terminal the key hint yields before any provider row. The chosen provider's connection screen follows, then the chat model and daily limit. Ollama asks for no key. OpenRouter and Codex say `enter connects in browser` on their connection screens and wait for Enter before opening sign-in. `alt+left` returns to provider choice; `esc` skips an idle setup diff --git a/internal/manual/providercatalog_test.go b/internal/manual/providercatalog_test.go new file mode 100644 index 0000000000..1fe4875d41 --- /dev/null +++ b/internal/manual/providercatalog_test.go @@ -0,0 +1,100 @@ +package manual + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// THE CATALOG OWNS ITS SIZE. A count in prose goes stale when a contributor +// adds a source, even if every connection door already reads the catalog. +func TestManualProviderListsDoNotPinTheCatalogSize(t *testing.T) { + paths := []string{"../../docs/GUIDE.md"} + for _, folder := range []string{"chat", "pages"} { + pages, err := filepath.Glob(folder + "/*.md") + if err != nil { + t.Fatal(err) + } + if len(pages) == 0 { + t.Fatalf("no manual pages in %s", folder) + } + paths = append(paths, pages...) + } + for _, path := range paths { + body, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, paragraph := range strings.Split(string(body), "\n\n") { + if pinsProviderCount(paragraph) { + t.Errorf("%s pins the built-in provider count: %s", path, flatten(paragraph)) + } + } + } +} + +var providerCount = regexp.MustCompile(`\b(?:[0-9]+|zero|one|two|three|four|five|six|seven|eight|nine|ten|eleven|twelve|thirteen|fourteen|fifteen|sixteen|seventeen|eighteen|nineteen|twenty|thirty|forty|fifty|sixty|seventy|eighty|ninety|hundred)(?:[- ](?:one|two|three|four|five|six|seven|eight|nine))?\s+(?:(?:built-in|supported|initial|setup|model|provider)\s+)*(?:providers|options)\b`) +var providerCountLead = regexp.MustCompile(`\b(?:all|same|shows?|includes?|supports?|offers?|lists?|holds?|contains?|has|there are)\s+(?:exactly\s+)?$`) +var providerOptionCatalog = regexp.MustCompile(`\b(?:provider (?:list|catalog|chooser|menu)|providers (?:group|list|catalog|menu)|flat list)\b`) + +// Match catalog claims, including wrapped sentences and Markdown emphasis, +// while allowing quantities about connected providers or a router's endpoints. +func pinsProviderCount(paragraph string) bool { + text := strings.ToLower(strings.Join(strings.Fields(strings.NewReplacer("*", "", "`", "", "_", "").Replace(paragraph)), " ")) + if !strings.Contains(text, "provider") { + return false + } + for _, at := range providerCount.FindAllStringIndex(text, -1) { + claim := text[at[0]:at[1]] + // A provider can offer counted connection choices without counting the + // catalog. Only provider options or a catalog list pin its size. + if strings.HasSuffix(claim, "options") && !strings.Contains(claim, "provider options") && !providerOptionCatalog.MatchString(text) { + continue + } + for _, catalogWord := range []string{"built-in", "supported", "initial", "setup"} { + if strings.Contains(claim, catalogWord) { + return true + } + } + if providerCountLead.MatchString(text[:at[0]]) || strings.HasPrefix(strings.TrimSpace(text[at[1]:]), ":") { + return true + } + } + return false +} + +func TestProviderCountGuardRecognizesCatalogClaims(t *testing.T) { + for _, claim := range []string{ + "One flat provider list shows all nine options: OpenRouter, Ollama.", + "The providers group includes all 10 setup options.", + "That menu includes all ten initial provider options.", + "Its provider list shows the same nine\noptions as initial setup.", + "It holds the six built-in model providers plus every one connected.", + "There are **11** supported providers.", + "There are twenty-one provider options.", + "12 providers: OpenRouter, Ollama.", + } { + if !pinsProviderCount(claim) { + t.Errorf("count guard missed %q", claim) + } + } + for _, claim := range []string{ + "Its flat list shows every built-in provider.", + "With two or more providers, models are grouped by provider.", + "Two providers may publish the same model.", + "Probe each of the two providers your next message may reach.", + "Choose between two connected providers.", + "The provider returns 10 models.", + "The provider offers two options: browser sign-in or an API key.", + "For this provider, setup offers two options: sign in or paste a key.", + "Two providers each offer two options: browser sign-in or an API key.", + "The router tried nine endpoints behind the provider.", + "Compare two options.", + } { + if pinsProviderCount(claim) { + t.Errorf("count guard rejected %q", claim) + } + } +} diff --git a/internal/modelsource/modelsource_test.go b/internal/modelsource/modelsource_test.go index 2d4134a39e..6f8303e29a 100644 --- a/internal/modelsource/modelsource_test.go +++ b/internal/modelsource/modelsource_test.go @@ -129,8 +129,9 @@ func TestVendoredRowsCarryTheCodexServiceInItsDecidedPlace(t *testing.T) { // C12: Codex is a model service whose models are qualified on every surface. rows := Vendored() want := []string{"deepseek", "z-ai", "moonshot", "minimax", "qwen", "codex", "ollama", "custom"} - if len(rows) != len(want) { - t.Fatalf("vendored rows = %d, want %d", len(rows), len(want)) + // The established order stays fixed, but an appended provider is welcome. + if len(rows) < len(want) { + t.Fatalf("vendored rows = %d, want at least %d", len(rows), len(want)) } for i := range want { if rows[i].ID != want[i] { diff --git a/internal/tui3/addprovider_scroll_test.go b/internal/tui3/addprovider_scroll_test.go index ae0ef0103a..a1d7279a25 100644 --- a/internal/tui3/addprovider_scroll_test.go +++ b/internal/tui3/addprovider_scroll_test.go @@ -15,12 +15,14 @@ func TestAddProviderSelectionStaysVisibleInShortTerminal(t *testing.T) { t.Run(fmt.Sprintf("rows_%d", height), func(t *testing.T) { p := addProviderPanel{open: true} p.rebuild(nil, nil, modelsource.Set{}) + seenCustom := false assertVisible := func() { t.Helper() selected, ok := p.current() if !ok { t.Fatal("selection landed on a heading") } + seenCustom = seenCustom || selected.custom rows := p.draw(100, height, a.pal, -1) if len(rows) > height { t.Fatalf("%d rows exceeds %d", len(rows), height) @@ -33,7 +35,7 @@ func TestAddProviderSelectionStaysVisibleInShortTerminal(t *testing.T) { assertVisible() p.move(1) } - if selected, _ := p.current(); !selected.custom { + if !seenCustom { t.Fatal("custom address row was not reachable") } for range len(p.items) { @@ -49,8 +51,14 @@ func TestProviderDiscoveryPreservesSelectionAndIgnoresOldPanel(t *testing.T) { a.openAddProvider(false) old := a.addPanel.probeContext for range len(a.addPanel.items) { + if item, _ := a.addPanel.current(); item.custom { + break + } a.addPanel.move(1) } + if item, _ := a.addPanel.current(); !item.custom { + t.Fatal("custom provider is absent before discovery") + } a.Update(localServersProbedMsg{ctx: old, probes: []LocalServerProbe{{Name: "local example", Address: "http://127.0.0.1:1234/v1"}}}) if item, _ := a.addPanel.current(); !item.custom { t.Fatal("discovery moved the selected custom provider") diff --git a/internal/tui3/onboardingproviders.go b/internal/tui3/onboardingproviders.go index 9f5751fa73..43a21602ed 100644 --- a/internal/tui3/onboardingproviders.go +++ b/internal/tui3/onboardingproviders.go @@ -246,10 +246,14 @@ func (a *app) setupProvidersFrame(width, height int) ([]string, int, int) { if inner < 58 { footer = "enter chooses · " + setupSkipKeysWord } - if height >= len(rows)+2 { - body = append(body, "") + // THE PROVIDERS COME BEFORE THE HINT. Keep the footer only when it cannot + // displace a provider; the shared block already lets the heading yield. + if height > len(rows) { + if height >= len(rows)+2 { + body = append(body, "") + } + body = append(body, a.pal.dim(footer)) } - body = append(body, a.pal.dim(footer)) return a.setupProviderBlock(body, hits, width, height, -1, 0) } diff --git a/internal/tui3/onboardingproviders_test.go b/internal/tui3/onboardingproviders_test.go index 2289da617a..dacc639966 100644 --- a/internal/tui3/onboardingproviders_test.go +++ b/internal/tui3/onboardingproviders_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "regexp" "strings" "testing" @@ -28,13 +29,18 @@ func TestProviderChooserBackSkipPasteAndShortPointerRows(t *testing.T) { } a.width, a.height = 40, 12 - a.setup.providerAt = len(a.setupProviderRows()) - 1 + for at, row := range a.setupProviderRows() { + if row.id == modelsource.CustomID { + a.setup.providerAt = at + break + } + } a.touch() screen := setupScreen(a) if strings.Contains(screen, "Skip for now") || strings.Contains(screen, "More providers") || !strings.Contains(screen, setupSkipKeysWord) { t.Fatal(screen) } - hit := a.setup.providerHits[len(a.setup.providerHits)-1] + hit := a.setup.providerHits[a.setup.providerAt] a.setupProviderPress(hit.x, hit.y) if a.setup.provider != "custom" || a.connPanel.entry == nil || a.connPanel.entry.blank != "base URL" { t.Fatal("short-screen click did not select the provider") @@ -191,6 +197,7 @@ func TestOnboardingCodexSignInIsVisibleAndLateFlowsAreCancelled(t *testing.T) { func TestProviderChooserShowsEverySupportedProviderInOneFlatList(t *testing.T) { a, _, _ := setupProviderApp(t, nil) + pageCounter := regexp.MustCompile(`\bof \d+\b`) rows := a.setupProviderRows() if len(rows) != len(modelsource.Vendored())+1 { t.Fatalf("registry providers=%v", rows) @@ -201,7 +208,7 @@ func TestProviderChooserShowsEverySupportedProviderInOneFlatList(t *testing.T) { a.setup.providerAt = selected a.touch() screen := setupScreen(a) - if len(a.setup.providerHits) != len(rows) || strings.Contains(screen, "More providers") || strings.Contains(screen, "Skip for now") || strings.Contains(screen, "of 9") { + if len(a.setup.providerHits) != len(rows) || strings.Contains(screen, "More providers") || strings.Contains(screen, "Skip for now") || pageCounter.MatchString(screen) { t.Fatalf("flat list at %dx%d: %s", size.width, size.height, screen) } for at, row := range rows { @@ -213,6 +220,32 @@ func TestProviderChooserShowsEverySupportedProviderInOneFlatList(t *testing.T) { } } +func TestProviderChooserYieldsTheFooterBeforeAnAddedProvider(t *testing.T) { + a, _, _ := setupProviderApp(t, nil) + a.modelCatalog = append(modelsource.Vendored(), modelsource.Source{ID: "extra", Name: "Extra provider"}) + rows := a.setupProviderRows() + for _, height := range []int{len(rows), len(rows) + 1} { + a.width, a.height = 40, height + for _, selected := range []int{0, len(rows) - 1} { + a.setup.providerAt = selected + a.touch() + screen := setupScreen(a) + if len(a.setup.providerHits) != len(rows) { + t.Fatalf("provider rows clipped at 40x%d: %s", height, screen) + } + for at, row := range rows { + hit := a.setup.providerHits[at] + if hit.at != at || hit.y < 0 || hit.y >= height || !strings.Contains(screen, row.name) { + t.Fatalf("%q missing at 40x%d: %s", row.id, height, screen) + } + } + if got, want := strings.Contains(screen, setupSkipKeysWord), height > len(rows); got != want { + t.Fatalf("footer visible=%v, want %v at 40x%d: %s", got, want, height, screen) + } + } + } +} + func TestBrowserSetupLinksStayOnOneRowAndCopyTheWholeAuthorizationURL(t *testing.T) { target := "https://auth.example/authorize?state=" + strings.Repeat("proof", 120) + "&redirect_uri=http%3A%2F%2Flocalhost%3A9999" for _, provider := range []string{"codex", "openrouter"} { diff --git a/internal/tui3/provideraccess_test.go b/internal/tui3/provideraccess_test.go index 2760c34cc3..708e5af047 100644 --- a/internal/tui3/provideraccess_test.go +++ b/internal/tui3/provideraccess_test.go @@ -61,7 +61,7 @@ func TestLaterProviderMenusShowTheInitialCatalogAndActualConnectionStatus(t *tes for _, row := range a.setupProviderRows() { initialIDs, initialNames = append(initialIDs, row.id), append(initialNames, row.name) } - if len(ids) != 9 || !reflect.DeepEqual(ids, initialIDs) || !reflect.DeepEqual(names, initialNames) { + if len(ids) != len(modelsource.Vendored())+1 || !reflect.DeepEqual(ids, initialIDs) || !reflect.DeepEqual(names, initialNames) { t.Fatalf("later catalog = %v/%v, initial = %v/%v", ids, names, initialIDs, initialNames) } seen := map[string]bool{}