Skip to content

fix(macos): hide redundant default Provider picker on Language Model card #30413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dvargasfuertes merged 2 commits into from
May 12, 2026
Merged

fix(macos): hide redundant default Provider picker on Language Model card #30413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
422d3d3
fix(macos): hide redundant default Provider picker on Language Model …
May 12, 2026
e64dcf4
ci: retrigger workflow dispatch
May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 6 additions & 119 deletions clients/macos/vellum-assistant/Features/Settings/InferenceServiceCard.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,62 +1,34 @@
import SwiftUI
import VellumAssistantShared

/// Card for the inference service provider and profile configuration.
/// Card for the inference service: active profile selector and management
/// buttons for provider connections, profiles, and per-call-site overrides.
///
/// Shows the global provider picker, active profile selector, and
/// management buttons for provider connections, profiles, and per-call-site
/// overrides. Saving persists the provider selection only — model selection
/// lives inside inference profiles.
/// Profiles already carry the provider they dispatch against, so the card
/// does not surface a separate global "default Provider" picker — picking a
/// profile picks its provider transitively. Provider connections themselves
/// are managed through the "Providers" button.
Comment on lines +7 to +10
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh effective provider after profile changes

When the user selects a profile whose provider differs from llm.default.provider, this card now has no remaining control or side effect that updates the settings store's effective provider/model. setActiveProfile only patches llm.activeProfile, while WebSearchServiceCard.availableProviders and its auto-fallback still read store.selectedInferenceProvider/selectedModel, and CallSiteOverridesSheet still receives store.selectedInferenceProvider as the default provider; after choosing, for example, a Fireworks profile while the default provider is Anthropic, those settings screens continue to offer/keep provider-native web search and show Anthropic defaults even though the main agent resolves through Fireworks. Please refresh/apply ModelInfo or derive these consumers from the selected active profile when the profile picker persists a new value.

Useful? React with 👍 / 👎.

@MainActor
struct InferenceServiceCard: View {
@ObservedObject var store: SettingsStore
var showToast: (String, ToastInfo.Style) -> Void

/// Local draft of the provider selection — only persisted on Save.
@State private var draftProvider: String = "anthropic"
/// Snapshot of the provider at card appear — used to detect provider changes.
@State private var initialProvider: String = ""
/// Whether the read-only per-call-site overrides sheet is presented.
@State private var showOverridesSheet = false
/// Whether the inference profiles management sheet is presented.
@State private var showProfilesSheet = false
/// Whether the provider connections management sheet is presented.
@State private var showProvidersSheet = false
/// Whether to show the per-call-site override confirmation dialog. Fires
/// when the user is about to switch the global provider AND has at least
/// one override pinned to the OLD provider — we ask whether to keep those
/// pins or reset them to follow the new default.
@State private var showOverrideConfirmation = false
/// Snapshot of the overrides pinned to the OLD provider at the moment the
/// confirmation dialog is shown. Used both to render the dialog message
/// (count + provider name) and to drive the "Reset" action.
@State private var pendingOverrideClears: [CallSiteOverride] = []
/// The provider name displayed in the confirmation dialog message.
@State private var pendingOverrideOldProviderName: String = ""
/// The most recent in-flight `setActiveProfile` task, retained so a
/// subsequent dropdown pick can cancel it and avoid an out-of-order
/// PATCH landing the older selection last.
@State private var activeProfileTask: Task<Void, Never>?

// MARK: - Computed State

private var hasChanges: Bool {
draftProvider != initialProvider
}

var body: some View {
SettingsCard(title: "Language Model", subtitle: "Configure the LLMs that power your assistant") {
VStack(alignment: .leading, spacing: VSpacing.sm) {
providerPicker
activeProfilePicker
secondaryActionsRow
if hasChanges {
ServiceCardActions(
hasChanges: true,
isSaving: false,
onSave: { save() }
)
}
}
}
.sheet(isPresented: $showOverridesSheet) {
Expand All @@ -68,48 +40,6 @@ struct InferenceServiceCard: View {
.sheet(isPresented: $showProvidersSheet) {
ProvidersSheet(store: store, isPresented: $showProvidersSheet)
}
.onAppear {
draftProvider = store.selectedInferenceProvider
initialProvider = store.selectedInferenceProvider
}
.onChange(of: store.selectedInferenceProvider) { _, newValue in
draftProvider = newValue
initialProvider = newValue
}
.confirmationDialog(
"Keep per-task overrides?",
isPresented: $showOverrideConfirmation,
titleVisibility: .visible
) {
Button("Keep overrides") {
performSaveCore(clearingOverrides: [])
}
Button("Reset to follow default") {
performSaveCore(clearingOverrides: pendingOverrideClears)
}
Button("Cancel", role: .cancel) {
pendingOverrideClears = []
pendingOverrideOldProviderName = ""
}
} message: {
let msg = "\(pendingOverrideClears.count) task(s) are pinned to \(pendingOverrideOldProviderName). Keep them as-is, or update them to follow the new default?"
Text(msg)
}
}

// MARK: - Provider Picker

private var providerPicker: some View {
VStack(alignment: .leading, spacing: VSpacing.sm) {
Text("Provider")
.font(VFont.labelDefault)
.foregroundStyle(VColor.contentSecondary)
VDropdown(
placeholder: "Select a provider\u{2026}",
selection: $draftProvider,
options: store.providerCatalog.map { (label: $0.displayName, value: $0.id) }
)
}
}

// MARK: - Secondary Actions Row
Expand Down Expand Up @@ -157,47 +87,4 @@ struct InferenceServiceCard: View {
)
}
}

// MARK: - Save

private func save() {
performSave()
}

private func performSave() {
let persistProvider = draftProvider
let providerIdChanged = persistProvider != initialProvider
if providerIdChanged {
let overridesPinnedToOldProvider = store.callSiteOverrides.filter {
$0.provider == initialProvider
}
if !overridesPinnedToOldProvider.isEmpty {
pendingOverrideClears = overridesPinnedToOldProvider
pendingOverrideOldProviderName = store.dynamicProviderDisplayName(initialProvider)
showOverrideConfirmation = true
return
}
}
performSaveCore(clearingOverrides: [])
}

private func performSaveCore(clearingOverrides overridesToClear: [CallSiteOverride]) {
store.apiKeySaveError = nil

for override in overridesToClear {
_ = store.clearCallSiteOverride(override.id)
}
pendingOverrideClears = []
pendingOverrideOldProviderName = ""

let persistProvider = draftProvider
let providerChanged = persistProvider != initialProvider
if providerChanged {
initialProvider = draftProvider
let capturedProvider = persistProvider
Task {
_ = await store.setLLMDefaultProvider(capturedProvider).value
}
}
}
}
45 changes: 8 additions & 37 deletions clients/macos/vellum-assistantTests/Features/Settings/InferenceServiceCardTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import XCTest

/// Structural tests for `InferenceServiceCard`. Exercises the bindings the
/// card surfaces — Active Profile selection routing through
/// `store.setActiveProfile`, the Manage Profiles sheet toggle, and the Save
/// path no longer writing `llm.default.model`. Mirrors the `InferenceProfilesSheetTests`
/// pattern: build the SwiftUI tree without rendering, drive store-backed
/// invariants directly, and assert the patches captured by
/// `MockSettingsClient`.
/// `store.setActiveProfile` and the management sheet toggles. Mirrors the
/// `InferenceProfilesSheetTests` pattern: build the SwiftUI tree without
/// rendering, drive store-backed invariants directly, and assert the
/// patches captured by `MockSettingsClient`.
@MainActor
final class InferenceServiceCardTests: XCTestCase {

Expand Down Expand Up @@ -78,7 +77,8 @@ final class InferenceServiceCardTests: XCTestCase {
}

/// True when any captured `llm.default` patch has touched `model`. Used
/// to assert the card's Save path no longer writes the model leaf.
/// to assert that flows driven from this card never mutate the model
/// leaf — model selection lives inside inference profiles.
private func anyPatchWroteLLMDefaultModel() -> Bool {
for payload in mockSettingsClient.patchConfigCalls {
guard let llm = payload["llm"] as? [String: Any],
Expand Down Expand Up @@ -124,9 +124,8 @@ final class InferenceServiceCardTests: XCTestCase {
XCTAssertEqual(lastActive, "quality-optimized")

// The patch must touch `activeProfile` — and nothing else under
// `llm.default`. This is the central invariant of PR 14: the
// active profile setter is its own path, distinct from
// `llm.default.{provider,model}`.
// `llm.default`. The active profile setter is its own path,
// distinct from `llm.default.{provider,model}`.
XCTAssertFalse(
anyPatchWroteLLMDefaultModel(),
"Active Profile selection must not write llm.default.model"
Expand Down Expand Up @@ -173,34 +172,6 @@ final class InferenceServiceCardTests: XCTestCase {
XCTAssertNotNil(sheet.body, "ProvidersSheet must be constructible with the card's store")
}

// MARK: - Save flow no longer writes llm.default.model

/// Persisting a provider change writes `llm.default.provider` only.
/// This is the second central invariant of PR 14: Save no longer
/// touches `llm.default.model`.
func testProviderOnlySetterPatchesProviderWithoutModel() async {
let task = store.setLLMDefaultProvider("openai")
_ = await task.value

// Find the captured provider patch.
let providerPatches = mockSettingsClient.patchConfigCalls.compactMap { payload -> [String: Any]? in
guard let llm = payload["llm"] as? [String: Any],
let llmDefault = llm["default"] as? [String: Any] else { return nil }
return llmDefault
}
XCTAssertEqual(providerPatches.count, 1, "Provider-only setter must emit exactly one patch")
XCTAssertEqual(providerPatches.first?["provider"] as? String, "openai")
XCTAssertNil(
providerPatches.first?["model"],
"Provider-only setter must not include the model leaf"
)

XCTAssertFalse(
anyPatchWroteLLMDefaultModel(),
"PR 14 invariant: the inference card's Save path never writes llm.default.model"
)
}

// MARK: - Profiles list flows through to dropdown options

/// The dropdown options come from `store.profiles.map { $0.name }` —
Expand Down
Loading