fix(whisper): fix recording restart and stuck transcribing#51
fix(whisper): fix recording restart and stuck transcribing#51
Conversation
Add IsTranscribing state flag that blocks the toggle shortcut from starting a new recording while transcription is still in progress.
Show error state with retry/close options when stop_and_transcribe fails, instead of leaving the UI stuck on "transcribing" forever.
📝 WalkthroughWalkthroughA new transcription state tracking mechanism is introduced to prevent concurrent recording and transcription operations. The Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Frontend UI
participant RecordHandler as Record Handler
participant StateManager as State Manager
participant TranscribeHandler as Transcribe Handler
User->>UI: Click stop & transcribe
UI->>StateManager: Check IsTranscribing state
StateManager-->>UI: IsTranscribing = false
UI->>RecordHandler: Verify not currently recording
RecordHandler->>StateManager: Set IsTranscribing = true
StateManager-->>RecordHandler: State updated
RecordHandler->>TranscribeHandler: Start transcription
TranscribeHandler->>TranscribeHandler: Process audio
TranscribeHandler-->>RecordHandler: Transcription complete/error
RecordHandler->>StateManager: Set IsTranscribing = false
StateManager-->>RecordHandler: State reset
RecordHandler-->>UI: Result (success/error)
alt Transcription Success
UI->>UI: Display transcribed text
else Transcription Error
UI->>UI: Display trimmed error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/tauri/src-tauri/src/whisper/commands.rs (1)
107-142: Good cleanup pattern; however, error path emits event AND returns Err causing duplicate frontend handling.The closure-based approach with guaranteed cleanup at line 141 is solid—
IsTranscribingresets regardless of success or failure.However, lines 134-136 emit
transcription-errorAND returnErr(e). The frontend both listens for the event (line 130-134 in WhisperApp.tsx) and catches the invoke error (lines 79-84). Both handlers set the same state, which is redundant. Consider either:
- Only emit the event and return
Ok(String::new())on error, or- Only return
Errand let the catch block handle it, removing the event emission.This isn't breaking—React batches the duplicate state updates—but it creates unnecessary coupling.
♻️ Option: Remove event emission on error path
match transcriber.transcribe(&audio) { Ok(text) => { let _ = app.emit("transcription-complete", text.clone()); Ok(text) } Err(e) => { - let _ = app.emit("transcription-error", e.clone()); Err(e) } }Then the frontend relies solely on the catch block for error handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/whisper/commands.rs` around lines 107 - 142, The error path currently both emits the "transcription-error" event and returns Err from the closure (see transcriber.transcribe() match and the emit("transcription-error") call), causing duplicate frontend handling; remove the event emission on the Err branch and only return Err(e) so the caller/invoker handles the error (keep the existing IsTranscribing reset logic and the successful-path emit("transcription-complete") intact).apps/tauri/src-tauri/src/lib.rs (1)
759-762: Guard only protects the shortcut path; direct command calls bypass it.The
IsTranscribingcheck prevents restarting via keyboard shortcut, butstart_recording,start_recording_button_mode, andstart_recording_field_modeTauri commands (seeapps/tauri/src-tauri/src/whisper/commands.rslines 16-78) do not check this flag. If any caller invokes these commands directly while transcription is active, recording will start anyway.The current UI flow naturally prevents this (no record buttons during transcription), so this may be acceptable. Consider adding the guard to the command functions for API consistency if other consumers may call them.
♻️ Proposed fix to add guard in start_recording
In
apps/tauri/src-tauri/src/whisper/commands.rs, add the check:#[tauri::command] pub fn start_recording( state: tauri::State<RecorderState>, app: tauri::AppHandle, ) -> Result<(), String> { let is_recording = app.state::<crate::IsRecording>(); if is_recording.0.load(Ordering::Relaxed) { return Err("Already recording".to_string()); } + let is_transcribing = app.state::<crate::IsTranscribing>(); + if is_transcribing.0.load(Ordering::Relaxed) { + return Err("Transcription in progress".to_string()); + } let app_data_dir = app.path().app_data_dir().map_err(|e| e.to_string())?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src-tauri/src/lib.rs` around lines 759 - 762, The IsTranscribing guard in lib.rs only protects the keyboard shortcut path; add the same guard to the Tauri command handlers so direct API calls cannot start a new recording while transcription is active: in apps/tauri/src-tauri/src/whisper/commands.rs update start_recording, start_recording_button_mode, and start_recording_field_mode to read the IsTranscribing state (handle.state::<IsTranscribing>() or via a passed tauri::State<IsTranscribing>), check its AtomicBool with .load(Ordering::Relaxed), and return early (no-op) when true before proceeding to start recording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/tauri/src-tauri/src/lib.rs`:
- Around line 759-762: The IsTranscribing guard in lib.rs only protects the
keyboard shortcut path; add the same guard to the Tauri command handlers so
direct API calls cannot start a new recording while transcription is active: in
apps/tauri/src-tauri/src/whisper/commands.rs update start_recording,
start_recording_button_mode, and start_recording_field_mode to read the
IsTranscribing state (handle.state::<IsTranscribing>() or via a passed
tauri::State<IsTranscribing>), check its AtomicBool with
.load(Ordering::Relaxed), and return early (no-op) when true before proceeding
to start recording.
In `@apps/tauri/src-tauri/src/whisper/commands.rs`:
- Around line 107-142: The error path currently both emits the
"transcription-error" event and returns Err from the closure (see
transcriber.transcribe() match and the emit("transcription-error") call),
causing duplicate frontend handling; remove the event emission on the Err branch
and only return Err(e) so the caller/invoker handles the error (keep the
existing IsTranscribing reset logic and the successful-path
emit("transcription-complete") intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fd58c1e-d6f6-48ea-a10d-484ea3766fec
📒 Files selected for processing (3)
apps/tauri/src-tauri/src/lib.rsapps/tauri/src-tauri/src/whisper/commands.rsapps/tauri/src/WhisperApp.tsx
1 participant
Summary
IsTranscribingstate flag checked by the toggle shortcut, so pressing the shortcut while transcription is running is a no-op instead of starting a new recordinghandleStopnow shows the error state with retry/close options whenstop_and_transcribefails, instead of leaving the UI stuck on "transcribing" foreverTest plan
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes