feat(inference): add status + label to provider connections and profiles#30254
Conversation
credence-the-bot
Bot
force-pushed
the
credence/provider-status-label
branch
from
d1f5a91 to
fb70ef6
Compare
credence-the-bot
Bot
force-pushed
the
credence/provider-status-label
branch
2 times, most recently
from
2a08e99 to
50114b7
Compare
![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: fb70ef6f42
ℹ️ 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".
| /// One of: `anthropic`, `openai`, `gemini`, `ollama`, `fireworks`, `openrouter`. | ||
| public let provider: String | ||
| public let auth: ProviderConnectionAuth | ||
| public let status: ConnectionStatus |
There was a problem hiding this comment.
Treating status as a required enum breaks compatibility with assistants that predate this API field: JSONDecoder will throw keyNotFound when /inference/provider-connections responses omit status, and then ProviderConnectionClient returns nil for list/get/create/update paths. In mixed-version setups (which the app explicitly supports via version-mismatch handling), this makes the Providers UI fail to load even though older servers are otherwise valid; status should decode with a default of .active when absent.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 merging() doesn't merge status, causing local cache divergence after setProfile calls
InferenceProfile.merging(_:) does not include status in its merge logic. The function's doc comment says it "mirrors the daemon's deep-merge semantics for llm.profiles.<name> patches", but the daemon's JSON-level deep-merge DOES merge status. When SettingsStore.setProfile() (the merge-based save path at clients/macos/vellum-assistant/Features/Settings/SettingsStore.swift:3338) succeeds, it updates the local cache via profiles[index] = profiles[index].merging(fragment) (line 3370). If the fragment carries a non-nil status (e.g. "disabled"), the daemon stores the new status correctly (via toJSON() which does emit "disabled"), but the local profiles array retains the old status until the next daemon config push. This means a user could toggle a profile to disabled through a code path that uses setProfile, see the profile still appear as active in the UI, and only have it correct after a config refresh.
(Refers to lines 197-210)
Prompt for agents
In InferenceProfile.swift, the merging(_:) function (line 197-210) does not merge the new status field. The daemon's JSON-level deep-merge handles all keys including status, but the local merging function skips it. This causes the local profiles cache to diverge from daemon state when setProfile (merge path) is used.
The fix is to add status merging to the merging() function, similar to how other Optional<String> fields could be merged. However, status has a special semantic: nil means active, so merging needs to handle both setting status to disabled and clearing it back to nil. A simple if let v = fragment.status { merged.status = v } would handle setting to disabled, but wouldn't handle clearing from disabled back to nil.
Consider either:
1. Adding if fragment.status != nil { merged.status = fragment.status } to merge status when explicitly set
2. Or, if the merge path should also support clearing status, a different mechanism (like the TemperatureValue three-state pattern) would be needed
Note: the same gap exists for label and description (pre-existing), but status is the most critical because it controls profile visibility in pickers.
Was this helpful? React with 👍 or 👎 to provide feedback.
credence-the-bot
Bot
force-pushed
the
credence/provider-status-label
branch
from
50114b7 to
5b5f8a4
Compare
- Migration 244 adds status (NOT NULL DEFAULT 'active') + label (nullable) columns to provider_connections, idempotent via PRAGMA table_info gate. - Zod schemas (ConnectionStatusSchema, ProfileStatusSchema), CRUD module, routes propagate the new fields. POST/PATCH accept and persist. - ProfileEntry gains optional status field (absent treated as active). - macOS connection editor: Display Name auto-derives Key (mirrors InferenceProfileEditor pattern with isKeyDirty lock). - macOS connection + profile editors: status toggle. - Profile + connection pickers filter status == disabled; settings sheets still show everything so users can re-enable.
- ProviderConnection now defaults missing status to .active when decoding (Codex P1). Mixed-version setups where the daemon predates this field no longer fail the keyNotFound check and strand the Providers UI. - InferenceProfile.merging() now merges status (Devin) so the local cache reflects setProfile-driven status flips without waiting for the next config refresh.
credence-the-bot
Bot
force-pushed
the
credence/provider-status-label
branch
from
8b47f26 to
7d6aeba
Compare
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Adds first-class `status` (active/disabled) and `label` (display name) to provider connections and `status` to inference profiles. Disabled entries are filtered from picker contexts but still visible in management settings so users can re-enable them. macOS connection editor gains a Display Name field that auto-derives the Key, mirroring the established profile editor pattern.
Behavior
Backend
macOS UI
Changes
21 files, +720/-51. Touches:
Test plan
Heads up
Another in-flight PR (provider-cred-edit) also modifies `ProvidersSheet.swift`. Whichever lands second will need a rebase. Same for the web counterpart.
🤖 Implementation by Claude Code, audit by Credence.