fix(voice): friendly STT error with direct settings link#761
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe changes extend the error categorization system to handle speech-to-text readiness failures by adding a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 1267-1268: The current JSX conditional in Conversations.tsx that
checks sendError.code includes 'voice_transcription' and causes the "Set up" CTA
to appear for non-setup transcription failures; update the conditional that
renders the "Set up" link/button to only include the setup-specific code
'stt_not_ready' (remove 'voice_transcription' from the OR check on
sendError.code), and handle 'voice_transcription' separately (e.g., render a
different error message or retry option) so only genuine setup errors in the
sendError.code branch trigger the setup flow.
- Around line 493-496: The current isSetupIssue check uses case-sensitive
message.includes(...) calls and can miss mixed-case backend messages; modify the
logic that computes isSetupIssue (the variable using message) to perform
case-insensitive matching by normalizing the message (e.g., const txt =
message.toLowerCase()) and then checking txt.includes('whisper'),
txt.includes('binary not found'), and txt.includes('stt model') or use
case-insensitive regexes; update any references to isSetupIssue accordingly.
In `@src/openhuman/channels/proactive.rs`:
- Around line 110-113: The change forces thread_id to "default-thread" and emits
"chat_done", which breaks the frontend's proactive routing that listens for
"proactive_message" and "proactive:*" thread IDs; revert the emission to
"proactive_message" (instead of "chat_done") and stop hardcoding thread_id —
either preserve the incoming proactive thread id or construct a "proactive:{id}"
pattern (use the existing thread_id variable rather than "default-thread") so
the frontend's ChatService/ChatRuntimeProvider can route the event correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 069653e9-7797-4c5d-9089-79eff02793bb
📒 Files selected for processing (5)
app/src/chat/chatSendError.tsapp/src/pages/Conversations.tsxsrc/openhuman/app_state/mod.rssrc/openhuman/app_state/ops.rssrc/openhuman/channels/proactive.rs
| const isSetupIssue = | ||
| message.includes('whisper') || | ||
| message.includes('binary not found') || | ||
| message.includes('STT model'); |
There was a problem hiding this comment.
Make setup-error detection case-insensitive.
Line 494-Line 496 use case-sensitive includes(...); mixed-case backend messages can be misclassified and miss the setup recovery path.
Suggested fix
const message = err instanceof Error ? err.message : String(err);
+ const normalizedMessage = message.toLowerCase();
const isSetupIssue =
- message.includes('whisper') ||
- message.includes('binary not found') ||
- message.includes('STT model');
+ normalizedMessage.includes('whisper') ||
+ normalizedMessage.includes('binary not found') ||
+ normalizedMessage.includes('stt model');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isSetupIssue = | |
| message.includes('whisper') || | |
| message.includes('binary not found') || | |
| message.includes('STT model'); | |
| const normalizedMessage = message.toLowerCase(); | |
| const isSetupIssue = | |
| normalizedMessage.includes('whisper') || | |
| normalizedMessage.includes('binary not found') || | |
| normalizedMessage.includes('stt model'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 493 - 496, The current
isSetupIssue check uses case-sensitive message.includes(...) calls and can miss
mixed-case backend messages; modify the logic that computes isSetupIssue (the
variable using message) to perform case-insensitive matching by normalizing the
message (e.g., const txt = message.toLowerCase()) and then checking
txt.includes('whisper'), txt.includes('binary not found'), and txt.includes('stt
model') or use case-insensitive regexes; update any references to isSetupIssue
accordingly.
| {(sendError.code === 'stt_not_ready' || | ||
| sendError.code === 'voice_transcription') && ( |
There was a problem hiding this comment.
Show “Set up” only for setup-specific errors.
Line 1267-Line 1268 currently includes voice_transcription, which also represents non-setup failures. That can send users to model setup for unrelated transcription errors.
Suggested fix
- {(sendError.code === 'stt_not_ready' ||
- sendError.code === 'voice_transcription') && (
+ {sendError.code === 'stt_not_ready' && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {(sendError.code === 'stt_not_ready' || | |
| sendError.code === 'voice_transcription') && ( | |
| {sendError.code === 'stt_not_ready' && ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 1267 - 1268, The current JSX
conditional in Conversations.tsx that checks sendError.code includes
'voice_transcription' and causes the "Set up" CTA to appear for non-setup
transcription failures; update the conditional that renders the "Set up"
link/button to only include the setup-specific code 'stt_not_ready' (remove
'voice_transcription' from the OR check on sendError.code), and handle
'voice_transcription' separately (e.g., render a different error message or
retry option) so only genuine setup errors in the sendError.code branch trigger
the setup flow.
| // Use "default-thread" so the message appears in the user's | ||
| // visible conversation thread (matches DEFAULT_THREAD_ID in | ||
| // the frontend Conversations page). | ||
| let thread_id = "default-thread".to_string(); |
There was a problem hiding this comment.
Proactive event contract change is likely to break frontend proactive routing.
Line 127 switches emission to chat_done, and Line 113 forces default-thread. The current frontend proactive flow still listens for proactive_message and resolves proactive:* thread IDs (app/src/services/chatService.ts and app/src/providers/ChatRuntimeProvider.tsx snippets provided). This can bypass/detach the existing proactive dispatch path.
Suggested fix
- // Use "default-thread" so the message appears in the user's
- // visible conversation thread (matches DEFAULT_THREAD_ID in
- // the frontend Conversations page).
- let thread_id = "default-thread".to_string();
+ // Preserve proactive thread semantics expected by frontend routing.
+ let thread_id = format!("proactive:{}", job_name.as_deref().unwrap_or("system"));
@@
- // Emit as `chat_done` so the existing frontend chat handlers
- // pick it up — no dedicated `proactive_message` listener needed.
+ // Emit proactive event for dedicated proactive frontend handling.
publish_web_channel_event(WebChannelEvent {
- event: "chat_done".to_string(),
+ event: "proactive_message".to_string(),Also applies to: 124-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/channels/proactive.rs` around lines 110 - 113, The change
forces thread_id to "default-thread" and emits "chat_done", which breaks the
frontend's proactive routing that listens for "proactive_message" and
"proactive:*" thread IDs; revert the emission to "proactive_message" (instead of
"chat_done") and stop hardcoding thread_id — either preserve the incoming
proactive thread id or construct a "proactive:{id}" pattern (use the existing
thread_id variable rather than "default-thread") so the frontend's
ChatService/ChatRuntimeProvider can route the event correctly.
…missing Replace technical "whisper.cpp binary not found" error with a user-friendly message and a "Set up" link that navigates directly to Settings > Local AI Models so non-technical users can easily download the required speech model.
1aea77d to
a894f9a
Compare
…i#761) * fix(voice): show friendly error with settings link when STT model is missing Replace technical "whisper.cpp binary not found" error with a user-friendly message and a "Set up" link that navigates directly to Settings > Local AI Models so non-technical users can easily download the required speech model. * style: apply prettier formatting
Summary
/settings/local-modelso users can download the STT model in one clickstt_not_readyerror codeTest plan
/settings/local-modelyarn typecheckpassesyarn lintpassesSummary by CodeRabbit