feat: Voice Commands for Hands-Free Trail Navigation#1904
Conversation
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
🧩 Analysis chainLet me gather the code from this PR to perform a thorough review. 🏁 Script executed: Length of output: 604 🏁 Script executed: Length of output: 22508 🏁 Script executed: Length of output: 1728
Voice Commands Feature — Code Review🔴 Critical Issues1. No actual Speech-to-Text (STT) backend wired upThe hold-to-speak microphone button calls
const startListening = useCallback(() => {
setListeningState('listening');
// ❌ No mic recording, no STT call — transcript never arrives
}, []);Consider keeping 2.
|
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🔴 | useVoiceCommands.ts |
STT not connected — mic button is a no-op |
| 2 | 🔴 | useVoiceCommands.ts |
gps object in deps defeats memoization |
| 3 | 🔴 | useGPSTracking.ts |
Subscription leak if startTracking called twice; no try/catch |
| 4 | 🟡 | useVoiceCommands.ts |
Timeout not cleared on unmount |
| 5 | 🟡 | useVoiceCommands.ts |
Stale closure in stopListening |
| 6 | 🟡 | useGPSTracking.ts |
waypoints.length dep causes unnecessary re-renders |
| 7 | 🟡 | useVoiceCommands.ts |
'error' state typed but never set; errorMessage is dead code |
| 8 | 🟡 | useVoiceCommands.ts |
navigate_to prefix stripping is fragile |
| 9 | 🟡 | VoiceCommandScreen.tsx |
Wrong SafeAreaView import |
| 10 | 🔵 | VoiceCommandPanel.tsx |
micIcon type cast bypasses type safety |
| 11 | 🔵 | useGPSTracking.ts |
Waypoints lost on app restart |
| 12 | 🔵 | useGPSTracking.ts |
Permission request race condition on mount |
| 13 | 🔵 | app.config |
Missing microphone permission declarations |
✅ Actions performed
Full review triggered.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@copilot CodeRabbit has provided review feedback on this PR. Please address the comments and suggestions. Once you've made the necessary changes, I'll ask CodeRabbit to re-review. |
All CodeRabbit issues have been addressed in commit
Also added a TTS announcement ( |
|
@copilot CodeRabbit has reviewed this PR and provided feedback. Please address the review comments and push updates. Once you've made the changes, the monitor will ask CodeRabbit to re-review. |
Fixed in commit
All 10 voice feature files now pass |
There was a problem hiding this comment.
Pull request overview
This PR implements a "Voice Commands for Hands-Free Trail Navigation" MVP feature for the PackRat Expo app. The implementation provides TTS feedback via expo-speech and GPS tracking via expo-location, with a UI for voice command control. The actual speech-to-text (STT) recognition backend is intentionally deferred — a processTranscript function serves as the integration point for future Vosk/Picovoice/Web Speech API backends.
Changes:
- New
features/voice/module with GPS tracking hook, speech hook, voice command orchestration hook, screen, and UI components - New route at
app/(app)/voice-commands/with Stack.Screen registration in_layout.tsx - Feature flag
enableVoiceCommands: trueadded toconfig.ts; dashboard tile wired into home screen - i18n types and
en.jsonlocale updated with 19 newvoice.*translation keys
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
apps/expo/features/voice/hooks/useGPSTracking.ts |
New hook for GPS position tracking, waypoint management, and distance calculation using expo-location |
apps/expo/features/voice/hooks/useSpeech.ts |
New hook wrapping expo-speech for TTS feedback |
apps/expo/features/voice/hooks/useVoiceCommands.ts |
New orchestration hook — wires STT transcript → command dispatch → GPS → TTS |
apps/expo/features/voice/hooks/index.ts |
Barrel export for all voice hooks |
apps/expo/features/voice/screens/VoiceCommandScreen.tsx |
New screen component composing the voice control panel with header and offline badge |
apps/expo/features/voice/components/VoiceCommandPanel.tsx |
Main UI panel — microphone button, status, transcript display, command reference list |
apps/expo/features/voice/components/VoiceCommandsTile.tsx |
Home dashboard tile navigating to the voice commands screen |
apps/expo/features/voice/types.ts |
TypeScript types for voice commands, GPS position, waypoints, and state |
apps/expo/features/voice/index.ts |
Module barrel export |
apps/expo/app/(app)/voice-commands/index.tsx |
Expo Router route file for the voice commands screen |
apps/expo/app/(app)/_layout.tsx |
Registers the new voice-commands/index Stack.Screen |
apps/expo/app/(app)/(tabs)/(home)/index.tsx |
Adds VoiceCommandsTile to the dashboard tile registry and layout |
apps/expo/config.ts |
Adds enableVoiceCommands: true feature flag |
apps/expo/package.json |
Adds expo-speech ~13.1.0 dependency |
apps/expo/lib/i18n/types.ts |
Adds 19 voice.* translation key types |
apps/expo/lib/i18n/locales/en.json |
Adds "voice" translation object (critical bug: missing root closing }) |
You can also share your feedback on Copilot code review. Take the survey.
| "tapCommandToTest": "Tap a command to test it", | ||
| "testCommand": "Test command" | ||
| } |
There was a problem hiding this comment.
The root JSON object in en.json is missing its closing }. The diff removes the root-level closing brace and adds the new "voice" section, but does not add a new closing } after the "voice" object's close on line 921. The file ends with } (closing the voice sub-object) followed by a blank line, leaving the root { opened on line 1 without a matching close. This makes en.json invalid JSON, which will cause i18n to fail entirely at runtime. A bare } needs to be added as the final line of the file.
|
|
||
| // Auto-timeout after 10 seconds if no transcript arrives | ||
| listeningTimeoutRef.current = setTimeout(() => { | ||
| setListeningState('idle'); |
There was a problem hiding this comment.
The startListening function overwrites listeningTimeoutRef.current without first clearing the previous timeout. If startListening is called a second time while a previous 10-second timeout is still pending (e.g., after an error state when the user tries again quickly), the original timer becomes orphaned and will fire later — calling setListeningState('idle') and announcing "Listening timed out. Please try again." unexpectedly. The existing timeout should be cleared before scheduling a new one.
| const processTranscript = useCallback( | ||
| async (transcript: string) => { | ||
| setLastTranscript(transcript); | ||
| setListeningState('processing'); |
There was a problem hiding this comment.
When a speech-to-text backend calls processTranscript while the 10-second auto-timeout is still pending (i.e., a transcript arrives before the button is released), the timeout is never cleared. Ten seconds after startListening was called, the timeout will fire and call setListeningState('idle') plus speak "Listening timed out. Please try again." — even though the command was already processed successfully. processTranscript should cancel the pending timeout at the top of the function, similarly to how stopListening clears it.
| name: 'where_am_i', | ||
| patterns: ['where am i', "what's my location", 'my location', 'current location'], |
There was a problem hiding this comment.
The 'my location' pattern in the where_am_i command creates a false-match ambiguity. Because matchCommand iterates through VOICE_COMMANDS in order and where_am_i is defined before navigate_to, a transcript like "navigate to my location" will match where_am_i (via the 'my location' substring) instead of the intended navigate_to command. The pattern 'my location' is a substring of "navigate to my location", causing incorrect command dispatch. Consider removing 'my location' as a standalone pattern since it is already covered by "what's my location", or move more specific/compound navigation patterns above where_am_i in the array.
| errorMessage: string | null; | ||
| } |
There was a problem hiding this comment.
The VoiceCommandsState interface still contains errorMessage: string | null, which is dead code. The PR description says issue #7 was addressed by removing this dead errorMessage state, but it remains in the type definition. Neither useVoiceCommands nor any component populates this field — VoiceCommandsState is not even used as an actual type in the codebase. The errorMessage field should be removed to keep the interface consistent with the hook's actual shape.
| @@ -0,0 +1,47 @@ | |||
| import { ListItem } from '@packrat/ui/nativewindui'; | |||
There was a problem hiding this comment.
The Text component is imported from @packrat/ui/nativewindui but is never used in this file. The component only uses ListItem. This unused import should be removed.
|
@copilot Please address the CodeRabbit review feedback on this PR (6 inline comments). |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
8e85dc2 to
5d92080
Compare
Coverage Report for API Unit Tests Coverage (./packages/api)
File CoverageNo changed files found. |
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
65d00d1 to
705329e
Compare
- Add expo-speech dependency for on-device TTS - Create features/voice module with types, hooks, components, and screen - useSpeech: TTS hook using expo-speech (offline, no internet needed) - useGPSTracking: GPS tracking, waypoint management, distance calculation - useVoiceCommands: orchestrates TTS + GPS + pattern-matching command processor - VoiceCommandsTile: dashboard tile - VoiceCommandPanel: microphone button, status, command reference UI - VoiceCommandScreen: full screen with offline badge - Add enableVoiceCommands feature flag - Register voice commands route and screen in app layout - Add translation keys (en.json + TypeScript types) - Add VoiceCommandsTile to home dashboard Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
- #2: Destructure GPS values in useVoiceCommands to stabilise useCallback deps - #3: Guard startTracking against double subscription; add try/catch with warn log - #4: Cancel listeningTimeoutRef on hook unmount - #5: Fix stale closure in stopListening with functional setState - #6: Use waypointCountRef instead of waypoints.length dep in markWaypoint - #7: Wire 'error' state into all processTranscript failure paths - #8: Replace fragile regex replace with extractNavigationTarget() (indexOf, longest-first) - #9: Use SafeAreaView from react-native-safe-area-context in VoiceCommandScreen - #10: Type STATE_ICONS as Record<VoiceListeningState, MaterialIconName> - #12: Add permissionRequestInFlightRef to prevent concurrent permission races - Announce timeout via TTS in startListening - Document startTracking idempotent guard comment Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
- Fix line-length formatting in useVoiceCommands.ts (navigate_to case) - Remove unused Text import from VoiceCommandsTile.tsx - Fix import order in VoiceCommandScreen.tsx (react-native before react-native-safe-area-context) Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
- Fix invalid JSON in en.json (add missing root closing brace) - Clear pending timeout before scheduling new one in startListening - Cancel pending timeout at start of processTranscript - Remove ambiguous 'my location' pattern from where_am_i command - Remove dead errorMessage field from VoiceCommandsState interface Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
- startListening() now requests mic permission and starts an expo-av recording session instead of being a no-op - stopListening() stops & unloads the recording and exposes the URI for future STT backend integration - Added TODO comment to wire the recorded URI to a real STT backend (e.g., Whisper) - Guard against double-recording with recordingRef check - Clean up active recording on unmount via useEffect - Added expo-av to package.json dependencies
- Clear pending timeout before starting new listen session - Clear timeout when transcript processed before auto-timeout fires - Remove ambiguous 'my location' pattern from where_am_i command - Fix malformed JSON in en.json voice section - Add tests covering timer cleanup and command disambiguation
- Convert apps/expo deps (ai, axios, radash, react, react-dom, tailwindcss, typescript, zod) to catalog: references - Add comment-stripping to no-circular-deps.ts to prevent JSDoc false positives - Create packs/input.ts and api/types/validation.ts to break circular chains - Fix import paths in catalog/types.ts, profile/types.ts, itemCalculations.ts - Fix etl.ts ↔ schema.ts circular dependency via validation.ts extraction - Replace @ts-ignore with @ts-expect-error in useCreatePackFromPack.ts - Apply biome formatter to PackCard.tsx and sort root package.json
705329e to
c16ef51
Compare
|
Closing as a stale POC — this hasn't been touched since early March and has no CI runs. Voice commands are a valuable direction, but we'd want to start fresh with current main rather than rebase 70 days of drift. If anyone wants to pick this back up, the diff is preserved in the branch. |
en.json(missing root closing brace)startListeningprocessTranscript'my location'pattern fromwhere_am_ito prevent false match on "navigate to my location"errorMessagefield fromVoiceCommandsStateinterfaceOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.