Reliability, performance, and UX overhaul: recording, model loading, downloads, updater, hotkeys#103
Open
schallau wants to merge 42 commits into
Open
Reliability, performance, and UX overhaul: recording, model loading, downloads, updater, hotkeys#103schallau wants to merge 42 commits into
schallau wants to merge 42 commits into
Conversation
Buffers delivered before the asset writer was wired up were silently discarded, so the session cold start (startRunning + a 300ms settle sleep) swallowed the first word(s) of every dictation. Retain early buffers on the audio queue and flush them once the writer is ready, create the writer before starting the capture session, and drop the settle sleep. Also compute the minimum-duration wait from recordingStartTime instead of parsing the filename.
The 'watchdog' comment in performModelLoad had no implementation and TranscriptionError.loadingTimeout was never thrown, so a hung WhisperKit init (corrupt model dir, low-RAM swap spiral) left the pill on 'Warming up model...' forever. Race the load against a bounded timeout (180s, 360s below recommended RAM) and discard a late orphan result instead of resurrecting stale state. Also let unloadModelIfCurrent match a variant that is still mid-load, so deleting a model during warm-up doesn't keep loading deleted files.
The commit path slept an unconditional 500ms 'waiting for focus' even though the recorder panel is non-activating and focus usually never left the target app — every dictation felt half a second slower than the model. Now the wait only happens when the target isn't frontmost, polling up to 500ms. Fall back to the current frontmost app when no target was captured, and widen the clipboard-restore window to 800ms so slow apps (Electron, remote desktops) don't paste the old clipboard — safe because restore is skipped unless the pasteboard still holds the transcript.
- Restart the rebuilt capture session when the input device changes while recording (unplug fallback or external switch); previously the new session was never started, so the dictation silently went dead and finalized truncated with no error. - Show 'Mic access off' in the pill when microphone permission is denied instead of proceeding into a silent zero-byte recording. - Treat models as present until the launch disk scan completes, so a hotkey press seconds after login no longer flashes a false 'Model not downloaded' error.
The auto-repair strings ('Cleaning duplicates...', 'Retrying
download...') were written into downloadError, so once ModelRow began
rendering that map they appeared as red failure notes while the retry
was actually progressing. Route them through a new downloadStatus map
rendered neutrally, and clear both maps on every terminal path
(success, failure, cancel). Also make file transcription's
no-model-selected error say where to fix it instead of the generic
'Model is not initialized'.
Parakeet models were marked 'Installed' if their cache directory contained any file at all, so an interrupted download showed as installed forever and then failed to load with no hint of the cause. Use FluidAudio's AsrModels.modelsExist required-files check instead. ParakeetEngine.loadModel also called downloadAndLoad, which silently kicked off a multi-hundred-MB Hugging Face fetch with no progress or cancel when files were missing; it now loads strictly from cache and throws an actionable modelFilesMissing error pointing at Settings → AI Models.
Every launch re-ran ~30 regex passes over every stored transcript, a cost that grows linearly with usage. Gate it behind a stored normalization-schema version so it runs once per rule change instead of every launch. Also sweep the Recordings directory at launch for audio files no history item references (pruned rows, failed imports) — only app-named files older than 24h, so in-flight recordings are never touched.
Two app-lifetime global keyDown monitors (modifier-combo cancel in AppDelegate, Escape in MiniRecorderView) woke SpeakType on every keystroke in every app just to bail on a guard. The combo-cancel monitors now live only while the hotkey is held, and the Escape monitors only while recording or processing. Also stop handling Fn in the NSEvent flagsChanged path while the suppressing CGEvent tap is active — a duplicate arriving after the 50ms dedupe window could double-toggle recording.
Removes files with no references outside themselves: the parallel onboarding PermissionsView, HistoryDetailView, HotkeyConfiguration (which claimed a Ctrl+Shift+Space default the real hotkey logic never used), RecordingSession, TranscriptionState, and the never-called WhisperService.transcribeChunk left over from removed chunk stitching. AudioInputView shrinks to just DeviceRow, the one component SettingsView actually uses. The KeyboardShortcuts package was imported in three files but its API never called and its one shortcut name never consumed — imports and README mention dropped (package ref left in the project for a separate pass).
The pill said only 'Warming up model...' for the whole 30-60s cold load, which reads as a hang at login. Surface the engine's live loadingStage (already tracked but never rendered) plus elapsed seconds once the load passes 5s, and widen the warming pill to fit.
Setup hard-blocked on granting Accessibility even though dictation works fine without it (text lands on the clipboard instead of auto-pasting). Once the mic is granted, offer 'Continue without auto-paste' with a note explaining the tradeoff and where to enable it later. Also drop a duplicated frame modifier.
WaveformView rendered Float.random noise re-rolled on every appearance, implying the bars represented the recording when they were decorative. Downsample the actual file into per-bucket peaks (16 kHz mono WAVs — a few ms of work, done off-main) and normalize so quiet recordings still show a shape.
- Progress callbacks now bail once a download is no longer active, so a cancelled transfer can't repaint a row the user already reset (or briefly mark a just-deleted model as installed at 100%). - Partial-download cleanup waits for the cancelled task to actually finish before deleting, instead of racing WhisperKit's in-flight writes and potentially leaving a tree the size scan later accepts. - Check free space (expected size + 20% margin) before starting a multi-GB download and fail immediately with an actionable message rather than minutes later with a raw write error.
Three @published writes per audio buffer re-rendered the entire observing pill 50-100x/s while recording. Accumulate the peak between publishes and emit at most 30x/s — the waveform gets a uniform sample cadence and older machines stop burning CPU on invisible re-renders.
While installing, the update sheet replaced every control with a static 'Update in progress' label, so a hung DMG download trapped the user until force-quit. Add UpdateService.cancelInstall(), which invalidates the download session and resolves the pending continuation with a cancellation the install flow resets on silently (no error banner for a user-initiated cancel). The Cancel button appears only during the download phase — once the installer is touching disk, finishing is safer than stopping halfway.
Nothing calls its API — the imports and the one unused shortcut name were already deleted — so drop the package reference, product dependency, and framework link from the project. Package.resolved regenerated by xcodebuild without the entry.
The old suite asserted on 'SpeakType Shortcuts' text and a 'Permissions' sidebar item, neither of which exists — it could never pass and guarded nothing. The new tests walk every real sidebar destination asserting a marker each screen renders unconditionally, and exercise the Settings General/Audio/Permissions tabs. Verified via build-for-testing only: executing XCUITests locally requires a code-signing identity (Gatekeeper refuses ad-hoc-signed test runners), which this environment lacks.
Same fix HistoryView already got: the hint hardcoded ⌘+Shift+Space, which has never been the default (Fn) or necessarily the user's choice.
The 180s load watchdog fired mid-load on a healthy 16GB machine: first load of a large model includes CoreML/ANE specialization, which legitimately runs several minutes, so the 'timeout' reported a working load as failed (this is what broke loading whisper-large-v3_turbo). Raise the ceiling to 600s/1200s — it exists to catch wedged loads, not to bound performance — and reword the error, which wrongly blamed RAM. Also from review: a load that finds its state reset by a concurrent unload now throws CancellationError instead of returning normally, and both selection flows re-check the model is still downloaded before selecting — deleting a model during warm-up could otherwise re-select it. ParakeetEngine gets the same protection (tracks its in-flight variant, unload cancels a warm-up, stale completions clean up and throw).
cancelDownload re-enables the Download button immediately, so a user could start a fresh download while the old task was still unwinding — whose deferred cleanup then deleted the new download's files. The cleanup now re-checks on the main actor that no new download is in flight for the variant before deleting.
On first run, startRecording fired the permission prompt and immediately began 'recording' into a mic it might never get — even a granted prompt produced an empty take. Recording now begins in the requestAccess callback when granted, and not at all when denied; the pill already handles a nil stop gracefully. Removes the fire-and- forget requestPermission.
The first load after downloading a model runs CoreML/ANE specialization — minutes for large models, cached by macOS afterward — but the UI presented it like any other load, which reads as a hang (and is exactly what made the too-tight watchdog look plausible). Track per-variant first-load completion and, during that one load, say 'First-time setup — optimizing model for your Mac' in the pill, the model row (with an honest few-minutes estimate replacing the 10-30s tooltip), and the recommended-model button. README's stale 30-60s note updated to match.
The hotkey system only supported bare modifier keys held alone; this adds the first modifier+key chord. Chords start on the key's keyDown (with the modifier held) and stop when the modifier is released, so hold-to-record and toggle modes both behave like the existing options. The suppressing event tap gains keyDown/keyUp in its mask — only when a chord is selected, so modifier-hotkey users keep the narrow flagsChanged-only mask — and swallows the chord's key (plus repeats and the trailing keyUp) so Option+Space doesn't also type into the target app. Without Accessibility the tap doesn't exist; NSEvent fallback monitors still trigger the chord, just without suppression. Because the tap mask now depends on the selection, the monitoring stack rebuilds when the hotkey setting changes.
The always-on resting pill divides opinion — some want the anchor, others see clutter that floats over everything while doing nothing. New General setting 'Show floating pill when idle' (default on keeps current behavior): when off, the panel is ordered out whenever it returns to idle and reappears the moment recording starts. Also rebuild the hotkey monitoring when Accessibility becomes granted mid-session: the suppressing event tap could only be created at launch, so granting the permission later left global hotkeys dead until an app restart (global key monitors need the grant; local ones don't — which made the hotkey appear to work only inside the app).
cancelDownload re-enabled downloads immediately and its deferred cleanup only skipped deletion when isDownloading was true. If a retry *completed* before the cancelled task's cleanup ran, isDownloading was false and the cleanup deleted the retry's freshly downloaded files. A boolean can't distinguish 'my download' from 'a newer retry that already finished'. Each download now claims a monotonic per-variant generation. Progress callbacks, completion/error handlers, and post-cancel cleanup only act while their generation is still current, so a superseded task can neither repaint the row nor delete a newer download's files. The auto-repair path (which also calls deleteModel) gets the same guard, and cancelDownload no longer runs cleanup at all when no task was actually running (avoids deleting a completed model).
On first run, startRecording requests mic access asynchronously, but the pill flipped isListening = true synchronously — so if the user denied the prompt, the HUD stayed stuck showing 'recording' until a manual stop/cancel. startRecording now reports back via an onStarted callback (true when capture begins, false when denied); the authorized path still fires it synchronously so the common case is unchanged. The pill enters the listening HUD only on true, and surfaces the mic-off hint on false.
The chord press is dispatched to the main queue asynchronously, but the release branch only queued a stop when isHotkeyPressed was already true. A fast tap can deliver the release event to the tap callback before the press block runs and sets isHotkeyPressed, so the stop was dropped and recording stuck on (and toggle mode wedged, since the release also resets the pressed flag). Queue the release unconditionally: press and release run on the main queue in FIFO order and handleHotkeyStateChange no-ops a release with no matching press, so this is safe. Applied to both the event-tap and NSEvent-fallback paths.
Inherited from the old AudioInputView when DeviceRow was extracted; git diff --check flagged it.
Adversarial review of the generation-token commit found two holes the guards missed: 1. Auto-repair's deleteModel was neither generation- nor cancellation- aware. Cancel-then-download during an in-flight 'Multiple models found' repair could let that deleteModel remove the new download's files and reset its UI — the exact race the tokens were meant to close. deleteModel now takes an expectedGeneration and bails (before any removal and before its terminal state writes) when a newer download has claimed the variant; cancelDownload tombstones the generation so an already-running repair loses ownership immediately. The user's trash button still passes nil for an unconditional delete. 2. refreshDownloadedModels' 'keep in-flight rows' filter dropped a download that COMPLETED during the disk scan (isDownloading flipped false, but the stale scan snapshot saw it <80% so didn't re-add it), reverting a just-installed model to the Download button. The filter now also retains completed rows (progress >= 1.0).
Two more eager-UI-state flips of the same class as the mic-permission fix: - selectAudioDevice resumed recording with a fire-and-forget startRecording() and then unconditionally set isListening = true, so switching to a device that can't be added (unplugged that instant, grabbed exclusively) left the pill showing a live HUD over nothing. It now uses the onStarted callback and returns to idle if capture doesn't restart. - The pill's model menu set selectedModel eagerly and only debugLogged a load failure, so picking an undownloaded/failing model left the app permanently switched to a model that can't load. It now reverts the selection on failure, matching the other selection paths.
A silent launch/periodic check reached showUpdateWindow twice for the same release — once via showUpdateWindowPublisher's sink (fired inside checkForUpdates) and once via performUpdateCheckIfNeeded's reminder check — and showUpdateWindow created a fresh NSWindow each time with no guard, stacking two identical windows. It now reuses the open window.
Author
|
Pushed a round of self-review fixes (through Three edge cases found and fixed:
Adjacent same-class issues fixed while in there:
Known, deliberately deferred (low priority, not regressions from this PR):
Happy to split any of this out into smaller PRs if that's easier to review. |
The generation tokens guarded UI/delete STATE but not concurrent WhisperKit writes: cancelDownload freed the slot (isDownloading=false) before the cancelled task's WhisperKit.download had actually stopped (cancellation is cooperative), and downloadModel gated only on isDownloading — so a quick cancel-then-retry could have two downloads writing the same cache directory, corrupting it or tripping 'Multiple models found'. The new download now captures the prior task and awaits it before writing (cancelDownload keeps the task reference for exactly this), so writers for a variant never overlap.
onStarted(true) fired immediately after beginAuthorizedRecording, but setupSession could leave an inputless session (device missing / can't be added) and the async writer setup could still throw — either way the pill entered the recording HUD over nothing. setupSession now returns whether a usable input was installed; beginAuthorizedRecording bails with onStarted(false) when there's no input, and onStarted(true) now fires from inside the writer-setup task, after the writer is created and the session start is scheduled (onStarted(false) on writer failure). Capture-time buffering is unaffected — isRecording still flips before the task, so no opening audio is lost.
The revert-on-failure added for the pill model menu was unconditional: selecting A then B, with A's load failing later, would revert to A's predecessor and clobber the newer B selection. Revert only when the failed variant is still the active selection.
…nload Adversarial re-verification found the download-serialization closed download-vs-download but left a residual: the cleanup task's file removal could still run concurrently with a retry's writes (retry awaited the cancelled download task, but the delete happens in a separate cleanup task). Point activeTasks at the cleanup task instead, so a retry captures it as previousTask and awaits BOTH the cancelled download unwinding and the cleanup delete before writing — the whole per-variant download/cancel/cleanup pipeline is now strictly serial. Also refresh a stale comment (onStarted is no longer synchronous).
Author
|
Another self-review round (through
On lint (raised in review): I checked — the serious violations in the touched files are all pre-existing (large files already over SwiftLint's length limits, plus long lines that predate this branch); none of the serious violations land on lines this PR added. The repo ships with ~380 violations and CI treats lint as advisory, so this isn't a regression from the PR. A cleanup pass would be a large, separate, noisy change — happy to do it as its own PR if you'd like, but I didn't want to bury these fixes under formatting churn. Build + unit tests green throughout. |
selectedDeviceId.didSet ignored setupSession()'s Bool result, so when a mic disappears mid-recording with no usable fallback (selectedDeviceId becomes nil), it still started an inputless session — the mid-recording twin of the false-HUD/no-capture bug just fixed on the start path. Now the restart only runs when a usable input was installed; otherwise the recording stays active but paused (capture resumes automatically if a mic reconnects, since fetchAvailableDevices reassigns the device), and the partial still finalizes on stop.
Adds tests for the pure logic introduced/changed in this branch that had no coverage (the reviewer flagged perf/UX-sensitive test gaps): - HotkeyOptionTests: the new Option+Space chord — keyCode(49)/.option/ isChord classification the event-tap branches on, plus the bare-modifier options, rawValue round-trip (persistence), and unique display names. - WaveformViewTests: peakSamples downsampling that replaced the random-noise placeholder — verifies real bucket count, 0...1 normalization, that peaks track actual loudness (a two-level synthesized WAV), and empty-on-missing. peakSamples changed from private to internal so @testable can reach it. - AIModelEngineRoutingTests: whisper/parakeet engine routing the Parakeet cache-validation fixes depend on — every catalog variant maps correctly, the partition is total and disjoint, unknown variants have no engine/size. 52 unit tests pass (was 36). make ci green; Release build clean.
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
1 participant
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.
This branch started as a privacy/updater hardening pass and grew into a broader reliability, performance, and UX overhaul driven by a full audit of the codebase. It's 29 commits, each small and self-contained with a detailed message explaining the defect and the fix — reviewing commit-by-commit is recommended. Happy to split this into smaller themed PRs if you'd prefer; the commits are organized so that's mechanical.
Recording pipeline
9ebdd5d). Buffers that arrived before theAVAssetWriterwas wired up were silently discarded; the session cold start plus a 300ms settle sleep meant every recording lost its opening word(s). Early buffers are now retained on the audio queue and flushed once the writer is ready; the writer is created beforestartRunning(); the settle sleep is gone. Min-duration wait now usesrecordingStartTimeinstead of parsing the filename.08925ec).selectedDeviceId.didSetrebuilt the capture session but never restarted it, so unplugging a mic finalized a truncated file with no error.0680a52). Recording now starts in therequestAccesscallback only when granted, instead of "recording" into a mic it might never get. Denied permission shows an actionable message in the pill instead of a silent zero-byte flow.2990c21) — was 3@Publishedwrites per audio buffer, re-rendering the whole pill 50–100×/s.Model loading
52da9bf, corrected byfd88eb8). A hungWhisperKitinit previously left the pill on "Warming up model..." forever (the old watchdog comment had no implementation). Loads now race a bounded timeout via a continuation (a task group can't work here — it awaits hung children). The timeout is 600s/1200s: first loads legitimately run minutes of CoreML/ANE specialization, verified on a 16GB machine where a 180s value killed a healthy load.ff182ae). During a variant's first-ever load the pill/model row/hero button say "First-time setup — optimizing model for your Mac" with elapsed time, instead of looking hung. README updated to match.fd88eb8). A load that finds its state reset by a concurrent unload/delete now throwsCancellationErrorinstead of returning as success (which re-selected a just-deleted model). Both "Use" flows re-verify the model is still downloaded before selecting. ParakeetEngine tracks its in-flight variant and gets the same protection.f151443). "Installed" previously meant "cache dir non-empty," so interrupted downloads showed installed forever; now validated with FluidAudio'smodelsExist.ParakeetEngine.loadModelloads strictly from cache instead ofdownloadAndLoad's silent Hugging Face fetch (no progress, no cancel, contradicts offline-first).Downloads & updater
12e52f2), and transient auto-repair status ("Retrying download...") renders neutrally instead of as a red failure (056d60b).8a5619a,7b5212b). Progress callbacks bail once inactive; partial-download cleanup waits for the cancelled task to finish and re-checks that no fresh download started (cancel-then-retry previously deleted the retry's files).8a5619a): multi-GB downloads fail immediately with an actionable message instead of minutes in with a raw write error.5c63a0f,f25ad31):autoUpdatehad three divergent defaults (Settings showed ON while the launch check read OFF) — now one registered default with a regression test; daily background re-check (menu-bar apps run for weeks); install is temp-copy-then-replaceItemAtinstead of delete-then-copy (a failed copy destroyed the installation); manual check shows "You're up to date"/failure feedback; a stalled download is cancellable instead of trapping the user in a frozen sheet;v1.2.3-devtag parsing fixed (leading-v strip only).Input & paste
2836b1d): the commit path slept an unconditional 500ms "waiting for focus" although the non-activating panel means focus usually never left — now polls only when the target isn't frontmost (zero delay in the common case). Clipboard restore window widened for slow apps; safe because restore is conditional on the pasteboard still holding the transcript.a077bf1): the combo-cancel and Escape global keyDown monitors previously lived for the app's lifetime, waking the app on every keystroke system-wide; now installed only while the hotkey is held / recording is active. Fn handling in the NSEvent path is skipped while the event tap owns it (closes a double-toggle window).25c501c): first modifier+key chord option. The event tap gains keyDown/keyUp in its mask only when a chord is selected and swallows the chord's key so it doesn't type into the target app; NSEvent fallback keeps the chord working without Accessibility. Monitoring rebuilds when the selection changes, and now also recovers when Accessibility is granted mid-session (337e815) — previously the tap could only be created at launch.337e815): Settings → General → "Show floating pill when idle" (default on preserves current behavior).Onboarding, history, misc UX
8f0fc60).LazyVStack; empty-state hints show the configured hotkey instead of a hardcoded wrong one (cd63f52,dcc14d3,3e7bd11).Float.randomnoise (932e1e0).dcc14d3).08925ec).Housekeeping
3bf61bd).bea407f,1cf9054).ea48c67).Validation
xcodebuild build+ the unit suite (speaktypeTests) pass at every commit-sized checkpoint; the recording, model-load, paste, hotkey, and update flows were exercised in a live build during development. Known gaps called out honestly: Escape still can't abort an in-flight WhisperKit transcription (not cancellable upstream), history persistence remains in UserDefaults, andModelDownloadService's state machine still needs a downloader-protocol refactor to be unit-testable.