fix(macos): filter provider picker to active connections (iter3 QA #1)#30330
Conversation
Parity with web PR #6509. The Settings → AI → Profile editor was listing every provider in the catalog regardless of whether the user had an active connection for it, so picking one bound the profile to a route the daemon would skip on dispatch. `InferenceProfileEditor.availableProviderIds` now: - Falls back to the full catalog when `connections` is empty (sheet is still loading, or an older daemon without the connections API). - Filters `store.dynamicProviderIds` to providers with at least one ACTIVE `ProviderConnection`. - Always keeps the currently-bound `profile.provider` in the list so editing a stale profile (its connection was later disabled) still renders a sensible trigger. Adds an empty-state hint under the picker pointing users to Providers when no active connections exist, matching the web modal's hint. 4 regression tests covering hide-inactive, stale-binding recovery, pre-load fallback, and the all-disabled empty case.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 219c19ed07
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// (PR #6509). | ||
| var availableProviderIds: [String] { | ||
| store.dynamicProviderIds | ||
| guard !connections.isEmpty else { return store.dynamicProviderIds } |
There was a problem hiding this comment.
The new fallback guard !connections.isEmpty else { return store.dynamicProviderIds } treats an actually-empty listProviderConnections response the same as the pre-load state, so users with zero configured connections still see the full provider catalog and can save profiles bound to non-dispatchable providers. This leaves the original QA trap in place for fresh workspaces and also suppresses the new empty-state hint in that scenario; the fallback needs a separate "connections loaded" signal instead of keying on emptiness alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in a351050.
Promoted connections from [ProviderConnection] = [] to [ProviderConnection]? = nil so nil (pre-load, fetch in flight) is distinct from [] (daemon returned zero). The fallback to the full catalog now only fires for the nil case; loaded-but-empty falls through to the filter and yields empty, surfacing the empty-state hint that steers the user to Providers.
InferenceProfilesSheet tracks the same nil/loaded state — refreshConnections() only writes on a successful response, so the flip from nil → [] happens exactly when the daemon confirms ground truth.
Added testAvailableProviderIdsIsEmptyWhenConnectionsLoadedButEmpty to lock in the fresh-workspace case and renamed the pre-load test to ...WhenConnectionsAreNil since the old name became misleading.
Web sibling (#6509, already merged) has the same trap — follow-up PR queued.
@codex review
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. Another round soon, please!
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| var availableProviderIds: [String] { | ||
| store.dynamicProviderIds | ||
| guard !connections.isEmpty else { return store.dynamicProviderIds } | ||
|
|
||
| var activeProviderSet = Set<String>() | ||
| for connection in connections where connection.status == .active { | ||
| activeProviderSet.insert(connection.provider) | ||
| } | ||
| if let bound = profile.provider, !bound.isEmpty { | ||
| activeProviderSet.insert(bound) | ||
| } | ||
| return store.dynamicProviderIds.filter { activeProviderSet.contains($0) } |
There was a problem hiding this comment.
🚩 Stale-provider profiles can still be saved without changing the provider
The availableProviderIds filter at InferenceProfileEditor.swift:336-346 keeps the currently-bound provider in the picker even when it has no active connections. This is intentional for UX (so the user sees the stale binding and can change it). However, note that canSave at InferenceProfileEditor.swift:115-117 does NOT gate on whether the selected provider has active connections — it only checks isProviderMissing, isModelMissing, and isModelInvalid. This means a user can open an existing profile bound to a provider with only disabled connections and hit Save without changing the provider, persisting a profile the daemon can't dispatch through. The PR's stated goal is to prevent picking such providers (which it does for new selections), but the save-side validation gap may warrant a follow-up if the intent is stricter enforcement.
Was this helpful? React with 👍 or 👎 to provide feedback.
The pre-load fallback in availableProviderIds was `guard !connections.isEmpty`, which conflated two distinct states: - Connections not yet fetched (`.task` still running, daemon round-trip in flight) → want full catalog so the picker isn't empty during the gap. - Connections fetched and confirmed empty (fresh workspace, nothing configured) → want filter to yield empty + empty-state hint, otherwise the user can still bind a profile to a non-dispatchable provider — the exact QA trap this PR was supposed to close. Promote `connections` from `[ProviderConnection] = []` to `[ProviderConnection]? = nil`. `nil` is pre-load. `[]` is loaded-and-empty. The parent (`InferenceProfilesSheet`) now tracks the same nil-vs-loaded distinction; `refreshConnections()` continues to only write on a successful daemon response, so the optional flips nil → [] once we have ground truth and stays nil through transport failures (stale-but-correct beats blank). `availableConnectionsForProvider` falls back to `[]` during pre-load so the connection sub-dropdown stays hidden until we have data. Renames `testAvailableProviderIdsFallsBackToFullCatalogWhenConnectionsEmpty` to `...WhenConnectionsAreNil` (the old name was misleading after the type change) and adds `testAvailableProviderIdsIsEmptyWhenConnectionsLoadedButEmpty` to lock in the fresh-workspace case. Addresses Codex P1 in #30330 (comment). The web sibling (#6509, already merged) has the same trap; a follow-up PR will mirror this fix.
1 participant
What
macOS sibling to web PR vellum-ai/vellum-assistant-platform#6509. Iter3 QA Issue #1: the Settings → AI → Profile editor on macOS lists every provider in the catalog regardless of whether the user has an active connection for it. Picking a provider with no active connection binds the profile to a route the daemon will skip on dispatch — same trap we fixed on web.
How
InferenceProfileEditor.availableProviderIdsis now a filtered computed property:connectionsis empty (sheet is still fetching its firstlistConnectionsresponse, or talking to an older daemon that predates the connections API), return the full catalog so the picker isn't empty on first open..status == .activeconnection, then filterstore.dynamicProviderIdsagainst it.profile.providerin the visible set. If a connection was disabled after a profile was bound to it, the trigger still shows the bound value (so the user can read it and replace it) instead of going empty.Adds an empty-state hint under the picker that points users to Providers when no active connections exist, matching the web modal's hint.
Tests
4 new tests in
InferenceProfileEditorTests, all targetingavailableProviderIds:testAvailableProviderIdsHidesProvidersWithoutActiveConnection— active anthropic shows, disabled openai hidden.testAvailableProviderIdsKeepsCurrentBoundProvider— stale-binding recovery: bound openai stays visible even when its connection is disabled.testAvailableProviderIdsFallsBackToFullCatalogWhenConnectionsEmpty— no connections passed → full catalog (pre-load fallback).testAvailableProviderIdsIsEmptyWhenOnlyDisabledConnectionsAndNoBoundProvider— all-disabled + nothing bound → empty.Web parity
Mirrors
visibleProviders+providerOptionsSourceinweb/src/app/(app)/assistant/settings/ai/profile-editor-modal.tsx(PR #6509). Same three behaviors: filter to active, fall back when empty, keep current binding.Out of scope
The other iter3 QA items were either web-only (model-profile row inset, Default Profile placeholder/empty state) or already correct on macOS (the default picker has a placeholder and never offered "None"). The "Save as New" parity for managed provider connections is a separate PR.