From 06b1513823ac00fe280731267b7a9216ec3eb4d4 Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Fri, 10 Jul 2026 17:08:51 -0700 Subject: [PATCH] Improve endpoint model setup UX --- Cotabby/App/Core/CotabbyAppEnvironment.swift | 9 +- .../OpenAICompatibleEndpointModels.swift | 25 +++ .../Panes/EngineAndModelPaneView.swift | 187 ++++++++---------- Cotabby/UI/Settings/SettingsIndex.swift | 11 +- .../OpenAICompatibleAPIClientTests.swift | 54 +++++ 5 files changed, 174 insertions(+), 112 deletions(-) diff --git a/Cotabby/App/Core/CotabbyAppEnvironment.swift b/Cotabby/App/Core/CotabbyAppEnvironment.swift index 99c2642d..04725258 100644 --- a/Cotabby/App/Core/CotabbyAppEnvironment.swift +++ b/Cotabby/App/Core/CotabbyAppEnvironment.swift @@ -436,9 +436,10 @@ final class CotabbyAppEnvironment { } } - /// Connect once when the endpoint engine becomes active. Configuration edits only invalidate - /// discovery so status never describes a different URL, mode, credential, or active model; - /// Settings still owns explicit Connect/Return refreshes and avoids network traffic per keystroke. + /// Connect once when the endpoint engine becomes active. Only server identity and credentials + /// invalidate discovery: the selected model and generation route do not affect `GET /models`, + /// so changing either must keep the catalog and connected status alive. Settings still owns + /// explicit Connect/Return refreshes and avoids network traffic per keystroke. private func observeOpenAICompatibleSelection() { suggestionSettings.$selectedEngine .removeDuplicates() @@ -462,8 +463,6 @@ final class CotabbyAppEnvironment { let configurationChanges: [AnyPublisher] = [ suggestionSettings.$openAICompatibleBaseURL.dropFirst().map { _ in () }.eraseToAnyPublisher(), - suggestionSettings.$openAICompatibleModelName.dropFirst().map { _ in () }.eraseToAnyPublisher(), - suggestionSettings.$openAICompatibleAPIMode.dropFirst().map { _ in () }.eraseToAnyPublisher(), suggestionSettings.$endpointCredentialRevision.dropFirst().map { _ in () }.eraseToAnyPublisher() ] diff --git a/Cotabby/Models/OpenAICompatibleEndpointModels.swift b/Cotabby/Models/OpenAICompatibleEndpointModels.swift index 4ddecb69..af1700ed 100644 --- a/Cotabby/Models/OpenAICompatibleEndpointModels.swift +++ b/Cotabby/Models/OpenAICompatibleEndpointModels.swift @@ -38,6 +38,31 @@ nonisolated struct OpenAICompatibleModelOption: Decodable, Equatable, Identifiab } } +/// Chooses the durable model identifier after endpoint discovery finishes. +/// +/// The connection model owns the fetched catalog, while `SuggestionSettingsModel` owns the user's +/// saved selection. Keeping the reconciliation rule in this pure type avoids teaching the SwiftUI +/// pane how to compare those two sources of truth and makes first-connection behavior testable. +/// The resolver lives for only the duration of the static call; no object owns it. +nonisolated enum OpenAICompatibleModelSelectionResolver { + /// Preserve a still-available choice; otherwise use the endpoint's first model. The API client + /// sorts its catalog before publishing it, so this fallback is stable rather than random. + /// An empty catalog returns nil so manual identifiers survive endpoints that cannot list models. + static func preferredSelection( + currentSelection: String, + discoveredModels: [OpenAICompatibleModelOption] + ) -> String? { + guard let firstModel = discoveredModels.first else { return nil } + + let current = currentSelection.trimmingCharacters(in: .whitespacesAndNewlines) + if let matchingModel = discoveredModels.first(where: { $0.id == current }) { + return matchingModel.id + } + + return firstModel.id + } +} + /// Privacy classification for the configured server host. nonisolated enum OpenAICompatibleHostScope: Equatable, Sendable { case loopback diff --git a/Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift b/Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift index afa44009..f3448468 100644 --- a/Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift +++ b/Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift @@ -357,64 +357,52 @@ struct EngineAndModelPaneView: View { Section("Connection") { VStack(alignment: .leading, spacing: 10) { SettingsRowLabel( - title: "Base URL", - description: "The OpenAI-compatible /v1 base URL. Ollama uses " + - "http://127.0.0.1:11434/v1 by default.", + title: "Server URL", + description: "The OpenAI-compatible server address. Cotabby adds /v1 when the " + + "address has no path; Ollama uses http://127.0.0.1:11434 by default.", systemImage: "network" ) - TextField( - OpenAICompatibleEndpointConfiguration.defaultBaseURLString, - text: endpointBaseURLBinding - ) - .textFieldStyle(.roundedBorder) - .onSubmit(refreshEndpointModels) + HStack(spacing: 8) { + TextField( + "", + text: endpointBaseURLBinding, + prompt: Text(OpenAICompatibleEndpointConfiguration.defaultBaseURLString) + ) + .labelsHidden() + .textFieldStyle(.roundedBorder) + .onSubmit(refreshEndpointModels) + + Button( + endpointConnectButtonTitle, + action: refreshEndpointModels + ) + .buttonStyle(.borderedProminent) + .disabled(openAICompatibleConnectionModel.state == .connecting) + } HStack(spacing: 8) { - Text("Press Return or connect to check this endpoint.") - .font(.caption) - .foregroundStyle(.secondary) + endpointConnectionStatus Spacer(minLength: 8) - Button("Reset to Default", systemImage: "arrow.counterclockwise") { + Button("Use Ollama Default", systemImage: "arrow.counterclockwise") { resetEndpointBaseURL() } .disabled( suggestionSettings.openAICompatibleBaseURL == OpenAICompatibleEndpointConfiguration.defaultBaseURLString ) - - Button( - endpointConnectButtonTitle, - action: refreshEndpointModels - ) - .buttonStyle(.borderedProminent) - .disabled(openAICompatibleConnectionModel.state == .connecting) } } .settingsItem(.endpointBaseURL) - Picker(selection: endpointAPIModeBinding) { - ForEach(OpenAICompatibleAPIMode.allCases) { mode in - Text(mode.displayLabel).tag(mode) - } - } label: { - SettingsRowLabel( - title: "Request Mode", - description: "Use Chat Completions for instruction-tuned models, or Completions " + - "for base models that continue a raw prompt.", - systemImage: "arrow.left.arrow.right" - ) - } - .pickerStyle(.menu) - .settingsItem(.endpointAPIMode) - LabeledContent { HStack(spacing: 8) { - SecureField("Optional", text: $endpointAPIKeyDraft) + SecureField("", text: $endpointAPIKeyDraft, prompt: Text("Paste API key")) + .labelsHidden() .textFieldStyle(.roundedBorder) - .frame(width: 240) + .frame(minWidth: 200, idealWidth: 260, maxWidth: 300) if !endpointAPIKeyDraft.isEmpty { Button("Clear") { @@ -437,9 +425,6 @@ struct EngineAndModelPaneView: View { } .settingsItem(.endpointAPIKey) - endpointConnectionSummary - .settingsItem(.endpointStatus) - if let warning = endpointPrivacyWarning { SettingsCalloutView(callout: SettingsPaneCallout(tone: .warning, message: warning)) } @@ -451,41 +436,52 @@ struct EngineAndModelPaneView: View { } Section("Model") { - if !openAICompatibleConnectionModel.models.isEmpty { - Picker(selection: endpointModelBinding) { - ForEach(openAICompatibleConnectionModel.models) { model in - Text(model.id).tag(model.id) - } - if !suggestionSettings.openAICompatibleModelName.isEmpty, - !openAICompatibleConnectionModel.models.contains(where: { - $0.id == suggestionSettings.openAICompatibleModelName - }) { - Text(suggestionSettings.openAICompatibleModelName) - .tag(suggestionSettings.openAICompatibleModelName) + LabeledContent { + HStack(spacing: 6) { + TextField("", text: endpointModelBinding, prompt: Text("Enter model identifier")) + .labelsHidden() + .textFieldStyle(.roundedBorder) + .frame(minWidth: 220, idealWidth: 280, maxWidth: 320) + + Menu { + ForEach(openAICompatibleConnectionModel.models) { model in + Button(model.id) { + suggestionSettings.setOpenAICompatibleModelName(model.id) + } + } + } label: { + Image(systemName: "chevron.up.chevron.down") + .frame(width: 18, height: 18) } - } label: { - SettingsRowLabel( - title: "Available Models", - description: "Models returned by the server's /v1/models endpoint.", - systemImage: "shippingbox" - ) + .menuStyle(.borderlessButton) + .menuIndicator(.hidden) + .fixedSize() + .disabled(openAICompatibleConnectionModel.models.isEmpty) + .help(endpointModelMenuHelp) + .accessibilityLabel("Choose a discovered model") } - .pickerStyle(.menu) + } label: { + SettingsRowLabel( + title: "Model", + description: "Choose a model returned by the server or enter its identifier manually.", + systemImage: "shippingbox" + ) } + .settingsItem(.endpointModel) - LabeledContent { - TextField("Model identifier", text: endpointModelBinding) - .textFieldStyle(.roundedBorder) - .frame(width: 300) + Picker(selection: endpointAPIModeBinding) { + Text("Chat Completions").tag(OpenAICompatibleAPIMode.chatCompletions) + Text("Text Completions").tag(OpenAICompatibleAPIMode.completions) } label: { SettingsRowLabel( - title: "Model Name", - description: "The exact model identifier sent with every request. You can enter " + - "one manually when model discovery is unavailable.", - systemImage: "text.cursor" + title: "API Format", + description: "Chat uses /v1/chat/completions (recommended). Text uses " + + "/v1/completions for base models that continue a raw prompt.", + systemImage: "arrow.left.arrow.right" ) } - .settingsItem(.endpointModel) + .pickerStyle(.segmented) + .settingsItem(.endpointAPIMode) } } @@ -602,11 +598,10 @@ struct EngineAndModelPaneView: View { (try? suggestionSettings.openAICompatibleConfiguration)?.privacyWarning } - /// The status card keeps the configured server identity visible without making the editable - /// text field carry two jobs. A user can now distinguish "this is what I typed" from "this is - /// the endpoint Cotabby most recently tried to reach" at a glance. - private var endpointConnectionSummary: some View { - HStack(spacing: 12) { + /// Compact feedback stays beside the connection action instead of repeating the URL in a second + /// card. This view has no independent lifetime; the endpoint connection model drives every state. + private var endpointConnectionStatus: some View { + HStack(spacing: 6) { Group { if openAICompatibleConnectionModel.state == .connecting { ProgressView() @@ -616,37 +611,17 @@ struct EngineAndModelPaneView: View { .foregroundStyle(endpointConnectionColor) } } - .frame(width: 18) - - VStack(alignment: .leading, spacing: 3) { - Text(openAICompatibleConnectionModel.state.summary) - .font(.callout.weight(.medium)) - .foregroundStyle(endpointConnectionColor) + .frame(width: 14) - Text(endpointDisplayURL) - .font(.caption.monospaced()) - .foregroundStyle(.secondary) - .lineLimit(1) - .truncationMode(.middle) - .textSelection(.enabled) - .help(endpointDisplayURL) - } - - Spacer(minLength: 0) - } - .padding(.horizontal, 12) - .padding(.vertical, 10) - .background { - RoundedRectangle(cornerRadius: 8, style: .continuous) - .fill(Color.secondary.opacity(0.08)) - } - .overlay { - RoundedRectangle(cornerRadius: 8, style: .continuous) - .stroke(Color.secondary.opacity(0.16), lineWidth: 1) + Text(openAICompatibleConnectionModel.state.summary) + .font(.caption.weight(.medium)) + .foregroundStyle(endpointConnectionColor) + .lineLimit(2) } .accessibilityElement(children: .combine) .accessibilityLabel("Server status") .accessibilityValue("\(openAICompatibleConnectionModel.state.summary), \(endpointDisplayURL)") + .settingsItem(.endpointStatus) } private var endpointConnectButtonTitle: String { @@ -682,6 +657,13 @@ struct EngineAndModelPaneView: View { return enteredURL.isEmpty ? "No endpoint configured" : enteredURL } + private var endpointModelMenuHelp: String { + if openAICompatibleConnectionModel.models.isEmpty { + return "Connect to load models, or enter an identifier manually." + } + return "Choose from \(openAICompatibleConnectionModel.models.count) discovered models." + } + private var pendingDeletionAlertBinding: Binding { Binding( get: { pendingDeletionModel != nil }, @@ -733,10 +715,11 @@ struct EngineAndModelPaneView: View { configuration: configuration, apiKey: endpointAPIKeyDraft ) - if suggestionSettings.openAICompatibleModelName.isEmpty, - openAICompatibleConnectionModel.models.count == 1, - let onlyModel = openAICompatibleConnectionModel.models.first { - suggestionSettings.setOpenAICompatibleModelName(onlyModel.id) + if let preferredModel = OpenAICompatibleModelSelectionResolver.preferredSelection( + currentSelection: suggestionSettings.openAICompatibleModelName, + discoveredModels: openAICompatibleConnectionModel.models + ), preferredModel != suggestionSettings.openAICompatibleModelName { + suggestionSettings.setOpenAICompatibleModelName(preferredModel) } } catch { endpointCredentialError = error.localizedDescription diff --git a/Cotabby/UI/Settings/SettingsIndex.swift b/Cotabby/UI/Settings/SettingsIndex.swift index 35e1e39c..81413ca7 100644 --- a/Cotabby/UI/Settings/SettingsIndex.swift +++ b/Cotabby/UI/Settings/SettingsIndex.swift @@ -144,8 +144,8 @@ enum SettingsItem: String, CaseIterable, Identifiable { case .huggingFaceBrowser: return "Hugging Face Model Browser" case .modelsFolder: return "Models Folder" case .lmStudio: return "LM Studio Models" - case .endpointBaseURL: return "Endpoint Base URL" - case .endpointAPIMode: return "Endpoint Request Mode" + case .endpointBaseURL: return "Endpoint Server URL" + case .endpointAPIMode: return "Endpoint API Format" case .endpointAPIKey: return "Endpoint API Key" case .endpointStatus: return "Endpoint Status" case .endpointModel: return "Endpoint Model" @@ -331,8 +331,8 @@ enum SettingsItem: String, CaseIterable, Identifiable { case .huggingFaceBrowser: return "Search Hugging Face for GGUF models." case .modelsFolder: return "Where downloaded model files live on this Mac." case .lmStudio: return "Also load models from your LM Studio library." - case .endpointBaseURL: return "Where the OpenAI-compatible server accepts /v1 requests." - case .endpointAPIMode: return "Choose Completions or Chat Completions for this model." + case .endpointBaseURL: return "The OpenAI-compatible server address; /v1 is added when omitted." + case .endpointAPIMode: return "Choose Chat Completions or Text Completions for this model." case .endpointAPIKey: return "Optional bearer token stored securely in Keychain." case .endpointStatus: return "Whether Cotabby can reach the server and list its models." case .endpointModel: return "The model identifier sent to the configured endpoint." @@ -510,7 +510,8 @@ enum SettingsItem: String, CaseIterable, Identifiable { case .endpointBaseURL: return ["endpoint", "url", "server", "host", "ollama", "localhost", "openai", "v1"] case .endpointAPIMode: - return ["completion", "chat completion", "api", "mode", "request", "openai"] + return ["completion", "chat completion", "text completion", "api", "format", + "mode", "request", "openai"] case .endpointAPIKey: return ["api key", "token", "bearer", "credential", "keychain", "authentication"] case .endpointStatus: diff --git a/CotabbyTests/OpenAICompatibleAPIClientTests.swift b/CotabbyTests/OpenAICompatibleAPIClientTests.swift index c7fa88c8..17d36773 100644 --- a/CotabbyTests/OpenAICompatibleAPIClientTests.swift +++ b/CotabbyTests/OpenAICompatibleAPIClientTests.swift @@ -6,6 +6,60 @@ import XCTest /// parsing are pure; URLProtocol stubs exercise the real URLSession request paths and headers. @MainActor final class OpenAICompatibleAPIClientTests: XCTestCase { + func test_modelSelectionResolver_choosesFirstDiscoveredModelForEmptySelection() { + let models = [ + OpenAICompatibleModelOption(id: "alpha", ownedBy: nil), + OpenAICompatibleModelOption(id: "beta", ownedBy: nil) + ] + + XCTAssertEqual( + OpenAICompatibleModelSelectionResolver.preferredSelection( + currentSelection: "", + discoveredModels: models + ), + "alpha" + ) + } + + func test_modelSelectionResolver_preservesAvailableSelection() { + let models = [ + OpenAICompatibleModelOption(id: "alpha", ownedBy: nil), + OpenAICompatibleModelOption(id: "beta", ownedBy: nil) + ] + + XCTAssertEqual( + OpenAICompatibleModelSelectionResolver.preferredSelection( + currentSelection: "beta", + discoveredModels: models + ), + "beta" + ) + } + + func test_modelSelectionResolver_replacesStaleSelectionWithFirstDiscoveredModel() { + let models = [ + OpenAICompatibleModelOption(id: "alpha", ownedBy: nil), + OpenAICompatibleModelOption(id: "beta", ownedBy: nil) + ] + + XCTAssertEqual( + OpenAICompatibleModelSelectionResolver.preferredSelection( + currentSelection: "removed-model", + discoveredModels: models + ), + "alpha" + ) + } + + func test_modelSelectionResolver_preservesManualSelectionForEmptyCatalog() { + XCTAssertNil( + OpenAICompatibleModelSelectionResolver.preferredSelection( + currentSelection: "manual-model", + discoveredModels: [] + ) + ) + } + override func tearDown() { EndpointStubURLProtocol.handler = nil EndpointStubURLProtocol.holdRequests = false