diff --git a/Sources/Fluid/UI/AISettings/VoiceEngineSettingsViewModel.swift b/Sources/Fluid/UI/AISettings/VoiceEngineSettingsViewModel.swift index bd561f02..a5d84292 100644 --- a/Sources/Fluid/UI/AISettings/VoiceEngineSettingsViewModel.swift +++ b/Sources/Fluid/UI/AISettings/VoiceEngineSettingsViewModel.swift @@ -28,8 +28,6 @@ final class VoiceEngineSettingsViewModel: ObservableObject { @Published var selectedSpeechProvider: SettingsStore.SpeechModel.Provider @Published var previewSpeechModel: SettingsStore.SpeechModel @Published var showAdvancedSpeechInfo: Bool = false - @Published var suppressSpeechProviderSync: Bool = false - @Published var skipNextSpeechModelSync: Bool = false var downloadingModel: SettingsStore.SpeechModel? { guard let modelID = self.asr.downloadingModelId else { return nil } @@ -72,11 +70,6 @@ final class VoiceEngineSettingsViewModel: ObservableObject { } func handleSelectedSpeechModelChange(_ newValue: SettingsStore.SpeechModel) { - if self.skipNextSpeechModelSync { - self.skipNextSpeechModelSync = false - return - } - guard !self.suppressSpeechProviderSync else { return } self.previewSpeechModel = newValue self.setSelectedSpeechProvider(newValue.provider) } @@ -172,32 +165,19 @@ final class VoiceEngineSettingsViewModel: ObservableObject { func deleteSpeechModel(_ model: SettingsStore.SpeechModel) { guard !self.areSpeechModelActionsBlocked else { return } - let previousActive = self.settings.selectedSpeechModel - Task { - let shouldRestore = previousActive != model - await MainActor.run { - if shouldRestore { - self.suppressSpeechProviderSync = true - } - self.settings.selectedSpeechModel = model - self.asr.resetTranscriptionProvider() - } - - defer { - Task { @MainActor in - guard shouldRestore else { return } - self.skipNextSpeechModelSync = true - self.settings.selectedSpeechModel = previousActive - self.asr.resetTranscriptionProvider() - if self.previewSpeechModel == model { - self.previewSpeechModel = model - } - self.suppressSpeechProviderSync = false - } + do { + try await self.asr.clearModelCache(for: model) + // Refresh installed-model state so the model cards reflect the deletion for + // every model, not only the active one (clearModelCache(for:) re-checks only + // when deleting the selected model). + await self.asr.checkIfModelsExistAsync() + } catch { + DebugLogger.shared.error( + "Failed to delete model \(model.displayName): \(error)", + source: "VoiceEngineVM" + ) } - - await self.deleteModels() } } diff --git a/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift b/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift index d659e03b..e187bb2b 100644 --- a/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift +++ b/Tests/FluidDictationIntegrationTests/DictationE2ETests.swift @@ -359,6 +359,56 @@ final class DictationE2ETests: XCTestCase { ) } + func testDeleteNonActiveSpeechModel_neverPersistsDeletedModelAsSelection() async throws { + // Regression: deleting a NON-ACTIVE speech model used to temporarily persist the + // deleted model as `selectedSpeechModel`, then restore the real selection through an + // un-awaited `defer { Task { ... } }`. If the process exited during that window (app + // quit, crash, or update relaunch) the user was left with a model they never chose. + // The fix deletes the target model's cache without ever writing the selection. + let defaults = UserDefaults.standard + let key = "SelectedSpeechModel" + let snapshot = defaults.object(forKey: key) + defer { + if let snapshot { + defaults.set(snapshot, forKey: key) + } else { + defaults.removeObject(forKey: key) + } + } + + let settings = SettingsStore.shared + let activeModel: SettingsStore.SpeechModel = .whisperTiny + let modelToDelete: SettingsStore.SpeechModel = .appleSpeech + XCTAssertNotEqual(activeModel, modelToDelete) + + settings.selectedSpeechModel = activeModel + XCTAssertEqual(settings.selectedSpeechModel, activeModel) + + // Record every value the persisted selection takes during the delete so a transient + // write of the deleted model is caught directly, not masked by the async restore. + let recorder = PersistedSpeechModelRecorder(defaults: defaults, key: key) + defer { recorder.stop() } + + let viewModel = VoiceEngineSettingsViewModel(settings: settings, appServices: AppServices.shared) + viewModel.deleteSpeechModel(modelToDelete) + + // `deleteSpeechModel` is fire-and-forget; let its async work and any deferred restore settle. + try await Task.sleep(nanoseconds: 1_000_000_000) + + XCTAssertEqual( + settings.selectedSpeechModel, + activeModel, + "Deleting a non-active model must leave the user's selection unchanged." + ) + XCTAssertFalse( + recorder.recordedRawValues.contains(modelToDelete.rawValue), + """ + Deleting a non-active model must never persist it as the selection, even transiently. \ + Recorded writes: \(recorder.recordedRawValues) + """ + ) + } + func testAppPromptBinding_profileOverridesModeSelection() { self.withPromptSettingsRestored { let settings = SettingsStore.shared @@ -1055,3 +1105,40 @@ final class DictationE2ETests: XCTestCase { ) } } + +// Classic key-path KVO is required here: an arbitrary UserDefaults key has no Swift `KeyPath` +// for the block-based API, and the change dictionary is delivered synchronously with the exact +// new value, which a coalescing `UserDefaults.didChangeNotification` cannot guarantee. +// swiftlint:disable block_based_kvo discouraged_optional_collection + +/// Records every value written to a UserDefaults key via KVO, so a transient write that is +/// later reverted can still be observed. Used to catch the non-active speech-model delete race, +/// where the deleted model was momentarily persisted as the selection before being restored. +private final class PersistedSpeechModelRecorder: NSObject { + private let defaults: UserDefaults + private let key: String + private(set) var recordedRawValues: [String] = [] + + init(defaults: UserDefaults, key: String) { + self.defaults = defaults + self.key = key + super.init() + defaults.addObserver(self, forKeyPath: key, options: [.new], context: nil) + } + + func stop() { + self.defaults.removeObserver(self, forKeyPath: self.key) + } + + override func observeValue( + forKeyPath keyPath: String?, + of object: Any?, + change: [NSKeyValueChangeKey: Any]?, + context: UnsafeMutableRawPointer? + ) { + guard keyPath == self.key, let rawValue = change?[.newKey] as? String else { return } + self.recordedRawValues.append(rawValue) + } +} + +// swiftlint:enable block_based_kvo discouraged_optional_collection