From 067aeda87897ed00011754b37bf3363b3b3b32c3 Mon Sep 17 00:00:00 2001 From: Alan Wizemann Date: Wed, 29 Apr 2026 12:30:29 +0200 Subject: [PATCH] =?UTF-8?q?fix(catalog):=20async=20catalog=20reads=20?= =?UTF-8?q?=E2=80=94=20unfreezes=20Model=20+=20Credential=20sheets=20(#59)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two views called ModelCatalogService.loadProviders() synchronously from .onAppear on the MainActor: - ModelPickerSheet (Settings → Model) - AddCredentialSheet (Credential Pools → +) loadProviders() walks loadCatalog() → transport.readFile() of ~/.hermes/models_dev_cache.json — a multi-megabyte JSON with ~1500 models across ~110 providers. On a remote SSH context that's a synchronous SSH file read on the main thread; the user's reported 1–2 minute UI freeze on first open is exactly that. Even on local contexts the JSONDecoder pass on the main thread is a noticeable hiccup. Direct violation of CLAUDE.md's rule against sync I/O on @MainActor. Compound case: ModelPickerSheet.loadModelsForSelection() did the same sync read every time the user clicked a different provider in the picker — re-froze the UI per click. Fix: - Add async wrappers on the service: loadProvidersAsync() -> [HermesProviderInfo] loadModelsAsync(for:) -> [HermesModelInfo] Each await Task.detached { sync method }.value. Existing sync methods stay for tests and any non-View consumers. - ModelPickerSheet: replace .onAppear with .task; await both async calls. Same conversion for loadModelsForSelection() — renamed to loadModelsForSelectionAsync() and called from the provider-list selection binding via Task { ... }. Subscription state load also routed through Task.detached since it's another auth.json read that's tiny on local but SSH-backed on remote. - AddCredentialSheet (CredentialPoolsView): same .onAppear → .task conversion with isLoadingProviders @State driving an overlay ProgressView "Loading providers..." while the read is in flight. No behavior or data-shape change; pure I/O dispatch fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Services/ModelCatalogService.swift | 24 ++++++++++ .../Views/CredentialPoolsView.swift | 24 +++++++++- .../Views/Components/ModelPickerSheet.swift | 46 ++++++++++++++++--- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/scarf/Packages/ScarfCore/Sources/ScarfCore/Services/ModelCatalogService.swift b/scarf/Packages/ScarfCore/Sources/ScarfCore/Services/ModelCatalogService.swift index 6cf3834..bcd5bb8 100644 --- a/scarf/Packages/ScarfCore/Sources/ScarfCore/Services/ModelCatalogService.swift +++ b/scarf/Packages/ScarfCore/Sources/ScarfCore/Services/ModelCatalogService.swift @@ -169,6 +169,19 @@ public struct ModelCatalogService: Sendable { Self.overlayOnlyProviders[providerID] } + /// Async wrapper around `loadProviders()` for use from MainActor view + /// code. The sync method does a transport-backed file read that on a + /// remote SSH context can take 1–2 minutes (ControlMaster setup + + /// pulling the multi-megabyte models.dev JSON), and on local contexts + /// still parses ~1500 models — both unsuitable for the main thread. + /// Issue #59. Existing call sites (tests, any non-View consumers) + /// can keep using the sync method. + public nonisolated func loadProvidersAsync() async -> [HermesProviderInfo] { + await Task.detached { [self] in + self.loadProviders() + }.value + } + /// Models for one provider, sorted by release date (newest first), then name. public func loadModels(for providerID: String) -> [HermesModelInfo] { guard let catalog = loadCatalog(), let provider = catalog[providerID] else { return [] } @@ -198,6 +211,17 @@ public struct ModelCatalogService: Sendable { } } + /// Async wrapper around `loadModels(for:)`. Same rationale as + /// `loadProvidersAsync()` — the View call site that fires on every + /// provider-switch click in the picker sheet was reading the catalog + /// synchronously on the MainActor, freezing the UI on remote contexts. + /// Issue #59. + public nonisolated func loadModelsAsync(for providerID: String) async -> [HermesModelInfo] { + await Task.detached { [self] in + self.loadModels(for: providerID) + }.value + } + /// Find the provider that ships a given model ID. Useful for auto-syncing /// provider when the user picks a model from a flat list or types one in. public func provider(for modelID: String) -> HermesProviderInfo? { diff --git a/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift b/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift index 79811fc..5172a2f 100644 --- a/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift +++ b/scarf/scarf/Features/CredentialPools/Views/CredentialPoolsView.swift @@ -357,6 +357,11 @@ private struct AddCredentialSheet: View { @State private var apiKey: String = "" @State private var label: String = "" @State private var providers: [HermesProviderInfo] = [] + /// True while the initial models.dev catalog read is in flight. + /// Drives the loading-overlay placeholder. Pre-fix this work ran + /// synchronously inside `.onAppear` and froze the sheet for 1–2 + /// minutes on remote contexts (issue #59). + @State private var isLoadingProviders: Bool = true @State private var oauthStarted: Bool = false @State private var authCode: String = "" /// Drives presentation of the dedicated Nous sign-in sheet from inside @@ -385,8 +390,23 @@ private struct AddCredentialSheet: View { } .padding() .frame(minWidth: 600, minHeight: 460) - .onAppear { - providers = catalog.loadProviders() + .overlay { + if isLoadingProviders { + ProgressView("Loading providers…") + .progressViewStyle(.circular) + .padding() + .background(.regularMaterial) + .clipShape(RoundedRectangle(cornerRadius: 10)) + } + } + .task { + // Off-MainActor read of the multi-megabyte models.dev cache + // (via SSHTransport on remote contexts). Pre-fix this ran + // sync inside `.onAppear` and froze the Add Credential sheet + // for 1–2 minutes on remote contexts (issue #59). + isLoadingProviders = true + providers = await catalog.loadProvidersAsync() + isLoadingProviders = false } // Auto-close the sheet once a credential is actually saved. We key // off `succeeded` which the controller sets only when hermes exited diff --git a/scarf/scarf/Features/Settings/Views/Components/ModelPickerSheet.swift b/scarf/scarf/Features/Settings/Views/Components/ModelPickerSheet.swift index d9df990..2c55d81 100644 --- a/scarf/scarf/Features/Settings/Views/Components/ModelPickerSheet.swift +++ b/scarf/scarf/Features/Settings/Views/Components/ModelPickerSheet.swift @@ -22,6 +22,12 @@ struct ModelPickerSheet: View { @State private var models: [HermesModelInfo] = [] @State private var selectedModelID: String = "" @State private var searchText: String = "" + /// True while the initial catalog load (or a per-provider model + /// reload) is in flight. Drives the loading-overlay placeholder. + /// Pre-fix this work ran synchronously inside `.onAppear` — issue + /// #59. The catalog file is multi-MB on remote contexts; sync I/O + /// on the MainActor froze the picker for 1–2 minutes. + @State private var isLoadingCatalog: Bool = true // Custom model entry — used when the catalog doesn't have the exact model // the user needs (e.g., provider-prefixed IDs like "openrouter/some/model"). @@ -67,13 +73,33 @@ struct ModelPickerSheet: View { footer } .frame(minWidth: 720, minHeight: 520) - .onAppear { - providers = catalog.loadProviders() + .overlay { + if isLoadingCatalog { + ProgressView("Loading providers…") + .progressViewStyle(.circular) + .padding() + .background(.regularMaterial) + .clipShape(RoundedRectangle(cornerRadius: 10)) + } + } + .task { + // Off-MainActor read of the multi-megabyte models.dev cache + // (via SSHTransport on remote contexts). Pre-fix this ran + // sync inside `.onAppear` and froze the picker for 1–2 + // minutes on remote contexts (issue #59). + isLoadingCatalog = true + providers = await catalog.loadProvidersAsync() selectedProviderID = initialProvider.isEmpty ? (providers.first?.providerID ?? "") : initialProvider selectedModelID = initialModel overlayModelID = initialModel - subscription = subscriptionService.loadState() - loadModelsForSelection() + // subscriptionService.loadState() reads auth.json — tiny + // on local but still SSH-backed on remote, so route it + // through a detached task too. The result is a small + // value type; safe to assign back onto MainActor. + let svc = subscriptionService + subscription = await Task.detached { svc.loadState() }.value + await loadModelsForSelectionAsync() + isLoadingCatalog = false } .sheet(isPresented: $showNousSignIn) { NousSignInSheet { @@ -134,7 +160,7 @@ struct ModelPickerSheet: View { get: { selectedProviderID }, set: { newValue in selectedProviderID = newValue - loadModelsForSelection() + Task { await loadModelsForSelectionAsync() } } )) { ForEach(filteredProviders) { provider in @@ -424,12 +450,18 @@ struct ModelPickerSheet: View { return resolved.isEmpty ? "Provider will not be changed" : "Provider → \(resolved)" } - private func loadModelsForSelection() { + /// Async variant of the per-provider catalog read. Pre-fix this + /// was synchronous on the MainActor and froze the picker every + /// time the user clicked a different provider — same root cause + /// as the open-sheet freeze (issue #59). Routes through + /// `loadModelsAsync(for:)` which dispatches the SSHTransport + /// file read off the main thread. + private func loadModelsForSelectionAsync() async { guard !selectedProviderID.isEmpty else { models = [] return } - models = catalog.loadModels(for: selectedProviderID) + models = await catalog.loadModelsAsync(for: selectedProviderID) // If the current selection is not in the new list, don't try to keep // stale highlight state — clear unless the user originally had this model. if !models.contains(where: { $0.modelID == selectedModelID }) {