Skip to content

Commit 913ae3f

Browse files
Address Copilot review feedback
1 parent 205bbea commit 913ae3f

9 files changed

Lines changed: 146 additions & 81 deletions

File tree

Sources/MLXInferenceCore/InferenceEngine.swift

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -528,11 +528,17 @@ public final class InferenceEngine: ObservableObject {
528528
}
529529

530530
let mlxMessages = finalMessages
531-
var params = GenerateParameters(temperature: config.temperature)
532-
params.topP = config.topP
533-
params.topK = config.topK
534-
params.minP = config.minP
535-
params.repetitionPenalty = config.repetitionPenalty
531+
var params = GenerateParameters(
532+
maxTokens: config.maxTokens,
533+
kvBits: config.kvBits,
534+
kvGroupSize: config.kvGroupSize,
535+
temperature: config.temperature,
536+
topP: config.topP,
537+
topK: config.topK,
538+
minP: config.minP,
539+
repetitionPenalty: config.repetitionPenalty,
540+
prefillStepSize: config.prefillSize
541+
)
536542
params.repetitionContextSize = 20
537543

538544
var thinkingActive = false
@@ -604,13 +610,7 @@ public final class InferenceEngine: ObservableObject {
604610
continuation.yield(GenerationToken(text: "\n\n[Error: \(error.localizedDescription)]"))
605611
}
606612

607-
// Also check the latch one final time (for errors that occurred during
608-
// generation and caused the stream to end without throwing)
609-
if let latchedError = SSDStreamingErrorLatch.shared.consume() {
610-
let msg = "Model weights are corrupted or incomplete. Please re-download the model."
611-
print("[InferenceEngine] Latched SSD error after generation: \(latchedError.localizedDescription)")
612-
self.markModelCorrupted(modelId: self.currentModelId, message: msg)
613-
} else if case .error = self.state {
613+
if case .error = self.state {
614614
// Already in error state from catch block above
615615
} else {
616616
self.state = self.currentModelId.map { .ready(modelId: $0) } ?? .idle

Sources/MLXInferenceCore/ModelDownloadManager.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ public final class ModelDownloadManager: ObservableObject {
5757
@Published public private(set) var totalDiskUsageBytes: Int64 = 0
5858
@Published public private(set) var networkStatus: NetworkStatus = .unknown
5959

60+
private var downloadedModelIDs: Set<String> = []
61+
6062
// MARK: Persistence
6163
private let lastModelKey = "swiftlm.lastLoadedModelId"
6264
public var lastLoadedModelId: String? {
@@ -131,6 +133,7 @@ public final class ModelDownloadManager: ObservableObject {
131133
modifiedDate: s.modifiedDate
132134
)
133135
}
136+
downloadedModelIDs = Set(downloadedModels.map(\.id))
134137
totalDiskUsageBytes = downloadedModels.reduce(0) { $0 + $1.sizeBytes }
135138

136139
// Scan for interrupted downloads that can be resumed
@@ -142,7 +145,7 @@ public final class ModelDownloadManager: ObservableObject {
142145
}
143146

144147
public func isDownloaded(_ modelId: String) -> Bool {
145-
ModelStorage.isDownloaded(modelId)
148+
downloadedModelIDs.contains(modelId)
146149
}
147150

148151
/// True if a model has a partial download that can be resumed.

Sources/MLXInferenceCore/ModelDownloader.swift

Lines changed: 62 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ public final class DownloadSpeedTracker: @unchecked Sendable {
166166
}
167167
}
168168

169+
private struct DownloadedFileSizeMismatchError: LocalizedError {
170+
let fileName: String
171+
let expectedSize: Int64
172+
let actualSize: Int64
173+
174+
var errorDescription: String? {
175+
"Downloaded file \(fileName) is incomplete (expected \(expectedSize) bytes, got \(actualSize))."
176+
}
177+
}
178+
169179
// MARK: — Downloader actor
170180

171181
public actor ModelDownloader {
@@ -276,14 +286,20 @@ public actor ModelDownloader {
276286
guard let freshHttp = freshResponse as? HTTPURLResponse, (200..<300).contains(freshHttp.statusCode) else {
277287
throw URLError(.badServerResponse)
278288
}
279-
try await streamToFile(
289+
let totalSize = freshHttp.expectedContentLength > 0 ? freshHttp.expectedContentLength : (expectedSize ?? 0)
290+
let writtenSize = try await streamToFile(
280291
asyncBytes: freshBytes,
281292
destURL: incompleteURL,
282293
resumeOffset: 0,
283-
totalSize: freshHttp.expectedContentLength > 0 ? freshHttp.expectedContentLength : (expectedSize ?? 0),
294+
totalSize: totalSize,
284295
speedTracker: speedTracker,
285296
onProgress: onProgress
286297
)
298+
try validateCompletedDownloadSize(
299+
fileName: fileName,
300+
actualSize: writtenSize,
301+
expectedSize: totalSize > 0 ? totalSize : expectedSize
302+
)
287303
} else if (200..<300).contains(http.statusCode) {
288304
// 200 = full content (server ignored Range), 206 = partial content (resume worked)
289305
let isResume = (http.statusCode == 206)
@@ -299,14 +315,19 @@ public actor ModelDownloader {
299315
} else {
300316
totalSize = http.expectedContentLength > 0 ? http.expectedContentLength : (expectedSize ?? 0)
301317
}
302-
try await streamToFile(
318+
let writtenSize = try await streamToFile(
303319
asyncBytes: asyncBytes,
304320
destURL: incompleteURL,
305321
resumeOffset: isResume ? resumeOffset : 0,
306322
totalSize: totalSize,
307323
speedTracker: speedTracker,
308324
onProgress: onProgress
309325
)
326+
try validateCompletedDownloadSize(
327+
fileName: fileName,
328+
actualSize: writtenSize,
329+
expectedSize: totalSize > 0 ? totalSize : expectedSize
330+
)
310331
} else {
311332
throw URLError(.badServerResponse)
312333
}
@@ -325,7 +346,7 @@ public actor ModelDownloader {
325346
totalSize: Int64,
326347
speedTracker: DownloadSpeedTracker,
327348
onProgress: @Sendable (Double, Double?) -> Void
328-
) async throws {
349+
) async throws -> Int64 {
329350
let fileHandle: FileHandle
330351
if resumeOffset > 0, FileManager.default.fileExists(atPath: destURL.path) {
331352
fileHandle = try FileHandle(forWritingTo: destURL)
@@ -336,43 +357,57 @@ public actor ModelDownloader {
336357
}
337358
defer { try? fileHandle.close() }
338359

339-
var bytesWritten: Int64 = resumeOffset
340-
var buffer = Data()
341360
let flushSize = 256 * 1024 // Flush every 256 KB
342361
var lastProgressUpdate = Date()
362+
var bytesWritten: Int64 = resumeOffset
363+
var iterator = asyncBytes.makeAsyncIterator()
364+
var chunkBuffer = [UInt8](repeating: 0, count: flushSize)
343365

344-
for try await byte in asyncBytes {
366+
while true {
345367
try Task.checkCancellation()
346-
buffer.append(byte)
347-
348-
if buffer.count >= flushSize {
349-
fileHandle.write(buffer)
350-
bytesWritten += Int64(buffer.count)
351-
buffer.removeAll(keepingCapacity: true)
352368

353-
speedTracker.record(totalBytes: bytesWritten)
369+
var chunkCount = 0
370+
while chunkCount < chunkBuffer.count,
371+
let byte = try await iterator.next() {
372+
chunkBuffer[chunkCount] = byte
373+
chunkCount += 1
374+
}
354375

355-
// Throttle progress updates to ~10/sec
356-
let now = Date()
357-
if now.timeIntervalSince(lastProgressUpdate) >= 0.1 {
358-
lastProgressUpdate = now
359-
if totalSize > 0 {
360-
onProgress(Double(bytesWritten) / Double(totalSize), speedTracker.speedBytesPerSec)
361-
}
362-
}
376+
if chunkCount == 0 {
377+
break
363378
}
364-
}
365379

366-
// Flush remaining bytes
367-
if !buffer.isEmpty {
368-
fileHandle.write(buffer)
369-
bytesWritten += Int64(buffer.count)
380+
fileHandle.write(Data(chunkBuffer[0..<chunkCount]))
381+
bytesWritten += Int64(chunkCount)
382+
370383
speedTracker.record(totalBytes: bytesWritten)
384+
385+
let now = Date()
386+
if now.timeIntervalSince(lastProgressUpdate) >= 0.1 {
387+
lastProgressUpdate = now
388+
if totalSize > 0 {
389+
onProgress(Double(bytesWritten) / Double(totalSize), speedTracker.speedBytesPerSec)
390+
}
391+
}
371392
}
372393

373394
if totalSize > 0 {
374395
onProgress(Double(bytesWritten) / Double(totalSize), speedTracker.speedBytesPerSec)
375396
}
397+
return bytesWritten
398+
}
399+
400+
private func validateCompletedDownloadSize(
401+
fileName: String,
402+
actualSize: Int64,
403+
expectedSize: Int64?
404+
) throws {
405+
guard let expectedSize, expectedSize > 0, actualSize != expectedSize else { return }
406+
throw DownloadedFileSizeMismatchError(
407+
fileName: fileName,
408+
expectedSize: expectedSize,
409+
actualSize: actualSize
410+
)
376411
}
377412

378413
/// Download all model files to `ModelStorage.cacheRoot` in the Hugging Face

Sources/SwiftLM/Server.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,6 +1904,8 @@ func handleChatStreaming(
19041904
switch info.stopReason {
19051905
case .length:
19061906
reason = "length"
1907+
case .error:
1908+
reason = "error"
19071909
case .cancelled, .stop:
19081910
reason = hasToolCalls ? "tool_calls" : "stop"
19091911
}
@@ -2264,6 +2266,8 @@ func handleTextStreaming(
22642266
switch info.stopReason {
22652267
case .length:
22662268
reason = "length"
2269+
case .error:
2270+
reason = "error"
22672271
case .cancelled, .stop:
22682272
reason = "stop"
22692273
}

SwiftBuddy/SwiftBuddy/ViewModels/ServerManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ private var swiftBuddyJSONHeaders: HTTPFields {
4848
private func swiftBuddyJSONString(_ value: String) -> String {
4949
guard let data = try? JSONEncoder().encode(value),
5050
let string = String(data: data, encoding: .utf8) else {
51-
return #"""#
51+
return "\"\""
5252
}
5353
return string
5454
}

SwiftBuddy/SwiftBuddy/Views/ModelsView.swift

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ struct ModelsView: View {
1212
@State private var showHFSearch = false
1313
@State private var showManagement = false
1414
@State private var device = DeviceProfile.current
15+
@State private var deleteErrorMessage: String?
1516

1617
private var dm: ModelDownloadManager { engine.downloadManager }
1718

@@ -47,7 +48,7 @@ struct ModelsView: View {
4748
IncompleteDownloadRow(
4849
incomplete: incomplete,
4950
onResume: { dm.resumeDownload(modelId: incomplete.id) },
50-
onDelete: { try? dm.delete(incomplete.id) }
51+
onDelete: { deleteModel(incomplete.id) }
5152
)
5253
.padding(.horizontal)
5354
if incomplete.id != dm.incompleteDownloads.last?.id {
@@ -83,7 +84,7 @@ struct ModelsView: View {
8384
entry: entry,
8485
isActive: isActive,
8586
onLoad: { Task { await engine.load(modelId: downloaded.id) } },
86-
onDelete: { try? dm.delete(downloaded.id) }
87+
onDelete: { deleteModel(downloaded.id) }
8788
)
8889
.padding(.horizontal)
8990
if downloaded.id != dm.downloadedModels.last?.id {
@@ -207,6 +208,25 @@ struct ModelsView: View {
207208
ModelManagementView()
208209
.environmentObject(engine)
209210
}
211+
.alert(
212+
"Delete Failed",
213+
isPresented: Binding(
214+
get: { deleteErrorMessage != nil },
215+
set: { if !$0 { deleteErrorMessage = nil } }
216+
)
217+
) {
218+
Button("OK", role: .cancel) {}
219+
} message: {
220+
Text(deleteErrorMessage ?? "")
221+
}
222+
}
223+
224+
private func deleteModel(_ modelId: String) {
225+
do {
226+
try dm.delete(modelId)
227+
} catch {
228+
deleteErrorMessage = error.localizedDescription
229+
}
210230
}
211231

212232
// MARK: — Helpers

SwiftBuddy/SwiftBuddy/Views/RootView.swift

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ struct RootView: View {
2525
@State private var showMindPalace = false
2626
@State private var showTextIngestion = false
2727
@State private var showModelManagement = false
28+
@State private var lastDownloadLogBucket: Int?
2829
enum Tab { case chat, models, palace, mindPalace, miner, settings }
2930

3031
var body: some View {
@@ -51,6 +52,7 @@ struct RootView: View {
5152
.sheet(isPresented: $showModelManagement) {
5253
ModelManagementView()
5354
.environmentObject(engine)
55+
.environmentObject(engine.downloadManager)
5456
}
5557

5658
.onReceive(NotificationCenter.default.publisher(for: .showTextIngestion)) { _ in
@@ -69,18 +71,26 @@ struct RootView: View {
6971
.onChange(of: engine.state) { oldState, newState in
7072
switch newState {
7173
case .idle:
74+
lastDownloadLogBucket = nil
7275
ConsoleLog.shared.info("Engine idle — no model loaded")
7376
case .loading:
77+
lastDownloadLogBucket = nil
7478
ConsoleLog.shared.info("Loading model…")
7579
case .downloading(let p, let speed):
76-
if Int(p * 100) % 25 == 0 {
77-
ConsoleLog.shared.debug("Downloading: \(Int(p * 100))% (\(speed))")
80+
let percent = Int(p * 100)
81+
let bucket = min((percent / 25) * 25, 100)
82+
if bucket != lastDownloadLogBucket, [0, 25, 50, 75, 100].contains(bucket) {
83+
lastDownloadLogBucket = bucket
84+
ConsoleLog.shared.debug("Downloading: \(percent)% (\(speed))")
7885
}
7986
case .ready(let modelId):
87+
lastDownloadLogBucket = nil
8088
ConsoleLog.shared.info("✓ Model ready: \(modelId)")
8189
case .generating:
90+
lastDownloadLogBucket = nil
8291
ConsoleLog.shared.debug("Generating…")
8392
case .error(let msg):
93+
lastDownloadLogBucket = nil
8494
ConsoleLog.shared.error("Engine error: \(msg)")
8595
}
8696
}

0 commit comments

Comments
 (0)