Skip to content

fix: do not persist a deleted speech model as the active selection - #468

Open
postoso wants to merge 1 commit into
altic-dev:mainfrom
postoso:fix/delete-speech-model-race
Open

fix: do not persist a deleted speech model as the active selection#468
postoso wants to merge 1 commit into
altic-dev:mainfrom
postoso:fix/delete-speech-model-race

Conversation

@postoso

@postoso postoso commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Description

Deleting a non-active speech model used to briefly persist the model being deleted as selectedSpeechModel, then restore the real selection through an un-awaited async restore. If the app quit, crashed, or relaunched during that window, the deleted model could become the saved active model.

This routes per-model deletion through an awaitable ASRService.clearModelCache(for:) call, which clears the target model without changing the saved selection. It also refreshes installed-model state after deletion so the model cards update without relying on a temporary selection change.

The removed suppressSpeechProviderSync and skipNextSpeechModelSync flags were only needed for the old selection bounce. With the bounce removed, the speech provider and preview state can stay directly synced to the actual selected model. The view-model method is now awaitable, so the regression test does not rely on a fixed sleep.

Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • 📝 Documentation update

Related Issue or Discussion

Related to #467. This fixes the delete-non-active-model persistence path only; it does not claim to close every possible path that could reset the active speech model.

Testing

  • Tested on Intel Mac
  • Tested on Apple Silicon Mac
  • Tested on macOS 15.7.7
  • Ran linter locally: swiftlint --strict --config .swiftlint.yml Sources Tests/FluidDictationIntegrationTests/DictationE2ETests.swift
  • Ran formatter locally: swiftformat --config .swiftformat Sources
  • Built locally: xcodebuild build-for-testing -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS,arch=arm64' CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO

Added DictationE2ETests.testDeleteNonActiveSpeechModel_doesNotBounceSelectionThroughDeletedModel. The test records every write to SelectedSpeechModel and fails if deleting .appleSpeech while .whisperTiny is active ever persists the deleted value, even transiently.

The test intentionally uses a built-in no-cache model so it can catch the selection-bounce regression without deleting a real downloaded speech model from the developer's machine.

Notes

This is intentionally narrow. It removes one concrete race in model deletion without changing model activation, update behavior, or broader speech-model fallback logic.

Full hosted test execution was not run locally because the installed FluidVoice app was open; the hosted test runner can hang in that state. build-for-testing compiled the app and test target, and CI should run the full hosted suite after the branch update.

Screenshots / Video

  • No UI/visual changes; screenshots/video are not applicable.

@postoso
postoso force-pushed the fix/delete-speech-model-race branch from bf50e0d to acf638e Compare July 6, 2026 10:41
Deleting a non-active speech model temporarily wrote the deleted model to
selectedSpeechModel and restored the real selection through an un-awaited
defer/Task that was never awaited. If the app quit, crashed, or relaunched
for an update during that window, the user was left with a model they never
chose.

Delete the target model's cache via the existing
ASRService.clearModelCache(for:) primitive, which clears the specific model
and never mutates the selection, and drop the now-unneeded
suppressSpeechProviderSync / skipNextSpeechModelSync scaffolding. Refresh
installed-model state after the delete so the model cards reflect the removal
for non-active models too, matching the onboarding uninstall path. Add a
regression test that fails if the deleted model is ever persisted as the
selection, even transiently.
@postoso
postoso force-pushed the fix/delete-speech-model-race branch from acf638e to 11ecadb Compare August 14, 2026 02:08
@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown

Greptile Summary

The PR removes the temporary selected-model swap previously used for cache deletion, directly clears the requested model cache, and refreshes installed-model state afterward.

  • Makes speech-model deletion awaitable from the settings UI.
  • Keeps provider and preview state synchronized directly with the actual selected model.
  • Adds regression coverage that records persisted model-selection writes during non-active-model deletion.

Confidence Score: 5/5

The PR appears safe to merge with no actionable changed-code defects identified.

Targeted cache deletion preserves the selected-model cleanup lifecycle while avoiding the transient persisted-selection mutation, and the new test directly covers that regression.

Reviews (1): Last reviewed commit: "fix: do not persist a deleted speech mod..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant