feat(tui): add search/filter to provider picker in setup wizard#400
feat(tui): add search/filter to provider picker in setup wizard#400Ashwinhegde19 wants to merge 2 commits into
Conversation
Add a search input to the provider step of the setup wizard, matching the existing model picker search UX. Typing filters the provider list in real-time via case-insensitive substring matching on name, ID, and aliases. Fixes Gitlawb#392
There was a problem hiding this comment.
Pull request overview
This PR adds an interactive search/filter input to the provider selection step in the TUI setup wizard, bringing the provider picker UX in line with the existing model picker search behavior.
Changes:
- Adds
providerSearchstate plus helper methods for updating and filtering the provider list. - Updates provider-step rendering and navigation to operate on the filtered provider list and display a search input.
- Adds key and paste handling for provider search (printable chars, backspace, Ctrl+U), and clears search when leaving/entering the provider step.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if wizard.oauthMode { | ||
| return | ||
| } | ||
| wizard.providerSearch = "" | ||
| wizard.refreshModels() |
| func (wizard *providerWizardState) renderProviderSearch(width int) string { | ||
| query := strings.TrimSpace(wizard.providerSearch) | ||
| return providerWizardInputLine("search > ", query, "provider name...", width) | ||
| } |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe provider setup wizard now filters the provider-selection step with a search query. It tracks the query in state, filters by provider ID/name/aliases, updates selection and step transitions around the filtered list, and renders the search line plus filtered or empty results. ChangesProvider search/filter feature
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant providerWizard as providerWizardState
participant filteredProviders as filteredProviders()
participant renderProviderStep as renderProviderStep()
User->>providerWizard: type, paste, or press arrow keys
providerWizard->>providerWizard: update providerSearch / selectedProvider
providerWizard->>filteredProviders: compute matching providers
providerWizard->>renderProviderStep: redraw provider step
renderProviderStep->>filteredProviders: read filtered list
renderProviderStep->>User: show search input, matches, or empty-state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/tui/provider_wizard.go (2)
639-659: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOAuth "d" device-login shortcut swallows the letter "d" in search.
When
oauthModeis true and the currently selected (filtered) provider hasOAuthDeviceFlow, pressing "d"/"D" always triggersstartProviderDeviceLogin()(lines 641-645) before falling through to the search-typing handler added at Line 648. This makes it impossible to type "d" into the provider search box in that state — e.g. searching for a provider whose name contains "d" while an unrelated device-capable provider is currently selected.Consider gating the shortcut behind an explicit modifier (e.g. Ctrl+D) or only intercepting it when
providerSearchis empty, so typed search text always wins once the user starts typing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/provider_wizard.go` around lines 639 - 659, The provider wizard key handler in provider_wizard.go is intercepting plain “d”/“D” before the search input logic, so typing into the provider search can accidentally start device login. Update the key handling in the providerWizardStepProvider branch so the device-login shortcut in startProviderDeviceLogin only triggers with an explicit modifier or when providerSearch is empty, and let the appendProviderSearch path win for normal typed characters. Keep the change localized around the existing oauthMode/currentProvider().OAuthDeviceFlow check and the search-handling switch.
460-476: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
providerSearchnot cleared on retreat back to the Method step.
advance()clearsproviderSearchonly when moving forward off the Provider step (line 466).retreat()'sproviderWizardStepProvidercase (transitioning back to Method) doesn't clear it, while theproviderWizardStepEndpointcase redundantly clears it again on the way back to Provider (it's already empty there sinceadvance()cleared it). Net effect: search text typed on the Provider step survives a Left/Esc-back-to-Method → forward-again round trip, silently filtering the list on re-entry, which contradicts the PR's stated intent to clear search "when leaving the provider step."🐛 Proposed fix
case providerWizardStepProvider: wizard.oauthMode = false wizard.oauthErr = "" + wizard.providerSearch = "" wizard.step = providerWizardStepMethodAlso applies to: 518-552
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/provider_wizard.go` around lines 460 - 476, The provider search text is only being cleared in advance() when leaving providerWizardStepProvider, so it can survive a retreat() back to the Method step and unexpectedly filter the list on re-entry. Update the retreat() handling for providerWizardStepProvider in internal/tui/provider_wizard.go to mirror the forward path by clearing providerSearch (and keeping the existing refresh/reset behavior consistent), and remove any redundant clearing from the providerWizardStepEndpoint backtrack path so providerSearch is reset exactly when leaving the Provider step.
🧹 Nitpick comments (1)
internal/tui/provider_wizard.go (1)
913-956: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider unit tests for the new filtering helpers.
filteredProviders/providerMatchesQueryare pure and easy to test (ID/name/alias matching, case-insensitivity, empty query, no-match case). No test changes are visible in this diff for them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/provider_wizard.go` around lines 913 - 956, Add unit tests for the new filtering helpers in providerWizardState: cover filteredProviders and providerMatchesQuery with empty query returning all providers, case-insensitive matching against ID/name/aliases, and a no-match case. Use the existing symbols providerMatchesQuery and filteredProviders to locate the logic, and keep the tests focused on the pure behavior without changing unrelated wizard flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/tui/provider_wizard.go`:
- Around line 639-659: The provider wizard key handler in provider_wizard.go is
intercepting plain “d”/“D” before the search input logic, so typing into the
provider search can accidentally start device login. Update the key handling in
the providerWizardStepProvider branch so the device-login shortcut in
startProviderDeviceLogin only triggers with an explicit modifier or when
providerSearch is empty, and let the appendProviderSearch path win for normal
typed characters. Keep the change localized around the existing
oauthMode/currentProvider().OAuthDeviceFlow check and the search-handling
switch.
- Around line 460-476: The provider search text is only being cleared in
advance() when leaving providerWizardStepProvider, so it can survive a retreat()
back to the Method step and unexpectedly filter the list on re-entry. Update the
retreat() handling for providerWizardStepProvider in
internal/tui/provider_wizard.go to mirror the forward path by clearing
providerSearch (and keeping the existing refresh/reset behavior consistent), and
remove any redundant clearing from the providerWizardStepEndpoint backtrack path
so providerSearch is reset exactly when leaving the Provider step.
---
Nitpick comments:
In `@internal/tui/provider_wizard.go`:
- Around line 913-956: Add unit tests for the new filtering helpers in
providerWizardState: cover filteredProviders and providerMatchesQuery with empty
query returning all providers, case-insensitive matching against
ID/name/aliases, and a no-match case. Use the existing symbols
providerMatchesQuery and filteredProviders to locate the logic, and keep the
tests focused on the pure behavior without changing unrelated wizard flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fea8c6cc-9e1b-4def-8d3d-936e0ca27aff
📒 Files selected for processing (1)
internal/tui/provider_wizard.go
Clearing providerSearch before using currentProvider() caused the wrong provider to be selected — selectedProvider is an index into the filtered slice, and clearing the search swaps to the full unfiltered list. Also update placeholder to reflect that search matches ID and aliases.
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Verdict: Request changes — the core is right, but there's one confirmed edge bug and no tests
Good feature, and the hard part is done correctly: advance() remaps the filtered selection index back to the full-list index before clearing the search, so selection is preserved across the filtered→full swap. I verified the happy path locally — filtering anthropic then pressing Enter advances with anthropic (remapped to full index 2), and the existing wizard tests still pass. gofmt/vet clean.
Two things to fix before merge.
🔴 Confirmed bug: Enter on an empty match advances with the wrong provider
Type a query that matches nothing → the list shows no matching providers → press Enter, and the wizard advances off the provider step selecting gitlawb-opengateway (the first full-list provider), not what the user intended. I reproduced it:
after Enter on empty match: step=Endpoint selectedProvider=0 provider="gitlawb-opengateway"
Root cause: in advance()'s providerWizardStepProvider case, when currentProvider() returns the empty descriptor (no match), the remap is skipped, providerSearch is cleared, and currentProvider() then falls back to full-list [selectedProvider] which the last keystroke reset to 0. Enter isn't caught by the search key-switch, so it reaches advance().
Fix: guard the empty case at the top of that advance() branch (right after the oauthMode return):
if len(wizard.filteredProviders()) == 0 {
return // nothing selectable; Enter is a no-op on an empty filter
}🟡 No tests for the filter/index logic
The PR changes only provider_wizard.go — no tests. This is exactly the fragile index-mapping surface that PR #402 just had to refactor the wizard tests around, so it needs coverage. Please add table/unit tests for:
- filtering narrows the list (
filteredProviders()/providerMatchesQueryon name, id, alias, case-insensitive), - selecting a filtered provider advances to that exact provider (the remap — e.g. filter to a mid/late provider, Enter, assert
currentProvider().ID), - empty match + Enter is a no-op (guards the bug above — red-green),
- clearing the search (
Ctrl+U/ backspace to empty) restores the full list with the selection consistent.
These are pure providerWizardState unit tests — no full TUI run needed (see the probes I used above).
Nit
appendProviderSearch/deleteProviderSearchRune reset selectedProvider = 0 on every edit (good — avoids a stale index past a narrowed filter); just make sure the new tests lock that in so a future refactor can't regress it.
Once the empty-Enter guard and tests are in, this is a clean, welcome UX win. Thanks @Ashwinhegde19!
Summary
/modelcommand)Changes
internal/tui/provider_wizard.go: 92 additions, 6 deletionsWhat changed
providerSearchfield toproviderWizardStatestructappendProviderSearch(),deleteProviderSearchRune(),filteredProviders(),providerMatchesQuery()methodsrenderProviderStep()to show search input and render filtered resultscurrentProvider()to use filtered list when on provider stepmove()to navigate within filtered resultsMotivation
With 28+ providers now in the catalog, scrolling through the full list is tedious. The model picker already has search — this brings the same UX to the provider picker.
Testing
go build ./...passesgo test ./internal/tui/...passesgo vet ./internal/tui/...passesFixes #392
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Ctrl+u.