Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 12 additions & 34 deletions Sources/Fluid/UI/AISettings/VoiceEngineSettingsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -75,11 +73,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)
}
Expand Down Expand Up @@ -173,34 +166,19 @@ final class VoiceEngineSettingsViewModel: ObservableObject {
self.asr.cancelModelPreparation()
}

func deleteSpeechModel(_ model: SettingsStore.SpeechModel) {
func deleteSpeechModel(_ model: SettingsStore.SpeechModel) async {
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
}
}

await self.deleteModels()
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"
)
}
}

Expand Down
4 changes: 3 additions & 1 deletion Sources/Fluid/UI/AISettingsView+SpeechRecognition.swift
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,9 @@ extension VoiceEngineSettingsView {
if !model.usesAppleLogo {
if isSelected {
Button {
self.viewModel.deleteSpeechModel(model)
Task {
await self.viewModel.deleteSpeechModel(model)
}
} label: {
Image(systemName: "trash")
.font(.system(size: 15))
Expand Down
86 changes: 86 additions & 0 deletions Tests/FluidDictationIntegrationTests/DictationE2ETests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,55 @@ final class DictationE2ETests: XCTestCase {
)
}

func testDeleteNonActiveSpeechModel_doesNotBounceSelectionThroughDeletedModel() 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.
// Use a built-in no-cache model here so the hosted test can catch the selection bounce
// without deleting a real downloaded speech model from the developer's machine.
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)
XCTAssertFalse(viewModel.areSpeechModelActionsBlocked)
await viewModel.deleteSpeechModel(modelToDelete)

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
Expand Down Expand Up @@ -1482,3 +1531,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
Loading