feat(setup): convert router tier provider and model fields to pickers… - #191
feat(setup): convert router tier provider and model fields to pickers…#191Preciousuche wants to merge 5 commits into
Conversation
andreapn
left a comment
There was a problem hiding this comment.
Thanks for picking this up — the shape is right, and the choice to keep the same value/onChange path so the config shape is unchanged is exactly what the issue asked for. The quality gate is green on this branch: tsc --noEmit clean, eslint src clean, 139 tests passing across the 5 affected files.
Requesting changes because two of the issue's four acceptance criteria don't hold in the common case, and the provider dropdown works against how the router actually behaves at runtime.
Acceptance criteria
| # | Criterion | Status |
|---|---|---|
| 1 | picking a provider populates the model options for every tier row | |
| 2 | selecting a model writes the same value the text input writes today | ✅ |
| 3 | an id not in the catalog can still be entered, and saving it shows a warning | |
| 4 | the image tier only offers vision-capable models | ✅ (subject to 1) |
Blocker 1 — the picker goes silently empty for any non-configured provider
models.list is not a global catalog. It aggregates the currently configured provider chain (selector.py:214), and ModelCatalog is only fetched for openrouter / bankr / opencap (boot.py:1423-1465). This PR calls it with {} and then filters client-side on m.provider === row.provider.
Repro (rendered RouterSection with models.list returning only anthropic models, as a real anthropic-configured gateway does):
fireEvent.change(screen.getByLabelText('c0 provider'), { target: { value: 'openai' } })
OPTIONS after switching to openai: 0
any empty-state text rendered? false
Zero options, no explanation. To an operator that reads as a broken control.
Suggested fix: fall back to catalog.router.judge.profiles[provider].models. It is per-provider, ships offline in onboarding.catalog (router_specs.py:72), and this very section already renders it for the Judge dropdown (RouterSection.tsx:85-89). At minimum, render an explicit "no catalog models for <provider> — type an id" hint instead of an empty list.
Blocker 2 — the unknown-model warning is gated off exactly when it matters
collectAndSave wraps the whole validation in if (allModels.length > 0). When models.list returns [] — local providers, no API key saved yet, a failed RPC — no warning is produced at all.
Repro:
models.list -> []
model = 'totally-made-up-model'
click Save Router
toast.warning calls: 0
onSave called: yes
That is the exact behaviour the issue calls out as unacceptable: "Warn, don't block — unknown ids are legitimate; silently accepting a typo is not." Please either warn from the fallback list above, or surface why validation could not run.
Blocker 3 — the provider cell should not become a 13-option dropdown
The issue raised this as an open question ("Worth asking whether it should be editable at all... a read-only chip would say more"). The runtime answers it — boot.py:1106-1120:
tiers only select the MODEL; requests always go through
llm.provider— mismatched tiers are degraded tollm.modelon local providers
So a per-tier provider that differs from llm.provider is a no-op at best and a silent model downgrade at worst. Before this PR, reaching that state meant typing a whole provider id by hand. After it, it is one click from a dropdown, with no warning anywhere in the UI — only a boot-time log the operator will never see.
Preferred: make the provider cell a read-only chip showing llm.provider. If it stays editable, it needs an inline warning on any row where row.provider !== configuredProvider, stating that tiers only select the model.
Not addressed from the issue
The issue's second cost bullet — "You can't read what you typed", anthropic/claude-opus-4.8 rendering as anthropic/claude-opus. The model column is still minmax(10rem, 1.45fr) at 0.85rem (setup.css:566-568); no CSS changed here. A picker reduces typing but the displayed value still truncates. Widening the column and/or adding title={row.model} to the input would close it.
Smaller items
AppShell.test.tsxtimeout bump to 5000 is unrelated to this issue and raises a flaky test's timeout rather than diagnosing it. Please drop it from this PR and file it separately.- Test coverage gaps: no test for the "provider has no catalog models" path or the "
models.listfails/returns empty" path — the two paths that carry the bugs above. import { QueryClient, QueryClientProvider }sits mid-file inRouterSection.test.tsx(aftercatalogWithTiers). Lint does not enforce it, but please move it to the import block.<datalist>accessibility: norole="combobox"/aria-expanded, and option label text (the ctx + pricing metadata) renders inconsistently across browsers — Safari generally shows values only. Givencomponents/uihas no Command/Popover primitive yet,datalistis a defensible pragmatic choice; just be aware the "richer metadata" benefit is browser-dependent.- Branch is 12 commits behind main; please rebase.
Happy to re-review once the three blockers are addressed.
|
I will work on the corrections
…On Mon, Aug 3, 2026 at 12:08 PM andreapn.eth ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thanks for picking this up — the shape is right, and the choice to keep
the same value/onChange path so the config shape is unchanged is exactly
what the issue asked for. The quality gate is green on this branch: tsc
--noEmit clean, eslint src clean, 139 tests passing across the 5 affected
files.
Requesting changes because two of the issue's four acceptance criteria
don't hold in the common case, and the provider dropdown works against how
the router actually behaves at runtime.
Acceptance criteria
# Criterion Status
1 picking a provider populates the model options for every tier row
|
|
|
Corrections have been made, please review. |
Description:
This PR replaces the manual free-text inputs for Provider and Model in the Router Tiers setup panel with interactive select dropdowns and comboboxes.
<SetupSelect>populated from the configured catalog providers.<input list={...} />+<datalist>) dynamically populated from themodels.listRPC call.image_modelrow automatically filters suggestions to only include models withvisioncapabilities.128k ctx) and input/output token pricing per 1M (e.g.$1.50/$3.00 per 1M).toast.warning) if a typed model ID is not in the catalog (warn-only, does not block save).RouterSection.test.tsxverifying these features, and updates mock setups to pass the entire test suite.Closes #142