fix: fixes the little mic icon when you stop talking in voice mode#9593
fix: fixes the little mic icon when you stop talking in voice mode#9593phact wants to merge 1 commit into
Conversation
WalkthroughAdds a cleanup in VoiceAssistant.handleCloseAudioInput to close audioContextRef.current and reset it to null when present; no other functional changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant VA as VoiceAssistant
participant AC as AudioContext (audioContextRef.current)
User->>VA: handleCloseAudioInput()
alt audioContext exists
VA->>AC: close()
AC-->>VA: closed
VA->>VA: audioContextRef.current = null
else no audioContext
VA->>VA: no-op
end
VA-->>User: return
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/frontend/src/modals/IOModal/components/chatView/chatInput/components/voice-assistant/voice-assistant.tsx (2)
94-99: Bug: undefined identifier in hook deps (openis not in scope)
hasOpenAIAPIKey’s deps includeopen, which is not defined in this component scope. This will fail TypeScript/lint and can break hook memoization.Apply this diff to fix the deps:
const hasOpenAIAPIKey = useMemo(() => { return ( variables?.find((variable) => variable === "OPENAI_API_KEY")?.length! > 0 ); - }, [variables, open, addKey]); + }, [variables, addKey]);Optional clarity (same behavior, cleaner):
- const hasOpenAIAPIKey = useMemo(() => { - return ( - variables?.find((variable) => variable === "OPENAI_API_KEY")?.length! > 0 - ); - }, [variables, addKey]); + const hasOpenAIAPIKey = useMemo( + () => Boolean(variables?.some((v) => v === "OPENAI_API_KEY")), + [variables, addKey], + );
320-343: Remove setTimeout race; await AudioContext close before re-initWhen reopening after settings, you
close()the AudioContext and then rely onsetTimeout(…, 100)to reinitialize.AudioContext.close()is async and timing can vary; racing re-init can cause intermittent errors or visual glitches (e.g., mic state).Apply this diff to make re-init deterministic:
- if (open) { + if (open) { stopRecording(); if (audioContextRef.current) { - audioContextRef.current.close(); + await audioContextRef.current.close(); audioContextRef.current = null; } setIsRecording(false); } else { setRecordingTime(0); setBarHeights(Array(30).fill(20)); if (hasOpenAIAPIKey) { if (audioContextRef.current) { - audioContextRef.current.close(); + await audioContextRef.current.close(); audioContextRef.current = null; } analyserRef.current = null; - setTimeout(() => { - initializeAudio(); - startRecording(); - setIsRecording(true); - }, 100); + await initializeAudio(); + await startRecording(); + setIsRecording(true); } }Note:
initializeAudio/startRecordingalready return promises (or can be made to). If they’re sync wrappers today, marking themasyncwithawait-ed internals is fine.
🧹 Nitpick comments (2)
src/frontend/src/modals/IOModal/components/chatView/chatInput/components/voice-assistant/voice-assistant.tsx (2)
278-289: Unmount effect dependencies: make it a true mount/unmount effectThis effect looks intended for mount work + cleanup. Tying it to
setShowAudioInputis unusual and brittle; the setter identity is stable and won’t drive re-runs meaningfully.- }, [setShowAudioInput]); + }, []);
282-289: DRY up AudioContext shutdown into a helperYou close + null the
AudioContextin three places. Centralize to avoid drift and future mistakes.Example helper (place inside component body):
const closeAudioContext = async () => { const ctx = audioContextRef.current; if (!ctx) return; try { await ctx.close(); } finally { audioContextRef.current = null; } };Then replace the repeated blocks with:
await closeAudioContext();Also applies to: 320-325, 331-335, 303-306
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/frontend/src/modals/IOModal/components/chatView/chatInput/components/voice-assistant/voice-assistant.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/frontend/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx,js,jsx}: All frontend TypeScript and JavaScript code should be located under src/frontend/src/ and organized into components, pages, icons, stores, types, utils, hooks, services, and assets directories as per the specified directory layout.
Use React 18 with TypeScript for all UI components in the frontend.
Format all TypeScript and JavaScript code using the make format_frontend command.
Lint all TypeScript and JavaScript code using the make lint command.
Files:
src/frontend/src/modals/IOModal/components/chatView/chatInput/components/voice-assistant/voice-assistant.tsx
🔇 Additional comments (2)
src/frontend/src/modals/IOModal/components/chatView/chatInput/components/voice-assistant/voice-assistant.tsx (2)
303-306: AudioContext cleanup on close — good fixClosing and nulling
audioContextRefwhen the user clicks the close button is correct and aligns with the PR’s goal to stop the little mic icon from lingering. No issues with the sequence givenstopRecording()is invoked first.
355-374: Confirm desired semantics for “Mute” toggle vs full stopToggling recording currently only disables/enables media tracks; it doesn’t stop the processor/WS pipeline. That’s fine if you want a quick “mute” without tearing down resources, but it will keep network/audio graph resources alive.
If the intent is to fully stop resources on mute:
- if (isRecording) { + if (isRecording) { if (microphoneRef?.current && microphoneRef?.current?.mediaStream) { microphoneRef.current.mediaStream.getAudioTracks().forEach((track) => { track.enabled = false; }); } setBarHeights(Array(30).fill(20)); - setIsRecording(false); + stopRecording(); + setIsRecording(false); } else {Would you like me to propagate this change and test for regressions?
2 participants
This pull request includes a small improvement to the audio input handling in the
VoiceAssistantcomponent. Now, when closing the audio input, the code ensures that theaudioContextis properly closed and cleaned up.Summary by CodeRabbit