Improvement v2 - #9
Merged
Merged
Conversation
aschwarzkopf
added a commit
to aschwarzkopf/speaktype
that referenced
this pull request
Apr 24, 2026
Full code audit surfaced 15 bugs across 4 severity tiers; this commit addresses 14 of them (karansinghgit#15 deferred — Polar.sh handles server-side). It also introduces MiniRecorderViewModel, which fixes a subscription-timing race that caused "box appears, disappears, no audio" when pressing the hotkey. All 58 unit tests pass. 2 pre-existing UI test failures are unrelated (sidebar link lookups). CRITICAL karansinghgit#1 AudioRecordingService writer race — replaced `isStopping: Bool` with a RecordingStatus enum gated on audioQueue (RosyWriter pattern). All writer-state mutation now happens on audioQueue; concurrent/double stopRecording calls are safe. karansinghgit#2 MiniRecorderView NSEvent monitor leak + duplicate Escape handling — deleted the global/local NSEvent monitors; introduced KeyableMiniRecorderPanel (canBecomeKey=true) so the existing KeyEventHandlerView becomes the sole Escape path. karansinghgit#3 MiniRecorder state hygiene — fixed via the VM refactor below. HIGH karansinghgit#4 ModelDownloadService retry bounds — added `maxDownloadAttempts = 2`, `defer { activeTasks[variant] = nil }` for exactly-once handle cleanup. karansinghgit#5 AIModel.recommendedModel force unwrap — replaced with precondition + guarded fallback to smallest model. karansinghgit#6 TranscribeAudioView security-scoped resource — applied Apple's `defer` pattern, UUID-prefixed temp names, dropped unsound fallback to scoped URL after access was released. karansinghgit#7 AudioPlayerService silent load failure — loadAudio now async throws; added @published var lastError for SwiftUI .alert(item:) observation; made the service @mainactor; updated 2 callers. karansinghgit#8 LicenseManager race — added @published isValidatingLicense flag; network/transport failures no longer revoke cached license (only explicit server-confirmed invalid responses do), per RevenueCat / StoreKit 2 pattern. MEDIUM karansinghgit#9 WhisperService loadModel cancellation — pre-guard and post-guard Task.checkCancellation() per WWDC21 guidance; `defer` guarantees isLoading resets; late-cancelled pipe is discarded. karansinghgit#10 ModelDownloadService deleteModel — dropped /tmp and ~/.cache paths; replaced substring `contains` match with exact last-path-component equality; extracted static `shouldDelete(fileName:patterns:)` for testability. karansinghgit#11 UpdateService stale state — on network error, clear availableUpdate to avoid showing outdated update banner; added clearAvailableUpdate() helper. karansinghgit#12 MiniRecorderView.barHeight clamp — defensive min/max on audioLevel before sqrt() to guard against upstream bugs producing NaN. LOW karansinghgit#13 HistoryService deleteItem — filter out-of-bounds offsets before `remove(atOffsets:)` to tolerate stale IndexSet during rapid deletes. karansinghgit#14 MiniRecorderView debugLog — moved off world-readable /tmp to ~/Library/Application Support/SpeakType/Logs; gated behind #if DEBUG so release builds use print (unified logging) instead. VM REFACTOR (Option D from research) Fixes the "no audio on Fn press" bug introduced by karansinghgit#3's initial attempt to use `.id(sessionID)` for SwiftUI @State reset. The .id() rebuild forced SwiftUI to teardown/rebuild the view, but notifications posted synchronously after the swap hit an empty subscriber list — recording was never triggered. - New MiniRecorderViewModel (@mainactor, ObservableObject) owns all session-lifecycle state: isListening, isProcessing, isWarmingUp, statusMessage, cancelCommit, sessionID, pendingAction - Controller drives the VM directly via startSession/requestStop/ requestCancel — no more NotificationCenter round-trip, no more .id() churn - View observes via @ObservedObject; three .onReceive blocks collapsed into one .onChange(of: viewModel.pendingAction) draining consumeAction() - SwiftUI guarantees @published subscribers are attached before any mutation is observed, eliminating the timing race Test additions: - 3 audio concurrency tests, 3 panel, 5 controller, 4 AIModel, 3 audio player, 2 license, 5 exact-match deletion + 1 retry bound, 1 Whisper cancellation, 2 update service, 2 history offset, 12 VM contract tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.