mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 18:44:45 +00:00
fix(catalog): async catalog reads — unfreezes Model + Credential sheets (#59)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -169,6 +169,19 @@ public struct ModelCatalogService: Sendable {
|
|||||||
Self.overlayOnlyProviders[providerID]
|
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.
|
/// Models for one provider, sorted by release date (newest first), then name.
|
||||||
public func loadModels(for providerID: String) -> [HermesModelInfo] {
|
public func loadModels(for providerID: String) -> [HermesModelInfo] {
|
||||||
guard let catalog = loadCatalog(), let provider = catalog[providerID] else { return [] }
|
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
|
/// 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.
|
/// provider when the user picks a model from a flat list or types one in.
|
||||||
public func provider(for modelID: String) -> HermesProviderInfo? {
|
public func provider(for modelID: String) -> HermesProviderInfo? {
|
||||||
|
|||||||
@@ -357,6 +357,11 @@ private struct AddCredentialSheet: View {
|
|||||||
@State private var apiKey: String = ""
|
@State private var apiKey: String = ""
|
||||||
@State private var label: String = ""
|
@State private var label: String = ""
|
||||||
@State private var providers: [HermesProviderInfo] = []
|
@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 oauthStarted: Bool = false
|
||||||
@State private var authCode: String = ""
|
@State private var authCode: String = ""
|
||||||
/// Drives presentation of the dedicated Nous sign-in sheet from inside
|
/// Drives presentation of the dedicated Nous sign-in sheet from inside
|
||||||
@@ -385,8 +390,23 @@ private struct AddCredentialSheet: View {
|
|||||||
}
|
}
|
||||||
.padding()
|
.padding()
|
||||||
.frame(minWidth: 600, minHeight: 460)
|
.frame(minWidth: 600, minHeight: 460)
|
||||||
.onAppear {
|
.overlay {
|
||||||
providers = catalog.loadProviders()
|
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
|
// Auto-close the sheet once a credential is actually saved. We key
|
||||||
// off `succeeded` which the controller sets only when hermes exited
|
// off `succeeded` which the controller sets only when hermes exited
|
||||||
|
|||||||
@@ -22,6 +22,12 @@ struct ModelPickerSheet: View {
|
|||||||
@State private var models: [HermesModelInfo] = []
|
@State private var models: [HermesModelInfo] = []
|
||||||
@State private var selectedModelID: String = ""
|
@State private var selectedModelID: String = ""
|
||||||
@State private var searchText: 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
|
// 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").
|
// the user needs (e.g., provider-prefixed IDs like "openrouter/some/model").
|
||||||
@@ -67,13 +73,33 @@ struct ModelPickerSheet: View {
|
|||||||
footer
|
footer
|
||||||
}
|
}
|
||||||
.frame(minWidth: 720, minHeight: 520)
|
.frame(minWidth: 720, minHeight: 520)
|
||||||
.onAppear {
|
.overlay {
|
||||||
providers = catalog.loadProviders()
|
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
|
selectedProviderID = initialProvider.isEmpty ? (providers.first?.providerID ?? "") : initialProvider
|
||||||
selectedModelID = initialModel
|
selectedModelID = initialModel
|
||||||
overlayModelID = initialModel
|
overlayModelID = initialModel
|
||||||
subscription = subscriptionService.loadState()
|
// subscriptionService.loadState() reads auth.json — tiny
|
||||||
loadModelsForSelection()
|
// 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) {
|
.sheet(isPresented: $showNousSignIn) {
|
||||||
NousSignInSheet {
|
NousSignInSheet {
|
||||||
@@ -134,7 +160,7 @@ struct ModelPickerSheet: View {
|
|||||||
get: { selectedProviderID },
|
get: { selectedProviderID },
|
||||||
set: { newValue in
|
set: { newValue in
|
||||||
selectedProviderID = newValue
|
selectedProviderID = newValue
|
||||||
loadModelsForSelection()
|
Task { await loadModelsForSelectionAsync() }
|
||||||
}
|
}
|
||||||
)) {
|
)) {
|
||||||
ForEach(filteredProviders) { provider in
|
ForEach(filteredProviders) { provider in
|
||||||
@@ -424,12 +450,18 @@ struct ModelPickerSheet: View {
|
|||||||
return resolved.isEmpty ? "Provider will not be changed" : "Provider → \(resolved)"
|
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 {
|
guard !selectedProviderID.isEmpty else {
|
||||||
models = []
|
models = []
|
||||||
return
|
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
|
// 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.
|
// stale highlight state — clear unless the user originally had this model.
|
||||||
if !models.contains(where: { $0.modelID == selectedModelID }) {
|
if !models.contains(where: { $0.modelID == selectedModelID }) {
|
||||||
|
|||||||
Reference in New Issue
Block a user