From f5992e7d207f7aefbbeb5f3bd4cf40e991809532 Mon Sep 17 00:00:00 2001 From: Simba Zhang Date: Wed, 26 Aug 2026 21:31:04 -0700 Subject: [PATCH 1/2] feat: let SwiftBuddy load a model from an arbitrary local folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #160. A user storing models on an external drive (downloaded via `hf download --local-dir `, entirely outside the app's HF cache) had no way to point SwiftBuddy at that folder directly — only the `SwiftLM` CLI's `--model ` already supported this. Distinct from issue #110's "hand-copied into a recognised cache-root layout" support (ModelStorage.localLoadDirectory, still used unchanged for that case): this is for a directory that can be anywhere on disk, addressed by its own absolute path rather than an HF-style "org/name" id resolved relative to cacheRoot. ## ModelStorage.swift - `isLocalDirectoryPath(_:)` — is this string actually a directory that exists on disk, mirroring the identical check Server.swift's CLI `--model` handling already does. - `validateLocalModelDirectory(_:)` — reuses the same weight/config validation `verifyModelIntegrity` applies to HF-cache layouts, so a bad folder selection is rejected with a clear message before `InferenceEngine.load` ever attempts to construct a model from it. - `readModelConfig(inDirectory:)` / `readMaxContextLength(inDirectory:)` — directory-addressed variants of the existing id-addressed functions (refactored to share the same underlying logic rather than duplicating it). ## InferenceEngine.swift `load(modelId:)` now recognises a local-directory modelId early and skips straight to loading it — the usual verify-or-download flow assumes an HF-style id and would otherwise try to download the path as if it were a repo id. `loadVerifiedModel` threads an `explicitLocalDirectory: URL?` through: building `ModelConfiguration(directory:)`, the SSD-streaming directory, the post-load integrity check, and the context-length read all branch on it instead of going through the id-based `ModelStorage` helpers (which only ever resolve paths under cacheRoot and would find nothing for an external path). Also: a load failure for a local directory no longer offers the "Delete & Re-download" recovery flow, since there's no repo to re-download from (confirmed `ModelStorage.delete` would silently no-op for such a path anyway — it only ever resolves candidates under cacheRoot — but showing that recovery option would still be misleading UX). ## ModelManagementView.swift (macOS only) "Add Local Model…" button next to "Search HuggingFace MLX Models" (in both the populated list and the empty state), opening an NSOpenPanel folder picker. Validates the selection with `ModelStorage.validateLocalModelDirectory` before calling `engine.load(modelId: url.path)`; a bad selection shows an alert instead of proceeding. ## Tests New tests/SwiftBuddyTests/LocalModelDirectoryTests.swift (13 tests) — deliberately does NOT use `cacheRootOverride`, unlike the #110 layout tests, since these functions must work independent of cacheRoot entirely. Full suite: 282/282 passing, zero regressions (including all 21 existing ModelStorageLayoutTests, confirming the readModelConfig/ readMaxContextLength refactor didn't change #110's behavior). Model persistence across app relaunch (`lastLoadedModelId`) works unchanged for a local path — it's stored as a plain string, same as an HF id, so no separate persistence code was needed. --- .../MLXInferenceCore/InferenceEngine.swift | 58 +++++++-- Sources/MLXInferenceCore/ModelStorage.swift | 56 ++++++++- .../Views/ModelManagementView.swift | 72 ++++++++++- .../LocalModelDirectoryTests.swift | 115 ++++++++++++++++++ 4 files changed, 285 insertions(+), 16 deletions(-) create mode 100644 tests/SwiftBuddyTests/LocalModelDirectoryTests.swift diff --git a/Sources/MLXInferenceCore/InferenceEngine.swift b/Sources/MLXInferenceCore/InferenceEngine.swift index b71cd91..e65dedb 100644 --- a/Sources/MLXInferenceCore/InferenceEngine.swift +++ b/Sources/MLXInferenceCore/InferenceEngine.swift @@ -308,6 +308,14 @@ public final class InferenceEngine: ObservableObject { } corruptedModelId = nil + // A path the user pointed the app at directly (e.g. via "Add Local + // Model…") — never downloaded, never in the HF cache, so the usual + // verify-or-download flow doesn't apply; go straight to loading it. + if ModelStorage.isLocalDirectoryPath(modelId) { + await loadVerifiedModel(modelId: modelId) + return + } + guard ModelStorage.verifyModelIntegrity(for: modelId) else { await downloadThenLoad(modelId: modelId) return @@ -347,6 +355,19 @@ public final class InferenceEngine: ObservableObject { setLoadingState(progress: 0.05, stage: "Preparing model configuration") currentModelId = modelId + // Two distinct kinds of "not in the normal cache" model share this function: + // - `isLocalDirectoryPath`: modelId IS the directory (e.g. picked via "Add + // Local Model…", possibly on an external drive entirely outside cacheRoot) + // - `localDirectory`: modelId is still an HF-style "org/name" id, but its + // files were copied by hand into a recognised cacheRoot-relative layout + // (issue #110) rather than downloaded — resolved through the existing + // ModelStorage id-based helpers below, which still work for this case. + // Only the first needs its own directory-based helpers throughout this + // function; ModelStorage.*(for: modelId) already finds the right directory + // for the second, the same way it does for a normally-downloaded model. + let explicitLocalDirectory: URL? = + ModelStorage.isLocalDirectoryPath(modelId) ? URL(filePath: modelId) : nil + do { let hub = HubApi(downloadBase: ModelStorage.cacheRoot) @@ -360,7 +381,9 @@ public final class InferenceEngine: ObservableObject { // pointing the loader at them would list the model and then re-download it — // several GB for a model already on disk. Load such models by directory. var config: ModelConfiguration - if let localDirectory = ModelStorage.localLoadDirectory(for: modelId) { + if let explicitLocalDirectory { + config = ModelConfiguration(directory: explicitLocalDirectory) + } else if let localDirectory = ModelStorage.localLoadDirectory(for: modelId) { config = ModelConfiguration(directory: localDirectory) } else { config = ModelConfiguration(id: modelId) @@ -377,7 +400,7 @@ public final class InferenceEngine: ObservableObject { let shouldStream = generationConfig.effectiveStreamExperts(defaultingTo: isMoE) if shouldStream { config.lazyLoad = true - let modelDir = ModelStorage.snapshotDirectory(for: modelId) + let modelDir = explicitLocalDirectory ?? ModelStorage.snapshotDirectory(for: modelId) ExpertStreamingConfig.shared.activate( modelDirectory: modelDir, useDirectIO: { @@ -436,15 +459,27 @@ public final class InferenceEngine: ObservableObject { downloadManager.lastLoadedModelId = modelId downloadManager.refresh() - // Verify integrity to catch incomplete downloads before marking as ready + // Verify integrity to catch incomplete downloads before marking as ready. + // A local directory was already validated once before load() was ever + // called (see ModelManagementView's "Add Local Model…" flow) — no + // "delete and re-download" recovery makes sense for a folder outside our + // cache, so this re-check exists to catch the same class of problem + // (missing/truncated weights) with a message that doesn't imply that. setLoadingState(progress: 0.94, stage: "Verifying model files") - guard ModelStorage.verifyModelIntegrity(for: modelId) else { - throw NSError(domain: "InferenceEngine", code: 1, userInfo: [NSLocalizedDescriptionKey: "Model safetensors files are incomplete. Please delete and re-download."]) + let integrityOK = explicitLocalDirectory.map(ModelStorage.validateLocalModelDirectory) + ?? ModelStorage.verifyModelIntegrity(for: modelId) + guard integrityOK else { + let message = explicitLocalDirectory != nil + ? "Model safetensors files are missing or incomplete in this folder." + : "Model safetensors files are incomplete. Please delete and re-download." + throw NSError(domain: "InferenceEngine", code: 1, userInfo: [NSLocalizedDescriptionKey: message]) } // Read the model's actual max context length from config.json setLoadingState(progress: 0.98, stage: "Reading model limits") - if let ctxLen = ModelStorage.readMaxContextLength(for: modelId) { + let ctxLen = explicitLocalDirectory.map(ModelStorage.readMaxContextLength(inDirectory:)) + ?? ModelStorage.readMaxContextLength(for: modelId) + if let ctxLen { self.maxContextWindow = ctxLen print("[InferenceEngine] Model context window: \(ctxLen) tokens") } else { @@ -459,9 +494,16 @@ public final class InferenceEngine: ObservableObject { downloadManager.clearProgress(modelId: modelId) state = .error("Failed to load \(modelId): \(error.localizedDescription)") - // If the model is incomplete/corrupted, flag it so the UI shows the "Delete & Re-download" button + // If the model is incomplete/corrupted, flag it so the UI shows the "Delete + // & Re-download" button — except for a local directory, where that recovery + // makes no sense: there's no repo to re-download from, and "delete" would + // silently no-op anyway (ModelStorage.delete only ever resolves paths under + // cacheRoot, so an external-drive path never matches anything it would try + // to remove). Surface the plain error instead. let nsError = error as NSError - if nsError.domain == "InferenceEngine" && nsError.code == 1 || Self.isModelCorruptionError(error) { + if explicitLocalDirectory == nil, + nsError.domain == "InferenceEngine" && nsError.code == 1 || Self.isModelCorruptionError(error) + { markModelCorrupted( modelId: modelId, message: "Model weights are corrupted or incomplete. Choose a recovery option." diff --git a/Sources/MLXInferenceCore/ModelStorage.swift b/Sources/MLXInferenceCore/ModelStorage.swift index 98aeb21..1e45a2e 100644 --- a/Sources/MLXInferenceCore/ModelStorage.swift +++ b/Sources/MLXInferenceCore/ModelStorage.swift @@ -223,7 +223,19 @@ public enum ModelStorage { /// Checks `text_config.max_position_embeddings` first (VLM/MoE models), /// then falls back to top-level `max_position_embeddings`. public static func readMaxContextLength(for modelId: String) -> Int? { - guard let config = readModelConfig(for: modelId) else { return nil } + maxContextLength(fromConfig: readModelConfig(for: modelId)) + } + + /// Same as `readMaxContextLength(for:)` but for a model addressed by an + /// arbitrary directory rather than a HuggingFace-cache-resolved model ID — + /// used when loading a model the user pointed at directly (see + /// `isLocalDirectoryPath`). + public static func readMaxContextLength(inDirectory directory: URL) -> Int? { + maxContextLength(fromConfig: readModelConfig(inDirectory: directory)) + } + + private static func maxContextLength(fromConfig config: [String: Any]?) -> Int? { + guard let config else { return nil } // VLM/MoE models nest the context length in text_config if let textConfig = config["text_config"] as? [String: Any], @@ -243,6 +255,31 @@ public enum ModelStorage { return nil } + // MARK: — Local Directory Models + + /// Whether `modelId` is actually a filesystem path to a directory the user + /// pointed the app at directly (e.g. a model on an external drive downloaded + /// via `hf download --local-dir`), rather than a HuggingFace repo ID. + /// + /// Mirrors the identical check the `SwiftLM` CLI's `Server.swift` already + /// does for `--model`: a plain `FileManager` existence + directory check, no + /// `~` expansion (the caller is expected to hand over an already-resolved + /// absolute path — see `ModelManagementView`'s `NSOpenPanel` flow). + public static func isLocalDirectoryPath(_ modelId: String) -> Bool { + var isDirectory: ObjCBool = false + return FileManager.default.fileExists(atPath: modelId, isDirectory: &isDirectory) + && isDirectory.boolValue + } + + /// Whether `directory` looks like a usable model folder — same weight/config + /// validation `verifyModelIntegrity` applies to HuggingFace-cache layouts, + /// reused here so a folder picked via `NSOpenPanel` can be rejected with a + /// clear error before `InferenceEngine.load` ever attempts to construct a + /// model from it. + public static func validateLocalModelDirectory(_ directory: URL) -> Bool { + validateModelFiles(in: directory, logFailures: true) + } + /// Read the raw config.json dictionary for a downloaded model. /// Verifies that all required safetensors files are present in the snapshot directory. /// This prevents the engine from entering `.ready` state if a download was interrupted or corrupted. @@ -347,15 +384,22 @@ public enum ModelStorage { public static func readModelConfig(for modelId: String) -> [String: Any]? { for directory in modelContentDirectories(for: modelId) { - let configPath = directory.appendingPathComponent("config.json") - guard let data = try? Data(contentsOf: configPath), - let config = try? JSONSerialization.jsonObject(with: data) as? [String: Any] - else { continue } - return config + if let config = readModelConfig(inDirectory: directory) { return config } } return nil } + /// Same as `readModelConfig(for:)` but for an already-resolved directory — + /// used for local models addressed directly by path (see + /// `isLocalDirectoryPath`), which have no HuggingFace-cache layout to scan. + public static func readModelConfig(inDirectory directory: URL) -> [String: Any]? { + let configPath = directory.appendingPathComponent("config.json") + guard let data = try? Data(contentsOf: configPath), + let config = try? JSONSerialization.jsonObject(with: data) as? [String: Any] + else { return nil } + return config + } + // MARK: — Disk Operations /// Total bytes used by all model files on disk. diff --git a/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift b/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift index e59cd66..3ed5236 100644 --- a/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift +++ b/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift @@ -10,6 +10,7 @@ struct ModelManagementView: View { @State private var showDeleteAll = false @State private var showHFSearch = false @State private var deletionError: String? = nil + @State private var localModelError: String? = nil @EnvironmentObject private var dm: ModelDownloadManager @@ -59,6 +60,14 @@ struct ModelManagementView: View { }, message: { Text(deletionError ?? "") }) + .alert("Can't Use This Folder", isPresented: Binding( + get: { localModelError != nil }, + set: { if !$0 { localModelError = nil } } + ), actions: { + Button("OK") { localModelError = nil } + }, message: { + Text(localModelError ?? "") + }) .safeAreaInset(edge: .bottom) { if let (modelId, progress) = dm.activeDownloads.first { FloatingDownloadBanner(modelId: modelId, progress: progress) @@ -130,8 +139,28 @@ struct ModelManagementView: View { .padding(.vertical, 4) } .buttonStyle(.plain) + #if os(macOS) + Button { addLocalModel() } label: { + HStack { + Image(systemName: "folder.badge.plus") + .foregroundStyle(.blue) + Text("Add Local Model…") + Spacer() + Image(systemName: "chevron.right") + .foregroundStyle(.secondary) + } + .padding(.vertical, 4) + } + .buttonStyle(.plain) + #endif + } footer: { + #if os(macOS) + Text("Point at a model folder you already have on disk — an external drive, a folder from `hf download --local-dir`, anywhere. It's loaded directly, without copying it into the cache above.") + .font(.caption) + .foregroundStyle(.secondary) + #endif } - + // Storage summary card Section { storageCard @@ -320,7 +349,17 @@ struct ModelManagementView: View { } } .buttonStyle(.borderedProminent) - + + #if os(macOS) + Button { addLocalModel() } label: { + HStack { + Image(systemName: "folder.badge.plus") + Text("Add Local Model…") + } + } + .buttonStyle(.bordered) + #endif + Button("Cancel") { dismiss() } .foregroundStyle(.secondary) .padding(.top, 4) @@ -330,6 +369,35 @@ struct ModelManagementView: View { // MARK: — Helpers + #if os(macOS) + /// Lets the user point directly at a model folder that already exists on + /// disk — an external drive, a folder from `hf download --local-dir`, + /// anywhere — without downloading or copying anything into the app's own + /// cache. Validated up front (same weight/config check `InferenceEngine` + /// itself would apply) so a bad selection fails here with a clear message + /// instead of surfacing as a confusing error partway through loading. + private func addLocalModel() { + let panel = NSOpenPanel() + panel.title = "Select Model Folder" + panel.canChooseDirectories = true + panel.canChooseFiles = false + panel.allowsMultipleSelection = false + panel.prompt = "Add" + + guard panel.runModal() == .OK, let url = panel.url else { return } + + guard ModelStorage.validateLocalModelDirectory(url) else { + localModelError = + "\(url.lastPathComponent) doesn't look like a model folder — " + + "no config.json or safetensors weights were found there." + return + } + + dismiss() + Task { await engine.load(modelId: url.path) } + } + #endif + private func deleteModel(_ modelId: String) { do { // Unload the currently loaded model BEFORE attempting filesystem deletion diff --git a/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift b/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift new file mode 100644 index 0000000..ef43ed8 --- /dev/null +++ b/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift @@ -0,0 +1,115 @@ +import XCTest +import Foundation +@testable import MLXInferenceCore + +// MARK: - Regression tests for Issue #160 — "Specify Alternate Model Location" +// +// A user with models on an external drive (downloaded via `hf download --local-dir`, +// entirely outside the app's own cache) had no way to point SwiftBuddy at that folder +// directly. Unlike issue #110's hand-copied-into-the-cache layouts, these directories +// can be anywhere on disk — the functions under test here must work independent of +// `ModelStorage.cacheRoot` (no `cacheRootOverride` needed). +final class LocalModelDirectoryTests: XCTestCase { + + private var externalDir: URL! + + override func setUpWithError() throws { + // Deliberately NOT under any cacheRoot — simulates an external drive folder. + externalDir = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("swiftlm-external-model-tests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: externalDir, withIntermediateDirectories: true) + } + + override func tearDownWithError() throws { + try? FileManager.default.removeItem(at: externalDir) + } + + private func writeConfig(_ json: String) throws { + try json.write( + to: externalDir.appendingPathComponent("config.json"), atomically: true, encoding: .utf8) + } + + private func writeWeights(bytes: Int = 4096) throws { + try Data(repeating: 0x7, count: bytes) + .write(to: externalDir.appendingPathComponent("model.safetensors")) + } + + // MARK: - isLocalDirectoryPath + + func testIsLocalDirectoryPathTrueForExistingDirectory() { + XCTAssertTrue(ModelStorage.isLocalDirectoryPath(externalDir.path)) + } + + func testIsLocalDirectoryPathFalseForHFStyleId() { + // A real HF repo id never happens to exist as a literal filesystem path from cwd. + XCTAssertFalse(ModelStorage.isLocalDirectoryPath("mlx-community/Qwen3-8B-4bit")) + } + + func testIsLocalDirectoryPathFalseForAFile() throws { + try writeConfig(#"{"model_type":"qwen3"}"#) + let filePath = externalDir.appendingPathComponent("config.json").path + XCTAssertFalse( + ModelStorage.isLocalDirectoryPath(filePath), + "a file, not a directory, must not be treated as a local model directory") + } + + func testIsLocalDirectoryPathFalseForNonexistentPath() { + let missing = externalDir.appendingPathComponent("does-not-exist").path + XCTAssertFalse(ModelStorage.isLocalDirectoryPath(missing)) + } + + // MARK: - validateLocalModelDirectory + + func testValidateLocalModelDirectoryAcceptsValidModel() throws { + try writeConfig(#"{"model_type":"qwen3","num_hidden_layers":28}"#) + try writeWeights() + XCTAssertTrue(ModelStorage.validateLocalModelDirectory(externalDir)) + } + + func testValidateLocalModelDirectoryRejectsMissingConfig() throws { + try writeWeights() + XCTAssertFalse( + ModelStorage.validateLocalModelDirectory(externalDir), + "a folder with weights but no config.json is not a usable model") + } + + func testValidateLocalModelDirectoryRejectsMissingWeights() throws { + try writeConfig(#"{"model_type":"qwen3"}"#) + XCTAssertFalse( + ModelStorage.validateLocalModelDirectory(externalDir), + "a folder with only config.json and no weights is not a usable model") + } + + func testValidateLocalModelDirectoryRejectsEmptyFolder() { + XCTAssertFalse(ModelStorage.validateLocalModelDirectory(externalDir)) + } + + // MARK: - readModelConfig(inDirectory:) / readMaxContextLength(inDirectory:) + + func testReadModelConfigInDirectory() throws { + try writeConfig(#"{"model_type":"qwen3","max_position_embeddings":32768}"#) + let config = ModelStorage.readModelConfig(inDirectory: externalDir) + XCTAssertEqual(config?["model_type"] as? String, "qwen3") + } + + func testReadModelConfigInDirectoryNilWhenMissing() { + XCTAssertNil(ModelStorage.readModelConfig(inDirectory: externalDir)) + } + + func testReadMaxContextLengthInDirectoryTopLevel() throws { + try writeConfig(#"{"model_type":"qwen3","max_position_embeddings":32768}"#) + XCTAssertEqual(ModelStorage.readMaxContextLength(inDirectory: externalDir), 32768) + } + + func testReadMaxContextLengthInDirectoryNestedTextConfig() throws { + // VLM/MoE-style configs nest it under text_config — same convention + // readMaxContextLength(for:) already handles for HF-cache models. + try writeConfig(#"{"model_type":"qwen3_vl","text_config":{"max_position_embeddings":131072}}"#) + XCTAssertEqual(ModelStorage.readMaxContextLength(inDirectory: externalDir), 131_072) + } + + func testReadMaxContextLengthInDirectoryNilWhenAbsent() throws { + try writeConfig(#"{"model_type":"qwen3"}"#) + XCTAssertNil(ModelStorage.readMaxContextLength(inDirectory: externalDir)) + } +} From c527eb33df2e13e2c32e0369ef61dddf4403794c Mon Sep 17 00:00:00 2001 From: Simba Zhang Date: Wed, 26 Aug 2026 23:12:26 -0700 Subject: [PATCH 2/2] fix: address correctness bugs found in review of local model directory support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A code review found several issues in the previous commit's local-directory support (issue #160), stemming from modelId being overloaded to mean either an HF repo id or a local path with no single place resolving that once. ## Fixed - isMoE always false for local-directory models: ModelCatalog.all.first(where: { $0.id == modelId }) can never match a filesystem path, silently disabling SSD expert streaming for exactly the "large MoE model on external drive" use case this feature exists for. Now falls back to inspecting the local config.json directly (new ModelStorage.configIndicatesMoE, checking the same expert-count field names ModelProfiler.findExpertCounts uses) when the catalog lookup misses. Fixed in both InferenceEngine.loadVerifiedModel and the same independent pattern in SettingsView's currentModelIsMoE. - markModelCorrupted's "Delete & Re-download" recovery was only suppressed for local directories at the load-time catch block; three generation-time call sites (SSD streaming error, generic corruption error, latched SSD error) had no such guard, so a transient error during generation on a local-directory model would still offer a recovery button that doesn't work (delete no-ops, re-download treats the path as a repo id). Moved the guard into markModelCorrupted itself so every current and future call site gets it, rather than relying on each one remembering to add it. - A stale persisted local path (drive unplugged, folder moved/deleted since last session) silently fell through to the HF-id download path, attempting to download the raw filesystem path as a repo id instead of showing a clear error. load() now recognizes a path-shaped modelId ("/...", which no real HF id ever is) that no longer resolves to a directory and surfaces "the folder may have moved or its drive isn't connected" immediately. - isLocalDirectoryPath was checked twice per load (once in load(), again inside loadVerifiedModel), leaving a narrow window where the two checks could disagree if the path's existence changed in between. loadVerifiedModel now takes the already-resolved localDirectory as a parameter instead of re-deriving it. - Local-directory detection was duplicated three ways with two different definitions: Server.swift's CLI --model handling (plain dir check), Server.swift's resolveModelDirectory (dir + config.json check), and the new ModelStorage.isLocalDirectoryPath (plain dir check, previously described in its own doc comment as merely "mirroring" Server.swift rather than sharing logic with it). Both Server.swift call sites now call ModelStorage.isLocalDirectoryPath directly (the CLI target already depends on MLXInferenceCore). - A model loaded via "Add Local Model…" was invisible everywhere in the app once loaded — never in dm.downloadedModels since it was never downloaded, so no view indicated it as the current model. ModelManagementView now shows a "Current Model (Local)" section for it, with a "Show in Finder" action and deliberately no delete option (there is nothing in the app's cache to remove). - Load-failure error messages interpolated the raw absolute path; now shown as just the folder name, matching how downloaded models are already displayed elsewhere trimmed to their last path component. ## Tests - New ModelStorage.configIndicatesMoE tests (top-level and nested text_config expert-count fields, dense-model and zero-count negative cases). - New InferenceEngine test verifying a missing local path produces a clear, immediate error without ever reaching the network (hermetic — no real download attempt). - Full suite: 288/288 passing, zero regressions. --- .../MLXInferenceCore/InferenceEngine.swift | 99 ++++++++++++++----- Sources/MLXInferenceCore/ModelStorage.swift | 25 +++++ Sources/SwiftLM/Server.swift | 16 +-- .../Views/ModelManagementView.swift | 67 +++++++++++++ .../SwiftBuddy/Views/SettingsView.swift | 13 ++- .../LocalModelDirectoryTests.swift | 38 +++++++ .../SwiftBuddyTests/ModelLifecycleTests.swift | 27 +++++ 7 files changed, 246 insertions(+), 39 deletions(-) diff --git a/Sources/MLXInferenceCore/InferenceEngine.swift b/Sources/MLXInferenceCore/InferenceEngine.swift index e65dedb..fc137b1 100644 --- a/Sources/MLXInferenceCore/InferenceEngine.swift +++ b/Sources/MLXInferenceCore/InferenceEngine.swift @@ -311,8 +311,26 @@ public final class InferenceEngine: ObservableObject { // A path the user pointed the app at directly (e.g. via "Add Local // Model…") — never downloaded, never in the HF cache, so the usual // verify-or-download flow doesn't apply; go straight to loading it. + // Resolved once here (not re-checked inside loadVerifiedModel) so the + // answer can't change between this check and that one. if ModelStorage.isLocalDirectoryPath(modelId) { - await loadVerifiedModel(modelId: modelId) + await loadVerifiedModel(modelId: modelId, localDirectory: URL(filePath: modelId)) + return + } + + // A path-shaped modelId (starts with "/") that ISN'T currently a valid + // directory — most likely a local model whose directory the app last saw + // is now inaccessible (external drive unplugged, folder moved/renamed). + // A real HuggingFace repo id is always "org/name" with no leading slash, + // so this can't misfire on one. Without this check, `lastLoadedModelId` + // persisting a local path and auto-resuming on launch (SwiftBuddyApp) + // would fall through to verifyModelIntegrity/downloadThenLoad below and + // try to download the raw filesystem path as if it were a repo id. + if modelId.hasPrefix("/") { + state = .error( + "Can't find \"\(URL(filePath: modelId).lastPathComponent)\" — the folder may have " + + "moved or its drive isn't connected." + ) return } @@ -351,22 +369,22 @@ public final class InferenceEngine: ObservableObject { } } - private func loadVerifiedModel(modelId: String) async { + /// - Parameter localDirectory: pre-resolved by `load()` when `modelId` is + /// itself a directory path (e.g. picked via "Add Local Model…", possibly on + /// an external drive entirely outside `cacheRoot`) — `nil` for every normal + /// HuggingFace-id model. Passed in rather than re-derived here so the two + /// checks can't disagree if the path's existence changes in between (the + /// directory is deleted/unmounted between `load()`'s check and this call). + /// + /// Distinct from `ModelStorage.localLoadDirectory(for:)` below: that's for + /// an HF-style "org/name" id whose files were copied by hand into a + /// recognised `cacheRoot`-relative layout (issue #110) rather than + /// downloaded, resolved through the existing `ModelStorage` id-based + /// helpers, which still work for that case unchanged. + private func loadVerifiedModel(modelId: String, localDirectory: URL? = nil) async { setLoadingState(progress: 0.05, stage: "Preparing model configuration") currentModelId = modelId - - // Two distinct kinds of "not in the normal cache" model share this function: - // - `isLocalDirectoryPath`: modelId IS the directory (e.g. picked via "Add - // Local Model…", possibly on an external drive entirely outside cacheRoot) - // - `localDirectory`: modelId is still an HF-style "org/name" id, but its - // files were copied by hand into a recognised cacheRoot-relative layout - // (issue #110) rather than downloaded — resolved through the existing - // ModelStorage id-based helpers below, which still work for this case. - // Only the first needs its own directory-based helpers throughout this - // function; ModelStorage.*(for: modelId) already finds the right directory - // for the second, the same way it does for a normally-downloaded model. - let explicitLocalDirectory: URL? = - ModelStorage.isLocalDirectoryPath(modelId) ? URL(filePath: modelId) : nil + let explicitLocalDirectory = localDirectory do { let hub = HubApi(downloadBase: ModelStorage.cacheRoot) @@ -388,7 +406,21 @@ public final class InferenceEngine: ObservableObject { } else { config = ModelConfiguration(id: modelId) } - let isMoE = ModelCatalog.all.first(where: { $0.id == modelId })?.isMoE ?? false + // A local directory never matches a catalog id (the catalog only lists + // HuggingFace-style ids) — fall back to inspecting its own config.json + // rather than silently defaulting to "not MoE" and disabling SSD expert + // streaming for exactly the large-MoE-on-external-drive case this local- + // directory support exists for. + let isMoE: Bool + if let catalogIsMoE = ModelCatalog.all.first(where: { $0.id == modelId })?.isMoE { + isMoE = catalogIsMoE + } else if let explicitLocalDirectory, + let localConfig = ModelStorage.readModelConfig(inDirectory: explicitLocalDirectory) + { + isMoE = ModelStorage.configIndicatesMoE(localConfig) + } else { + isMoE = false + } let generationConfig = GenerationConfig.load() if generationConfig.enableMTP { setenv("SWIFTLM_MTP_ENABLE", "1", 1) @@ -492,18 +524,19 @@ public final class InferenceEngine: ObservableObject { } catch { ExpertStreamingConfig.shared.deactivate() downloadManager.clearProgress(modelId: modelId) - state = .error("Failed to load \(modelId): \(error.localizedDescription)") + // A local directory's modelId is a full absolute path — show just the + // folder name in the error text, matching how "Downloaded" rows + // elsewhere in the app already display HF ids trimmed to their last + // component, rather than a raw POSIX path that wraps awkwardly in a + // narrow error banner. + let displayName = explicitLocalDirectory?.lastPathComponent ?? modelId + state = .error("Failed to load \(displayName): \(error.localizedDescription)") // If the model is incomplete/corrupted, flag it so the UI shows the "Delete - // & Re-download" button — except for a local directory, where that recovery - // makes no sense: there's no repo to re-download from, and "delete" would - // silently no-op anyway (ModelStorage.delete only ever resolves paths under - // cacheRoot, so an external-drive path never matches anything it would try - // to remove). Surface the plain error instead. + // & Re-download" button. markModelCorrupted itself no-ops corruptedModelId + // for a local directory (see its doc comment) — no guard needed here. let nsError = error as NSError - if explicitLocalDirectory == nil, - nsError.domain == "InferenceEngine" && nsError.code == 1 || Self.isModelCorruptionError(error) - { + if nsError.domain == "InferenceEngine" && nsError.code == 1 || Self.isModelCorruptionError(error) { markModelCorrupted( modelId: modelId, message: "Model weights are corrupted or incomplete. Choose a recovery option." @@ -539,11 +572,25 @@ public final class InferenceEngine: ObservableObject { state = .loading(progress: min(max(progress, 0), 1), stage: stage) } + /// Flags a model as corrupted so the UI offers "Delete & Re-download" — except + /// for a local directory, where that recovery makes no sense: there's no repo + /// to re-download from, and delete would silently no-op anyway + /// (`ModelStorage.delete` only ever resolves paths under `cacheRoot`, so an + /// external-drive path never matches anything it would try to remove). This + /// guard lives here, not at each call site, so every current and future + /// caller gets it — a per-call-site guard is easy to add to one call and + /// forget on the others, which is exactly what happened before this was + /// centralized (three generation-time call sites had no guard while the + /// load-time one did). private func markModelCorrupted(modelId: String?, message: String) { let failedModelId = modelId ?? currentModelId releaseLoadedModelResources() state = .error(message) - corruptedModelId = failedModelId + if let failedModelId, ModelStorage.isLocalDirectoryPath(failedModelId) { + corruptedModelId = nil + } else { + corruptedModelId = failedModelId + } } private static func isModelCorruptionError(_ error: Error) -> Bool { diff --git a/Sources/MLXInferenceCore/ModelStorage.swift b/Sources/MLXInferenceCore/ModelStorage.swift index 1e45a2e..9c39c85 100644 --- a/Sources/MLXInferenceCore/ModelStorage.swift +++ b/Sources/MLXInferenceCore/ModelStorage.swift @@ -400,6 +400,31 @@ public enum ModelStorage { return config } + /// A cheap "does this config declare a Mixture-of-Experts model" check, for + /// models with no `ModelCatalog` entry to consult — chiefly local-directory + /// models (`isLocalDirectoryPath`), which can never match a catalog id since + /// the catalog only lists HuggingFace-style ids. + /// + /// Checks the same field names `Sources/SwiftLM/ModelProfiler.swift`'s + /// `findExpertCounts` looks for (`num_local_experts`/`num_experts`/ + /// `n_routed_experts`), at the top level and inside `text_config` (the most + /// common one-level VLM/multimodal wrapper). `ModelProfiler` itself lives in + /// the CLI target and isn't importable from here; this intentionally doesn't + /// replicate its full breadth-first nested-wrapper walk — this check only + /// needs a yes/no answer for "should SSD expert streaming default on," not + /// the exact expert counts, so the common cases are enough. + public static func configIndicatesMoE(_ config: [String: Any]) -> Bool { + let expertKeys = ["num_local_experts", "num_experts", "n_routed_experts"] + func hasExpertCount(_ container: [String: Any]) -> Bool { + expertKeys.contains { (container[$0] as? Int).map { $0 > 0 } ?? false } + } + if hasExpertCount(config) { return true } + if let textConfig = config["text_config"] as? [String: Any], hasExpertCount(textConfig) { + return true + } + return false + } + // MARK: — Disk Operations /// Total bytes used by all model files on disk. diff --git a/Sources/SwiftLM/Server.swift b/Sources/SwiftLM/Server.swift index 9ec27fe..d00754a 100644 --- a/Sources/SwiftLM/Server.swift +++ b/Sources/SwiftLM/Server.swift @@ -438,16 +438,9 @@ struct MLXServer: AsyncParsableCommand { // ── Load model ── var modelConfig: ModelConfiguration - let fileManager = FileManager.default - if fileManager.fileExists(atPath: modelId) { - var isDir: ObjCBool = false - fileManager.fileExists(atPath: modelId, isDirectory: &isDir) - if isDir.boolValue { - print("[SwiftLM] Loading from local directory: \(modelId)") - modelConfig = ModelConfiguration(directory: URL(filePath: modelId)) - } else { - modelConfig = ModelConfiguration(id: modelId) - } + if ModelStorage.isLocalDirectoryPath(modelId) { + print("[SwiftLM] Loading from local directory: \(modelId)") + modelConfig = ModelConfiguration(directory: URL(filePath: modelId)) } else if let localDirectory = ModelStorage.validatedContentDirectory(for: modelId) { // Any validated copy in the shared HF cache, in any supported layout. Note // this deliberately does NOT use localLoadDirectory: that skips the @@ -1340,8 +1333,7 @@ func resolveModelDirectory(modelId: String) -> URL? { let fm = FileManager.default // Direct local path - var isDir: ObjCBool = false - if fm.fileExists(atPath: modelId, isDirectory: &isDir), isDir.boolValue { + if ModelStorage.isLocalDirectoryPath(modelId) { let url = URL(filePath: modelId) // Verify config.json exists if fm.fileExists(atPath: url.appendingPathComponent("config.json").path) { diff --git a/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift b/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift index 3ed5236..479724b 100644 --- a/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift +++ b/SwiftBuddy/SwiftBuddy/Views/ModelManagementView.swift @@ -166,6 +166,16 @@ struct ModelManagementView: View { storageCard } + // A model loaded via "Add Local Model…" never shows up in the + // "Models" list below (it was never downloaded into the cache + // dm.downloadedModels scans) — without this, there is nowhere in + // the app that indicates a local model is the one currently loaded. + if let currentLocalModelPath { + Section("Current Model (Local)") { + localModelRow(path: currentLocalModelPath) + } + } + // Individual models Section("Models") { ForEach(dm.downloadedModels) { downloaded in @@ -243,6 +253,63 @@ struct ModelManagementView: View { .padding(.vertical, 4) } + // MARK: — Local Model Row + + /// The currently-loaded model's path, if it's a local directory not already + /// represented in `dm.downloadedModels` (which only ever contains models + /// scanned out of the app's own cache). + private var currentLocalModelPath: String? { + guard case .ready(let modelId) = engine.state, + ModelStorage.isLocalDirectoryPath(modelId) + else { return nil } + return modelId + } + + private func localModelRow(path: String) -> some View { + let url = URL(filePath: path) + return HStack(spacing: 12) { + ZStack { + RoundedRectangle(cornerRadius: 8) + .fill(colorForModel(path)) + .frame(width: 36, height: 36) + Image(systemName: "externaldrive") + .font(.callout) + .foregroundStyle(.white) + } + + VStack(alignment: .leading, spacing: 2) { + HStack { + Text(url.lastPathComponent) + .font(.headline) + .foregroundStyle(.primary) + Text("IN USE") + .font(.caption2.weight(.bold)) + .padding(.horizontal, 5).padding(.vertical, 2) + .background(Color.green.opacity(0.15)) + .foregroundStyle(.green) + .clipShape(Capsule()) + } + Text(url.deletingLastPathComponent().path) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + .truncationMode(.middle) + } + Spacer() + } + .contextMenu { + Button { + #if os(macOS) + NSWorkspace.shared.open(url) + #endif + } label: { + Label("Show in Finder", systemImage: "folder") + } + // No "Delete" here, deliberately: this folder isn't in the app's + // cache, so there's nothing here for the app to safely remove. + } + } + // MARK: — Model Row private func downloadedModelRow(_ downloaded: DownloadedModel) -> some View { diff --git a/SwiftBuddy/SwiftBuddy/Views/SettingsView.swift b/SwiftBuddy/SwiftBuddy/Views/SettingsView.swift index 3db7b41..59db680 100644 --- a/SwiftBuddy/SwiftBuddy/Views/SettingsView.swift +++ b/SwiftBuddy/SwiftBuddy/Views/SettingsView.swift @@ -34,7 +34,18 @@ struct SettingsView: View { private var currentModelIsMoE: Bool { guard case .ready(let modelId) = engine.state else { return false } - return ModelCatalog.all.first(where: { $0.id == modelId })?.isMoE ?? false + if let catalogIsMoE = ModelCatalog.all.first(where: { $0.id == modelId })?.isMoE { + return catalogIsMoE + } + // A local directory never matches a catalog id — fall back to its own + // config.json rather than silently reporting "not MoE" (mirrors + // InferenceEngine.loadVerifiedModel's identical fallback). + if ModelStorage.isLocalDirectoryPath(modelId), + let config = ModelStorage.readModelConfig(inDirectory: URL(filePath: modelId)) + { + return ModelStorage.configIndicatesMoE(config) + } + return false } private var currentModelId: String? { diff --git a/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift b/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift index ef43ed8..8debf60 100644 --- a/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift +++ b/tests/SwiftBuddyTests/LocalModelDirectoryTests.swift @@ -112,4 +112,42 @@ final class LocalModelDirectoryTests: XCTestCase { try writeConfig(#"{"model_type":"qwen3"}"#) XCTAssertNil(ModelStorage.readMaxContextLength(inDirectory: externalDir)) } + + // MARK: - configIndicatesMoE + // + // A local directory never matches a ModelCatalog entry (the catalog only + // lists HuggingFace ids), so InferenceEngine falls back to this to decide + // whether to default SSD expert streaming on — without it, a local MoE + // model would silently load with streaming disabled. + + func testConfigIndicatesMoETopLevelNumExperts() { + let config: [String: Any] = ["model_type": "qwen3_moe", "num_experts": 128] + XCTAssertTrue(ModelStorage.configIndicatesMoE(config)) + } + + func testConfigIndicatesMoENRoutedExperts() { + let config: [String: Any] = ["model_type": "deepseek_v3", "n_routed_experts": 256] + XCTAssertTrue(ModelStorage.configIndicatesMoE(config)) + } + + func testConfigIndicatesMoENestedInTextConfig() { + // The common one-level VLM/multimodal wrapper shape. + let config: [String: Any] = [ + "model_type": "qwen3_vl_moe", + "text_config": ["num_local_experts": 64], + ] + XCTAssertTrue(ModelStorage.configIndicatesMoE(config)) + } + + func testConfigIndicatesMoEFalseForDenseModel() { + let config: [String: Any] = ["model_type": "qwen3", "num_hidden_layers": 28] + XCTAssertFalse(ModelStorage.configIndicatesMoE(config)) + } + + func testConfigIndicatesMoEFalseForZeroExpertCount() { + // A placeholder/zero value must not be treated as "this is MoE" — + // matches ModelProfiler.findExpertCounts' "positive values only" rule. + let config: [String: Any] = ["model_type": "qwen3", "num_experts": 0] + XCTAssertFalse(ModelStorage.configIndicatesMoE(config)) + } } diff --git a/tests/SwiftBuddyTests/ModelLifecycleTests.swift b/tests/SwiftBuddyTests/ModelLifecycleTests.swift index 9a33ecf..b5ac6cf 100644 --- a/tests/SwiftBuddyTests/ModelLifecycleTests.swift +++ b/tests/SwiftBuddyTests/ModelLifecycleTests.swift @@ -63,6 +63,33 @@ final class ModelLifecycleTests: XCTestCase { XCTAssertEqual(status2, .requiresFlash) } + // Local-directory model support (issue #160): a modelId that looks like a + // path (leading "/") but no longer resolves to a real directory — the drive + // was unplugged, or the folder was moved/deleted since the app last saw it. + // Before this guard, load() fell through to verifyModelIntegrity/ + // downloadThenLoad and tried to download the raw filesystem path as if it + // were a HuggingFace repo id. This must return a clear, immediate error + // instead — critically, without ever reaching the network, so this test + // stays fast and hermetic. + @MainActor + func testLoadWithMissingLocalPathShowsClearErrorWithoutDownloadAttempt() async { + let engine = InferenceEngine() + let missingPath = "/private/tmp/swiftlm-lifecycle-tests-\(UUID().uuidString)/my-model" + XCTAssertFalse(FileManager.default.fileExists(atPath: missingPath)) + + await engine.load(modelId: missingPath) + + guard case .error(let message) = engine.state else { + return XCTFail("expected .error, got \(engine.state)") + } + XCTAssertTrue( + message.contains("my-model"), + "error should name the folder, not just say something generic: \(message)") + XCTAssertFalse( + message.lowercased().contains("download"), + "a missing local path must not be reported as a download failure: \(message)") + } + // Feature 15: TurboQuant Footprint Estimates func testFeature15_TurboQuantFootprint() { let qwen27 = ModelCatalog.all.first { $0.id == "mlx-community/Qwen3.5-27B-4bit" }!