feat(patient): add voice input/output to chatbot (NeuroBot)#176
Open
hasanravda wants to merge 7 commits intoAOSSIE-Org:mainfrom
Open
feat(patient): add voice input/output to chatbot (NeuroBot)#176hasanravda wants to merge 7 commits intoAOSSIE-Org:mainfrom
hasanravda wants to merge 7 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds voice input/output to the Patient app: Android RECORD_AUDIO permission and iOS microphone/speech usage strings, Kotlin Gradle plugin bump, new Dart VoiceService (STT/TTS) and dependencies, plus ChatbotScreen integration for listening, speech-to-text input, and text-to-speech output. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Chatbot as ChatbotScreen
participant Voice as VoiceService
participant STT as SpeechToText
participant TTS as FlutterTts
participant Chat as ChatManager
User->>Chatbot: Tap mic button
Chatbot->>Voice: startListening()
Voice->>STT: initialize & startListening()
STT-->>Voice: recognized text
Voice-->>Chatbot: speechStream update
Chatbot->>Chat: submit message
Chat-->>Chatbot: assistant message
Chatbot->>Voice: speak(assistantText)
Voice->>TTS: synthesize & play
TTS-->>User: audio output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
patient/lib/core/services/voice_service.dart (1)
137-141: Avoid exposing a terminaldispose()on an app-wide singleton.After these controllers are closed once, the singleton cannot be safely reused. Consider making disposal internal/testing-only, or rebuilding controllers on re-init.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patient/lib/core/services/voice_service.dart` around lines 137 - 141, The public dispose() on the app-wide singleton closes _speechController, _listeningController, and _speakingController and makes the singleton unusable; make disposal internal or resilient instead: either rename dispose() to a private/test-only method (e.g., _disposeForTesting()) and annotate for testing, or change dispose() to check-and-recreate controllers on next use (guard with isClosed checks and lazily reinitialize _speechController, _listeningController, _speakingController in their getters/initializers) so the singleton can be safely reused; update any callers/tests accordingly to use the new private/test-only method or rely on lazy init.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patient/android/app/src/main/AndroidManifest.xml`:
- Line 3: The iOS patient app is missing the NSMicrophoneUsageDescription and
NSSpeechRecognitionUsageDescription keys in its Info.plist, which will crash
voice capture at runtime; open the patient app's Info.plist (iOS Runner
Info.plist) and add NSMicrophoneUsageDescription and
NSSpeechRecognitionUsageDescription entries with clear, user-facing strings
(e.g., why the app needs microphone access and why speech recognition is used),
mirroring the therapist app's plist wording as a reference to ensure consistency
and proper localization.
In `@patient/lib/core/services/voice_service.dart`:
- Around line 30-47: The first initialize call on _speechToText inside
initialize() currently ignores its boolean result and startListening() re-calls
initialize() redundantly; capture the bool returned by
_speechToText.initialize(...) (e.g. assign to a new or existing flag like
_speechAvailable or reuse _isInitialized) when running initialize(), store that
result, and remove the second initialize() call from startListening(); then have
startListening() check the captured flag (and _isInitialized) to guard starting
the listening flow. Ensure you update references to initialize(), _speechToText,
and startListening() so the initialization result is used rather than
re-invoking initialize().
In `@patient/lib/presentation/chatbot/chatbot_screen.dart`:
- Line 20: The deduplication currently compares bot replies by text via the
_lastSpokenText field which suppresses valid repeated replies across different
turns; change the logic to dedupe by a stable message identifier instead:
introduce a new field like _lastSpokenMessageId, update the speak/tts trigger to
compare the incoming message.id (or timestamp/uniqueTurnId) against
_lastSpokenMessageId rather than the message text, and set _lastSpokenMessageId
when a message is actually spoken; update any usages around where
_lastSpokenText is referenced (including the block at lines 45-49) to use the
new identifier-based check so identical text in different turns will still be
spoken.
- Around line 64-69: In _toggleListening(), handle the boolean result returned
by _voiceService.startListening(): call startListening() into a variable, check
if it returned true before updating _isListening or relying on listening state,
and on false handle the failure path (e.g., show user feedback via
ScaffoldMessenger/SnackBar or update UI state and log the error); ensure the
stopListening() branch remains unchanged and that any state changes to
_isListening or UI are performed inside setState only when startListening()
succeeded.
---
Nitpick comments:
In `@patient/lib/core/services/voice_service.dart`:
- Around line 137-141: The public dispose() on the app-wide singleton closes
_speechController, _listeningController, and _speakingController and makes the
singleton unusable; make disposal internal or resilient instead: either rename
dispose() to a private/test-only method (e.g., _disposeForTesting()) and
annotate for testing, or change dispose() to check-and-recreate controllers on
next use (guard with isClosed checks and lazily reinitialize _speechController,
_listeningController, _speakingController in their getters/initializers) so the
singleton can be safely reused; update any callers/tests accordingly to use the
new private/test-only method or rely on lazy init.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
patient/android/app/src/main/AndroidManifest.xmlpatient/android/settings.gradlepatient/lib/core/services/voice_service.dartpatient/lib/presentation/chatbot/chatbot_screen.dartpatient/pubspec.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patient/lib/core/services/voice_service.dart`:
- Around line 137-140: The dispose() method currently closes singleton
StreamControllers (_speechController, _listeningController, _speakingController)
which breaks reuse and causes add-after-close errors; remove the calls that
close those controllers from dispose(), instead set a private boolean
_isDisposed flag in dispose(), and update startListening(), speak(), and any
async STT/TTS callbacks to check if (!_isDisposed) before calling add/emit to
those controllers; if you need to fully tear down, add a separate
shutdownOrReset() method that safely closes and recreates controllers so
singleton reuse remains safe.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
patient/lib/core/services/voice_service.dart (4)
150-154: State not explicitly updated afterstopSpeaking().Similar to the
speak()method,_isSpeakingis not explicitly set to false afterstop(). This relies on the completion/error handler firing, which may not happen on all platforms when speech is manually stopped.♻️ Explicit state update
Future<void> stopSpeaking() async { if (_isSpeaking) { await _flutterTts.stop(); + _isSpeaking = false; + _emitSpeaking(_isSpeaking); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patient/lib/core/services/voice_service.dart` around lines 150 - 154, After calling _flutterTts.stop() in stopSpeaking(), explicitly set the internal speaking state variable (_isSpeaking) to false immediately after the await to ensure consistent state across platforms; update the stopSpeaking() method to await _flutterTts.stop(), then set _isSpeaking = false and (optionally) notify any listeners or emit the same state change path used by speak() so callers rely on the explicit state instead of depending solely on completion/error handlers.
104-117: Consider settingpartialResults: falsesince only final results are used.
partialResults: true(line 112) causesonResultto fire repeatedly with intermediate transcriptions, but the callback only emits whenresult.finalResultis true (line 106). SettingpartialResults: falsewould reduce unnecessary callback invocations.♻️ Suggested change
await _speechToText.listen( onResult: (result) { if (result.finalResult) { _emitSpeech(result.recognizedWords); } }, listenFor: const Duration(seconds: 30), pauseFor: const Duration(seconds: 3), - partialResults: true, + partialResults: false, localeId: "en_US", onSoundLevelChange: (level) { // Optional: Handle sound level changes }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patient/lib/core/services/voice_service.dart` around lines 104 - 117, The listen call on _speechToText currently sets partialResults: true but only acts when result.finalResult is true; change the listen invocation in the _speechToText.listen call to set partialResults: false so onResult is only invoked for final results and reduce unnecessary callbacks—locate the _speechToText.listen block that uses onResult and _emitSpeech and update the partialResults flag accordingly.
136-148: Potential state inconsistency afterstop()before newspeak().When
_isSpeakingis true and_flutterTts.stop()is called (line 140), thesetCompletionHandlermay or may not fire depending on platform behavior. If it doesn't fire,_isSpeakingremains true until the new speech starts and triggerssetStartHandler. This creates a brief window where_isSpeakingis inconsistent.♻️ Explicit state reset after stop
Future<void> speak(String text) async { if (!_isInitialized) await initialize(); if (_isSpeaking) { await _flutterTts.stop(); + _isSpeaking = false; + _emitSpeaking(_isSpeaking); } try { await _flutterTts.speak(text);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patient/lib/core/services/voice_service.dart` around lines 136 - 148, The speak() method can leave _isSpeaking true if _flutterTts.stop() doesn't trigger the completion handler; after awaiting _flutterTts.stop() in speak(), explicitly reset the internal speaking state (e.g. set _isSpeaking = false and notify any listeners/handlers) before calling _flutterTts.speak(text) so state is consistent; reference the speak() method, the _flutterTts.stop() call, the _isSpeaking flag, and the existing setCompletionHandler/setStartHandler logic when applying the change.
15-23: Singleton cannot recover afterdispose()is called.Once
dispose()is called,_isDisposed = trueand controllers are closed. However, the singleton instance remains, and any subsequent calls toinitialize()will return early due to_isInitializedstill being true, while the streams are permanently closed. If the chatbot screen is revisited afterdispose()is called elsewhere, voice features will silently fail.Based on the relevant context from
chatbot_screen.dart(lines 86-93),dispose()on the screen does NOT call_voiceService.dispose(), so this may not be a current issue—but the API is misleading. Consider either:
- Removing
dispose()entirely (singleton lives for app lifetime), or- Adding a
reset()method that properly recreates controllers for reuse.♻️ Option 1: Remove dispose or make it a no-op for singleton safety
- Future<void> dispose() async { - if (_isDisposed) return; - _isDisposed = true; - await _speechToText.stop(); - await _flutterTts.stop(); - await _speechController.close(); - await _listeningController.close(); - await _speakingController.close(); - } + /// Stops active STT/TTS but keeps the singleton usable. + /// Call this when the app is backgrounded or voice features are no longer needed. + Future<void> stopAll() async { + await _speechToText.stop(); + await _flutterTts.stop(); + _isListening = false; + _isSpeaking = false; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patient/lib/core/services/voice_service.dart` around lines 15 - 23, The singleton's dispose() currently closes _speechController/_listeningController/_speakingController and sets _isDisposed, leaving the singleton unusable because initialize() returns early when _isInitialized is still true; add a public reset() method that re-creates the three StreamControllers, clears _isDisposed and _isInitialized (and any other internal state like _isListening/_isSpeaking), and update initialize() to call reset() or recreate controllers when it detects _isDisposed so the singleton can be re-initialized; alternatively, if you prefer lifecycle simpler, make dispose() a no-op for the singleton (leave controllers open) and remove setting _isDisposed, but do not both close controllers and keep initialize() returning early. Ensure you reference dispose(), initialize(), _isDisposed, _isInitialized, _speechController, _listeningController, and _speakingController when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@patient/lib/core/services/voice_service.dart`:
- Around line 150-154: After calling _flutterTts.stop() in stopSpeaking(),
explicitly set the internal speaking state variable (_isSpeaking) to false
immediately after the await to ensure consistent state across platforms; update
the stopSpeaking() method to await _flutterTts.stop(), then set _isSpeaking =
false and (optionally) notify any listeners or emit the same state change path
used by speak() so callers rely on the explicit state instead of depending
solely on completion/error handlers.
- Around line 104-117: The listen call on _speechToText currently sets
partialResults: true but only acts when result.finalResult is true; change the
listen invocation in the _speechToText.listen call to set partialResults: false
so onResult is only invoked for final results and reduce unnecessary
callbacks—locate the _speechToText.listen block that uses onResult and
_emitSpeech and update the partialResults flag accordingly.
- Around line 136-148: The speak() method can leave _isSpeaking true if
_flutterTts.stop() doesn't trigger the completion handler; after awaiting
_flutterTts.stop() in speak(), explicitly reset the internal speaking state
(e.g. set _isSpeaking = false and notify any listeners/handlers) before calling
_flutterTts.speak(text) so state is consistent; reference the speak() method,
the _flutterTts.stop() call, the _isSpeaking flag, and the existing
setCompletionHandler/setStartHandler logic when applying the change.
- Around line 15-23: The singleton's dispose() currently closes
_speechController/_listeningController/_speakingController and sets _isDisposed,
leaving the singleton unusable because initialize() returns early when
_isInitialized is still true; add a public reset() method that re-creates the
three StreamControllers, clears _isDisposed and _isInitialized (and any other
internal state like _isListening/_isSpeaking), and update initialize() to call
reset() or recreate controllers when it detects _isDisposed so the singleton can
be re-initialized; alternatively, if you prefer lifecycle simpler, make
dispose() a no-op for the singleton (leave controllers open) and remove setting
_isDisposed, but do not both close controllers and keep initialize() returning
early. Ensure you reference dispose(), initialize(), _isDisposed,
_isInitialized, _speechController, _listeningController, and _speakingController
when making the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
patient/lib/core/services/voice_service.dartpatient/lib/presentation/chatbot/chatbot_screen.dart
Member
Author
|
Can you review this @jddeep ? |
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.
Closes #175
📝 Description
Adds full voice capability to the patient-side NeuroBot chatbot screen. Patients can now speak their messages instead of typing, and the bot's responses are read aloud automatically using text-to-speech (TTS).
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
WhatsApp.Video.2026-03-04.at.4.09.34.AM.mp4
✅ Checklist
Summary by CodeRabbit
New Features
Chores