Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-device Bluetooth PTT/media-button monitoring with watchdogs and read-polling fallbacks, wires microphone routing/state into the LiveKit store (isMicrophoneEnabled), adds Android InCallAudioModule routing, hardens CallKeep mute-event handling, adds system-audio UI/translations, and updates tests, logging, and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Device as Bluetooth Device
participant BluetoothSvc as BluetoothAudioService
participant Watchdog as Watchdog
participant Polling as ReadPolling
participant ButtonHandler as ButtonHandler
participant LiveKitStore as LiveKit Store
participant FeatureStore as Feature Store
participant LegacyStore as Legacy LiveKit Store
participant InCall as InCallAudioModule
Device->>BluetoothSvc: connect
BluetoothSvc->>Watchdog: startMonitoringWatchdog()
BluetoothSvc->>Polling: startReadPollingFallback()
BluetoothSvc->>BluetoothSvc: startNotificationsForButtonControls()
Device->>BluetoothSvc: characteristic update / media button event
alt notify available
BluetoothSvc->>ButtonHandler: handleCharacteristicValueUpdate(...)
ButtonHandler->>BluetoothSvc: handleButtonEventFromCharacteristic(...)
else fallback polling
Polling->>BluetoothSvc: pollReadCharacteristics()
BluetoothSvc->>ButtonHandler: handleButtonEventFromCharacteristic(...)
end
ButtonHandler->>LiveKitStore: requestMicrophoneState(enabled)
alt feature store connected
LiveKitStore->>FeatureStore: setMicrophoneEnabled(enabled)
FeatureStore-->>LiveKitStore: success
else feature store unavailable
LiveKitStore->>LegacyStore: setMicrophoneEnabled(enabled)
LegacyStore-->>LiveKitStore: success
end
LiveKitStore->>BluetoothSvc: applyMicrophoneEnabled(state)
LiveKitStore->>InCall: setAudioRoute(normalizedDeviceType)
BluetoothSvc->>BluetoothSvc: clear retries / reset PTT flags
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx (1)
385-387:⚠️ Potential issue | 🟡 MinorInner
beforeEachwipesconnectToDevicemock set by the outerbeforeEachThe outer
beforeEach(line 229) setsconnectToDevice.mockResolvedValue(undefined). The innerbeforeEachat line 386 callsjest.clearAllMocks(), which erases that resolved-value mock. The "Device Selection Flow" tests then run withconnectToDevicereturning a plainjest.fn()that returnsundefinedsynchronously, not a resolved Promise. The tests happen to pass becauseawait undefinedresolves immediately in JavaScript, but this is fragile — any component-side guard likePromise.resolve()around the call or stricter typing could surface the gap.🔧 Proposed fix — re-set the mock in the inner `beforeEach`
describe('Device Selection Flow', () => { beforeEach(() => { jest.clearAllMocks(); + (bluetoothAudioService.connectToDevice as jest.Mock).mockResolvedValue(undefined); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx` around lines 385 - 387, The inner beforeEach is calling jest.clearAllMocks(), which removes the outer beforeEach setup connectToDevice.mockResolvedValue(undefined); update the inner beforeEach (the one that calls jest.clearAllMocks()) to re-apply the async mock by calling connectToDevice.mockResolvedValue(undefined) after clearing mocks so connectToDevice remains a Promise-returning mock for the "Device Selection Flow" tests; reference the connectToDevice mock and the inner beforeEach where jest.clearAllMocks() is invoked and reset the mock there.src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
513-560:⚠️ Potential issue | 🟡 MinorDuplicate nested
describe('Error Handling')with a duplicated test caseLines 514–548 contain an inner
describe('Error Handling', ...)that is nested directly inside the outerdescribe('Error Handling', ...)at line 513. The inner block's second test ("should handle basic error state management") is duplicated verbatim at lines 549–559 outside the inner block. This creates two identical test runs and misleading nesting in the test report.🔧 Proposed fix — remove the extra nesting and duplicate test
describe('Error Handling', () => { - describe('Error Handling', () => { it('should handle room initialization errors', async () => { ... }); it('should handle basic error state management', async () => { ... }); - }); - it('should handle basic error state management', async () => { - ... - }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts` around lines 513 - 560, Remove the duplicated nested describe('Error Handling') block and the duplicate "should handle basic error state management" test: keep a single outer describe('Error Handling') with the room-initialization error test and one "should handle basic error state management" test that exercises result.current.actions._clearError via renderHook(() => useLiveKitCallStore()); delete the inner describe and the repeated test so the tests run only once and the nesting is correct.src/features/livekit-call/store/useLiveKitCallStore.ts (2)
63-71:⚠️ Potential issue | 🟡 Minor
ensurePttInputMonitoringis called before the early-return guard.If the user is already connecting or connected, the function still invokes
ensurePttInputMonitoringbefore bailing out on line 70. Move it after the guard so it only runs on valid connection attempts:Proposed fix
connectToRoom: async (roomId, participantIdentity) => { - bluetoothAudioService.ensurePttInputMonitoring('useLiveKitCallStore connectToRoom start'); - if (get().isConnecting || get().isConnected) { logger.warn({ message: 'Connection attempt while already connecting or connected', context: { roomId, participantIdentity, isConnecting: get().isConnecting, isConnected: get().isConnected }, }); return; } + bluetoothAudioService.ensurePttInputMonitoring('useLiveKitCallStore connectToRoom start'); + set({ isConnecting: true, error: null, selectedRoomForJoining: roomId });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/livekit-call/store/useLiveKitCallStore.ts` around lines 63 - 71, The call to bluetoothAudioService.ensurePttInputMonitoring is executed before the early-return guard, causing it to run even when a connection should be skipped; move the ensurePttInputMonitoring(...) invocation to after the guard that checks get().isConnecting and get().isConnected (the same block where logger.warn is used) so it only runs on valid connection attempts inside the connectToRoom flow in useLiveKitCallStore.
229-230:⚠️ Potential issue | 🔴 CriticalRoom connection is commented out —
newRoom.connect(...)is never called, breaking the connection flow.Line 229 shows
//await newRoom.connect(LIVEKIT_URL, token, connectOptions);commented out. While event listeners are registered on the room object (lines 93–223), they will never fire because the room is never actually connected to the LiveKit server. The comment at line 230 is misleading — theConnectionStateChangedlistener cannot trigger without an active connection. The room will remain disconnected,isConnectedwill never be set totrue, and the entire call functionality is broken. Uncomment theconnect()call to fix this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/livekit-call/store/useLiveKitCallStore.ts` around lines 229 - 230, Room never connects because newRoom.connect(...) is commented out; uncomment the call to establish the LiveKit connection. Restore the awaited call to newRoom.connect(LIVEKIT_URL, token, connectOptions) (or call newRoom.connect with the same variables) right after the event listeners are registered so the ConnectionStateChanged handlers can fire and isConnected can be set to true; ensure the call remains awaited and any thrown errors are handled or propagated consistent with surrounding error handling.src/components/settings/audio-device-selection.tsx (1)
46-48:⚠️ Potential issue | 🟡 MinorHardcoded English string
' Device'bypasses i18n.The default branch concatenates
+ ' Device'which won't be translated. Wrap this int()or add a generic translation key.Proposed fix
default: - return deviceType.charAt(0).toUpperCase() + deviceType.slice(1) + ' Device'; + return t('settings.audio_device_selection.generic_device', { type: deviceType.charAt(0).toUpperCase() + deviceType.slice(1) });As per coding guidelines, "Ensure all text is wrapped in
t()fromreact-i18nextfor translations with dictionary files stored insrc/translations".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/audio-device-selection.tsx` around lines 46 - 48, The default branch in the device label builder uses a hardcoded English suffix "' Device'" which bypasses i18n; update the code that builds the label (where deviceType is used) to use t() with a translation key (e.g., t('deviceTypeLabel', { type: deviceType })) or a keyed template like t('Device {{type}}', { type: deviceType }) instead of string concatenation, and add the corresponding entry to the translations dictionary in src/translations so all locales get the translated template.src/components/livekit/livekit-bottom-sheet.tsx (1)
199-205:⚠️ Potential issue | 🟡 MinorDevice names displayed without localization normalization.
Lines 200 and 204 render
selectedAudioDevices.microphone?.nameandselectedAudioDevices.speaker?.namedirectly. When the device is "system-audio," users will see the raw name instead of the localized label. ThegetDeviceDisplayNamehelper fromaudio-device-selection.tsx(or a shared utility) should be used here for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/livekit/livekit-bottom-sheet.tsx` around lines 199 - 205, The component currently renders raw device names via selectedAudioDevices.microphone?.name and selectedAudioDevices.speaker?.name; replace those with the normalized/localized display from the existing helper getDeviceDisplayName (e.g., getDeviceDisplayName(selectedAudioDevices.microphone) and getDeviceDisplayName(selectedAudioDevices.speaker)), import the helper from audio-device-selection.tsx (or the shared utility) if missing, and keep the t('common.unknown') fallback for null/undefined results.src/components/settings/bluetooth-device-selection-bottom-sheet.tsx (1)
247-259:⚠️ Potential issue | 🟡 MinorStoring a translated string as the device name creates a language-dependent value.
Line 258 saves
t('bluetooth.system_audio')as the preferred device'sname. If the user later switches the app language, the stored name will be stale in the previous locale. SincegetDeviceDisplayName(and the inline logic on line 40) already resolves the display name from the'system-audio'ID at render time, store a fixed English identifier instead:Proposed fix
- await setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }); + await setPreferredDevice({ id: 'system-audio', name: 'System Audio' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` around lines 247 - 259, The code stores a translated label as the preferred device name; replace the locale-dependent value passed to setPreferredDevice with a fixed English identifier instead of t('bluetooth.system_audio'). In the onPress flow around setConnectingDeviceId, connectToSystemAudio and setPreferredDevice, change the call setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }) to use a constant English name (e.g. name: 'System Audio' or name: 'system-audio') so the stored name is language-independent and getDeviceDisplayName/getDeviceDisplayName-style render logic continues to resolve the localized display string at render time.src/services/bluetooth-audio.service.ts (1)
2386-2388:⚠️ Potential issue | 🟡 MinorDuplicate
this.isInitialized = falseassignment.Line 2387 is an exact duplicate of line 2386.
Proposed fix
this.isInitialized = false; - this.isInitialized = false; this.hasAttemptedPreferredDeviceConnection = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2386 - 2388, There is a duplicate assignment to this.isInitialized (appears twice in the same initialization block); remove the redundant line so that only one this.isInitialized = false remains and preserve the subsequent this.hasAttemptedPreferredDeviceConnection = false initialization; update the initialization block (e.g., in the constructor or initializer where isInitialized and hasAttemptedPreferredDeviceConnection are set) to contain each assignment exactly once.
🧹 Nitpick comments (13)
src/components/status/__tests__/status-gps-integration.test.tsx (1)
68-71: Remove debugconsole.logfrom the mock implementation.This
console.logwill fire on everyuseLocationStoreinvocation across all tests in this file, producing noise in CI output.♻️ Proposed fix
mockUseLocationStore.mockImplementation(() => { - console.log('useLocationStore called, returning:', mockLocationStore); return mockLocationStore; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/status/__tests__/status-gps-integration.test.tsx` around lines 68 - 71, The mock implementation for mockUseLocationStore currently logs to console on every call; remove the debug console.log from the mockImplementation so it simply returns mockLocationStore (update the mockUseLocationStore.mockImplementation callback to only return mockLocationStore), ensuring no noisy output from useLocationStore calls in tests.src/services/callkeep.service.android.ts (2)
224-252:ignoreMuteEvents,removeMuteListener, andrestoreMuteListenerlackPlatform.OSguard
setup(),startCall(), andendCall()all short-circuit on non-Android platforms. These three newly added / updated public methods do not, creating an inconsistency: callers that check the platform before invoking other service methods may inadvertently trigger RNCallKeep on iOS through these paths.♻️ Proposed fix
ignoreMuteEvents(durationMs: number): void { + if (Platform.OS !== 'android') return; const now = Date.now(); ... } removeMuteListener(): void { + if (Platform.OS !== 'android') return; RNCallKeep.removeEventListener('didPerformSetMutedCallAction'); ... } restoreMuteListener(): void { + if (Platform.OS !== 'android') return; RNCallKeep.removeEventListener('didPerformSetMutedCallAction'); ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/callkeep.service.android.ts` around lines 224 - 252, These three public methods (ignoreMuteEvents, removeMuteListener, restoreMuteListener) call RNCallKeep and should short-circuit on non-Android platforms like other methods; wrap their bodies with a Platform.OS === 'android' guard (or early return when Platform.OS !== 'android') so RNCallKeep.removeEventListener/addEventListener and logger.debug only run on Android, keeping behavior consistent with setup/startCall/endCall and avoiding accidental RNCallKeep calls on iOS.
24-26: New timing fields are not reset incleanup()
lastMuteEventTime,muteEventStormEndTime, andignoreEventsUntilare initialised to0at declaration but are not zeroed duringcleanup(). If the service is ever re-setup after cleanup (e.g., reconnect scenario), a staleignoreEventsUntilvalue could silently block mute events until it naturally expires.♻️ Proposed fix
// Inside cleanup(), after removing event listeners: + this.lastMuteEventTime = 0; + this.muteEventStormEndTime = 0; + this.ignoreEventsUntil = 0; this.isSetup = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/callkeep.service.android.ts` around lines 24 - 26, The cleanup() method fails to reset timing fields lastMuteEventTime, muteEventStormEndTime, and ignoreEventsUntil, which can leave stale values blocking events after a re-setup; update the cleanup() implementation in the CallKeepService (the class that declares these fields) to explicitly set this.lastMuteEventTime = 0, this.muteEventStormEndTime = 0, and this.ignoreEventsUntil = 0 (or the class-equivalent names) along with any existing teardown logic so the service state is fully cleared for subsequent initializations.src/components/settings/bluetooth-device-item.tsx (1)
23-25: Magic string'system-audio'should be a shared constantThe device ID
'system-audio'is a sentinel value that now needs to be matched in at least two components (BluetoothDeviceItemandBluetoothDeviceSelectionBottomSheet) and likely wherever preferred devices are persisted. A typo in any one location would cause the translation lookup to silently fall through topreferredDevice.name.♻️ Proposed fix — define a constant in a shared location
// e.g., src/services/bluetooth-audio.service.ts or a new constants file export const SYSTEM_AUDIO_DEVICE_ID = 'system-audio';// bluetooth-device-item.tsx +import { SYSTEM_AUDIO_DEVICE_ID } from '@/services/bluetooth-audio.service'; ... -if (preferredDevice.id === 'system-audio') { +if (preferredDevice.id === SYSTEM_AUDIO_DEVICE_ID) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-item.tsx` around lines 23 - 25, Replace the hard-coded sentinel 'system-audio' string with a shared exported constant (e.g., SYSTEM_AUDIO_DEVICE_ID) and import it where needed so all comparisons use the same source of truth; update the equality check in BluetoothDeviceItem (the preferredDevice.id === 'system-audio' branch) to use SYSTEM_AUDIO_DEVICE_ID and do the same in BluetoothDeviceSelectionBottomSheet and any persistence logic that reads/writes preferred devices to avoid typos and silent fall-throughs.src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx (1)
300-307: Missing test coverage for the new system-audio device selection pathThis PR introduces the
'system-audio'virtual device (with special display-name logic inBluetoothDeviceItemand the bottom sheet). The test suite covers Bluetooth device connect/disconnect flows but has no test case for selecting the System Audio option. Consider adding a test that:
- Renders the sheet with a
'system-audio'preferred device- Verifies the translated label (
bluetooth.system_audio) is displayed instead of a raw device name- Verifies
setPreferredDeviceis called with{ id: 'system-audio', ... }on selection🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx` around lines 300 - 307, Add a test that covers the new 'system-audio' virtual device path: render BluetoothDeviceSelectionBottomSheet with mockProps where preferredDevice or devices includes an entry with id 'system-audio', assert the UI shows the translated label 'bluetooth.system_audio' (handled by BluetoothDeviceItem) instead of the raw name, simulate selecting that item and assert setPreferredDevice was called with an object containing id: 'system-audio' (and expected payload shape). Use the existing test helpers/props and screen/events utilities consistent with the other tests to locate BluetoothDeviceSelectionBottomSheet, BluetoothDeviceItem, and the mocked setPreferredDevice function.plugins/withInCallAudioModule.js (2)
113-113:AudioManager.modeis set toMODE_IN_COMMUNICATIONbut never restoredThe mode is unconditionally applied before the
whendispatch (line 113) and is not reverted by the "earpiece"/"default" branch or bycleanup(). After the VoIP session ends the AudioManager remains inMODE_IN_COMMUNICATION, which can distort media playback, ringtone output, and subsequent audio sessions.♻️ Proposed fix
+ val previousMode = audioManager.mode audioManager.mode = AudioManager.MODE_IN_COMMUNICATION when (normalizedRoute) { "bluetooth" -> { ... } "speaker" -> { ... } "earpiece", "default" -> { audioManager.stopBluetoothSco() audioManager.isBluetoothScoOn = false audioManager.isSpeakerphoneOn = false + audioManager.mode = previousMode } else -> { ... } }Alternatively, restore
AudioManager.MODE_NORMALincleanup()if a single reset point is preferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/withInCallAudioModule.js` at line 113, AudioManager.mode is being set to AudioManager.MODE_IN_COMMUNICATION unconditionally (see the assignment to audioManager.mode and the when dispatch) and is never restored, leaving the device stuck in in-communication mode; update the control flow so that after the when(...) dispatch resolves you restore audioManager.mode back to its previous value (or explicitly to AudioManager.MODE_NORMAL) in both the "earpiece"/"default" branch and in cleanup(), ensuring any early returns or errors also trigger the restore; reference the audioManager.mode assignment, the when(...) handler, the "earpiece"/"default" branch, and the cleanup() function to implement the restore logic.
103-104: Indentation inconsistency in generated KotlinThe new
@ReactMethodannotation andfun setAudioRoutedeclaration use 6-space indentation while the rest of the class methods (e.g.,@ReactMethod fun playSound) use 4-space indentation. The generatedInCallAudioModule.ktwill have inconsistent formatting.♻️ Proposed fix
- `@ReactMethod` - fun setAudioRoute(route: String, promise: Promise) { + `@ReactMethod` + fun setAudioRoute(route: String, promise: Promise) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/withInCallAudioModule.js` around lines 103 - 104, The generated Kotlin snippet for the setAudioRoute method has inconsistent indentation: adjust the template in withInCallAudioModule.js so the lines containing the `@ReactMethod` annotation and the fun setAudioRoute(route: String, promise: Promise) declaration use 4-space indentation to match other methods (e.g., `@ReactMethod` fun playSound) and ensure the resulting InCallAudioModule.kt formatting is consistent.src/components/settings/audio-device-selection.tsx (1)
138-144: Anonymous arrow functions in.map()callbacks create new references each render.Lines 139 and 157 use
() => setSelectedMicrophone(device)and() => setSelectedSpeaker(device)inline. Per coding guidelines, avoid anonymous functions in render-item or event handlers to prevent unnecessary re-renders. SincerenderDeviceItemalready receivesonSelect, the concern is the anonymous closure passed to it frommap. This is a minor optimization opportunity—could memoize or use a stable callback pattern.Also applies to: 156-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/audio-device-selection.tsx` around lines 138 - 144, The inline anonymous callbacks passed to renderDeviceItem from availableMicrophones.map and availableSpeakers.map (currently using () => setSelectedMicrophone(device) and () => setSelectedSpeaker(device)) create new function references each render; replace them with stable callbacks: create a memoized handler factory (e.g., a useCallback that returns a handler given a device id or a reuseable onSelect handler that accepts the device as an argument) and pass that stable function to renderDeviceItem, or alternatively pass a device id to renderDeviceItem and let renderDeviceItem call a single memoized select function (referencing setSelectedMicrophone, setSelectedSpeaker and renderDeviceItem) so the onSelect prop remains referentially stable across renders.src/components/livekit/livekit-bottom-sheet.tsx (1)
107-108: Replaceconsole.logwithlogger.Line 108 uses
console.log('Updating audio routing for:', ...)— the rest of the codebase uses the structuredloggerfrom@/lib/logging. Uselogger.debug(...)here for consistency.Proposed fix
- console.log('Updating audio routing for:', speaker.type); + logger.debug({ message: 'Updating audio routing', context: { speakerType: speaker.type } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/livekit/livekit-bottom-sheet.tsx` around lines 107 - 108, Replace the console.log call that prints the speaker type with the project's structured logger: change console.log('Updating audio routing for:', speaker.type) to logger.debug(`Updating audio routing for: ${speaker.type}`) (or logger.debug('Updating audio routing for:', { type: speaker.type }) if you prefer structured args), and ensure the logger from '@/lib/logging' is imported at the top of the module if it's not already; update the snippet near selectedAudioDevices.speaker handling in the livekit-bottom-sheet component.src/services/bluetooth-audio.service.ts (3)
2265-2272: Identical cleanup sequence repeated in four places — extract a shared helper.The same pattern (reset
pttPressActive, clear fallback/retry timeouts, nullpendingMicEnabled, stop watchdog/polling/media-button monitoring) is duplicated inrevertLiveKitAudioRouting,disconnectDevice,destroy, andreset. A singleprivate resetMonitoringState()method would reduce maintenance risk.Also applies to: 2283-2290, 2366-2372, 2418-2424
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2265 - 2272, Extract the repeated cleanup block into a single private method (e.g. private resetMonitoringState()) that performs bluetoothStore.setIsHeadsetButtonMonitoring(false), this.pttPressActive = false, this.clearPttReleaseFallback(), this.clearMicApplyRetry(), this.pendingMicEnabled = null, this.stopMonitoringWatchdog(), this.stopReadPollingFallback(), and this.stopMediaButtonFallbackMonitoring(); then replace the duplicated sequences in revertLiveKitAudioRouting, disconnectDevice, destroy, and reset with a call to this.resetMonitoringState() to centralize maintenance and avoid duplication.
1210-1228: Watchdog interval only self-terminates; it never restarts or repairs monitoring.The 4-second interval checks if the device is still connected and monitoring is active, but when
monitoringStartedAtis set (line 1225-1227), it simply returns — no corrective action is taken. If the intent is to detect and recover from stalled BLE subscriptions, the watchdog needs a restart path. If this is purely for cleanup, a more descriptive name (e.g.,startMonitoringCleanupTimer) would clarify the intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1210 - 1228, The watchdog started by startMonitoringWatchdog currently only stops itself and never attempts to repair or restart stalled monitoring when monitoringStartedAt is set; update startMonitoringWatchdog to detect a stale subscription (e.g., compare Date.now() - this.monitoringStartedAt to a threshold) and trigger a recovery path: call a recovery helper (create or reuse a method like restartMonitoring or reinitializeHeadsetButtonMonitoring) that re-subscribes to BLE notifications, and ensure monitoringWatchdogInterval is cleared and restarted around that flow; keep stopMonitoringWatchdog for cleanup and use useBluetoothAudioStore.getState().isHeadsetButtonMonitoring to gate recovery attempts.
1408-1419: Read-polling every 700 ms across all characteristics of a specialized device is aggressive.Lines 1408-1419 register every characteristic on a specialized device for read polling, and the interval at line 1463 fires every 700 ms. For devices with many characteristics this can saturate the BLE link, drain battery, and increase the chance of read timeouts interfering with notification delivery. Consider limiting the polled set to characteristics that actually lack notification support, or increasing the polling interval for non-button characteristics.
Also applies to: 1450-1463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1408 - 1419, The current loop registers every characteristic for 700ms read-polling which can overload BLE; change the logic in the block that iterates peripheralInfo.characteristics so registerReadPollingCharacteristic is only called for characteristics that do not support notifications/indications (check characteristic.properties for 'notify' or 'indicate' or an explicit characteristic.notify flag), and further restrict to a smaller set (e.g., only known button UUIDs or a configurable whitelist) or tag non-button characteristics to use a longer polling interval; also introduce or use a configurable polling interval variable (instead of the hardcoded 700ms) and enforce a maximum number of concurrently polled characteristics to avoid saturating the link.src/stores/app/livekit-store.ts (1)
32-48: Use the existing top-levelreact-nativeimport instead of inlinerequire.
react-nativeis already imported at line 7. AddNativeModulesto that import rather than using a dynamicrequireinside the function body.Proposed fix
At the top of the file (line 7):
-import { Alert, Linking, PermissionsAndroid, Platform } from 'react-native'; +import { Alert, Linking, NativeModules, PermissionsAndroid, Platform } from 'react-native';Then at lines 32–33:
- const { NativeModules } = require('react-native'); - const inCallAudioModule = NativeModules?.InCallAudioModule; + const inCallAudioModule = NativeModules?.InCallAudioModule;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/app/livekit-store.ts` around lines 32 - 48, Replace the inline dynamic require for NativeModules with the existing top-level react-native import: add NativeModules to the existing import at the top of the file and remove the inline "const { NativeModules } = require('react-native');" line; leave the rest of the logic using inCallAudioModule (NativeModules?.InCallAudioModule), setAudioRoute, normalizedDeviceType and logger unchanged so the try/catch and fallback to Audio.setAudioModeAsync still work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 112: The package "babel-plugin-transform-import-meta" is listed under
dependencies but it's a compile-time Babel transform and should be a dev-only
dependency; remove the "babel-plugin-transform-import-meta" entry from the
dependencies section of package.json and add the same package (same version
"^2.3.3") under devDependencies, then reinstall (npm install / yarn install) to
update lockfile so production installs won't include this plugin.
- Around line 194-195: Update the `@typescript-eslint` packages in package.json by
bumping "@typescript-eslint/eslint-plugin" and "@typescript-eslint/parser" from
"~7.18.0" to a v8 release (e.g. "^8.x") and then restore TypeScript to a
compatible 5.8 range (e.g. "typescript": "5.8.x") so the parser/plugin support
newer TS syntax; also ensure the existing "eslint" version (~8.57.0) satisfies
v8's peer requirement and run install + lint/test to confirm nothing else
breaks.
In `@plugins/withInCallAudioModule.js`:
- Around line 116-122: The "bluetooth" branch sets audioManager.isBluetoothScoOn
= true and disables speakerphone but may never call
audioManager.startBluetoothSco(), causing a false-success resolution; update the
handler in withInCallAudioModule.js so that when selecting "bluetooth" you call
audioManager.startBluetoothSco() unconditionally (or, if you must respect
audioManager.isBluetoothScoAvailableOffCall, reject the promise when that flag
is false) and only resolve the promise after confirming startBluetoothSco() was
invoked/succeeded; reference audioManager, isSpeakerphoneOn, isBluetoothScoOn,
isBluetoothScoAvailableOffCall, and startBluetoothSco to locate and modify the
logic and ensure the promise correctly reflects actual SCO start state.
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Around line 57-62: The effect that calls startScan when the sheet opens can
enter an infinite retry loop because startScan resets hasScanned to false on
errors; update the error handling so that a caught error does NOT set hasScanned
back to false (or alternatively add a bounded retry counter like scanRetryCount
that increments on each failure and prevents further automatic retries after N
attempts), keeping the existing useEffect, isOpen, hasScanned, and startScan
symbols intact and ensuring the user still sees the alert but must manually
trigger subsequent scans.
In `@src/services/__tests__/bluetooth-audio.service.test.ts`:
- Around line 226-245: The featureStore.getState mock is using mockReturnValue
which persists across tests; change the test that sets featureStore.getState to
use mockReturnValueOnce (or restore the mock in an afterEach/afterAll) so the
override doesn't leak — update the test's call to
featureStore.getState.mockReturnValue(...) to
featureStore.getState.mockReturnValueOnce({...}) (or add an afterEach that calls
featureStore.getState.mockReset()/mockRestore()) to ensure
bluetoothAudioService.setMicrophoneEnabled and subsequent tests see the default
behavior.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1519-1526: The final clause normalizedValue.length > 0 always
evaluates true and makes the "read" capability check a no-op; remove that clause
from the return expression so explicit falsy values like { Read: false } are
rejected. Update the return in the method(s) (e.g., hasReadCapability and
hasNotificationOrReadCapability) to only check value === true || value === 1 ||
normalizedValue === normalizedKey || normalizedValue === 'true' ||
normalizedValue === '1', and apply the same change at the other occurrence noted
in hasNotificationOrReadCapability.
- Around line 1128-1129: The event listener array is growing with stale
references because startMediaButtonFallbackMonitoring pushes
this.mediaButtonEventListener into this.eventListeners but
stopMediaButtonFallbackMonitoring only calls .remove() on the listener and does
not remove the reference from this.eventListeners; update
stopMediaButtonFallbackMonitoring to also remove the listener reference from
this.eventListeners (e.g., find the index of this.mediaButtonEventListener and
splice it out or filter the array to exclude it) and optionally guard
startMediaButtonFallbackMonitoring against pushing duplicate references so the
array reflects only active listeners.
- Around line 208-212: The characteristic update is being subscribed twice in
setupEventListeners(): keep the modern
BleManager.onDidUpdateValueForCharacteristic subscription (valueUpdateListener)
and remove the legacy DeviceEventEmitter.addListener subscription
(legacyValueUpdateListener) along with its push to this.eventListeners so
handleCharacteristicValueUpdate is only registered once; locate the
legacyValueUpdateListener variable and its this.eventListeners.push call and
delete them.
In `@src/services/callkeep.service.android.ts`:
- Around line 316-332: The cooldown branch unconditionally extends
muteEventStormEndTime causing the window to grow; in the second if (now <
this.muteEventStormEndTime) block remove the update that sets
this.muteEventStormEndTime = now + 800 so the original storm deadline can expire
naturally — keep the logger.debug call and return as-is, but do not modify
muteEventStormEndTime when handling cooldown events (use the initial assignment
only in the first storm-detection branch that checks timeSinceLastEvent < 500).
---
Outside diff comments:
In `@src/components/livekit/livekit-bottom-sheet.tsx`:
- Around line 199-205: The component currently renders raw device names via
selectedAudioDevices.microphone?.name and selectedAudioDevices.speaker?.name;
replace those with the normalized/localized display from the existing helper
getDeviceDisplayName (e.g.,
getDeviceDisplayName(selectedAudioDevices.microphone) and
getDeviceDisplayName(selectedAudioDevices.speaker)), import the helper from
audio-device-selection.tsx (or the shared utility) if missing, and keep the
t('common.unknown') fallback for null/undefined results.
In
`@src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx`:
- Around line 385-387: The inner beforeEach is calling jest.clearAllMocks(),
which removes the outer beforeEach setup
connectToDevice.mockResolvedValue(undefined); update the inner beforeEach (the
one that calls jest.clearAllMocks()) to re-apply the async mock by calling
connectToDevice.mockResolvedValue(undefined) after clearing mocks so
connectToDevice remains a Promise-returning mock for the "Device Selection Flow"
tests; reference the connectToDevice mock and the inner beforeEach where
jest.clearAllMocks() is invoked and reset the mock there.
In `@src/components/settings/audio-device-selection.tsx`:
- Around line 46-48: The default branch in the device label builder uses a
hardcoded English suffix "' Device'" which bypasses i18n; update the code that
builds the label (where deviceType is used) to use t() with a translation key
(e.g., t('deviceTypeLabel', { type: deviceType })) or a keyed template like
t('Device {{type}}', { type: deviceType }) instead of string concatenation, and
add the corresponding entry to the translations dictionary in src/translations
so all locales get the translated template.
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Around line 247-259: The code stores a translated label as the preferred
device name; replace the locale-dependent value passed to setPreferredDevice
with a fixed English identifier instead of t('bluetooth.system_audio'). In the
onPress flow around setConnectingDeviceId, connectToSystemAudio and
setPreferredDevice, change the call setPreferredDevice({ id: 'system-audio',
name: t('bluetooth.system_audio') }) to use a constant English name (e.g. name:
'System Audio' or name: 'system-audio') so the stored name is
language-independent and getDeviceDisplayName/getDeviceDisplayName-style render
logic continues to resolve the localized display string at render time.
In `@src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts`:
- Around line 513-560: Remove the duplicated nested describe('Error Handling')
block and the duplicate "should handle basic error state management" test: keep
a single outer describe('Error Handling') with the room-initialization error
test and one "should handle basic error state management" test that exercises
result.current.actions._clearError via renderHook(() => useLiveKitCallStore());
delete the inner describe and the repeated test so the tests run only once and
the nesting is correct.
In `@src/features/livekit-call/store/useLiveKitCallStore.ts`:
- Around line 63-71: The call to bluetoothAudioService.ensurePttInputMonitoring
is executed before the early-return guard, causing it to run even when a
connection should be skipped; move the ensurePttInputMonitoring(...) invocation
to after the guard that checks get().isConnecting and get().isConnected (the
same block where logger.warn is used) so it only runs on valid connection
attempts inside the connectToRoom flow in useLiveKitCallStore.
- Around line 229-230: Room never connects because newRoom.connect(...) is
commented out; uncomment the call to establish the LiveKit connection. Restore
the awaited call to newRoom.connect(LIVEKIT_URL, token, connectOptions) (or call
newRoom.connect with the same variables) right after the event listeners are
registered so the ConnectionStateChanged handlers can fire and isConnected can
be set to true; ensure the call remains awaited and any thrown errors are
handled or propagated consistent with surrounding error handling.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2386-2388: There is a duplicate assignment to this.isInitialized
(appears twice in the same initialization block); remove the redundant line so
that only one this.isInitialized = false remains and preserve the subsequent
this.hasAttemptedPreferredDeviceConnection = false initialization; update the
initialization block (e.g., in the constructor or initializer where
isInitialized and hasAttemptedPreferredDeviceConnection are set) to contain each
assignment exactly once.
---
Nitpick comments:
In `@plugins/withInCallAudioModule.js`:
- Line 113: AudioManager.mode is being set to AudioManager.MODE_IN_COMMUNICATION
unconditionally (see the assignment to audioManager.mode and the when dispatch)
and is never restored, leaving the device stuck in in-communication mode; update
the control flow so that after the when(...) dispatch resolves you restore
audioManager.mode back to its previous value (or explicitly to
AudioManager.MODE_NORMAL) in both the "earpiece"/"default" branch and in
cleanup(), ensuring any early returns or errors also trigger the restore;
reference the audioManager.mode assignment, the when(...) handler, the
"earpiece"/"default" branch, and the cleanup() function to implement the restore
logic.
- Around line 103-104: The generated Kotlin snippet for the setAudioRoute method
has inconsistent indentation: adjust the template in withInCallAudioModule.js so
the lines containing the `@ReactMethod` annotation and the fun
setAudioRoute(route: String, promise: Promise) declaration use 4-space
indentation to match other methods (e.g., `@ReactMethod` fun playSound) and ensure
the resulting InCallAudioModule.kt formatting is consistent.
In `@src/components/livekit/livekit-bottom-sheet.tsx`:
- Around line 107-108: Replace the console.log call that prints the speaker type
with the project's structured logger: change console.log('Updating audio routing
for:', speaker.type) to logger.debug(`Updating audio routing for:
${speaker.type}`) (or logger.debug('Updating audio routing for:', { type:
speaker.type }) if you prefer structured args), and ensure the logger from
'@/lib/logging' is imported at the top of the module if it's not already; update
the snippet near selectedAudioDevices.speaker handling in the
livekit-bottom-sheet component.
In
`@src/components/settings/__tests__/bluetooth-device-selection-bottom-sheet.test.tsx`:
- Around line 300-307: Add a test that covers the new 'system-audio' virtual
device path: render BluetoothDeviceSelectionBottomSheet with mockProps where
preferredDevice or devices includes an entry with id 'system-audio', assert the
UI shows the translated label 'bluetooth.system_audio' (handled by
BluetoothDeviceItem) instead of the raw name, simulate selecting that item and
assert setPreferredDevice was called with an object containing id:
'system-audio' (and expected payload shape). Use the existing test helpers/props
and screen/events utilities consistent with the other tests to locate
BluetoothDeviceSelectionBottomSheet, BluetoothDeviceItem, and the mocked
setPreferredDevice function.
In `@src/components/settings/audio-device-selection.tsx`:
- Around line 138-144: The inline anonymous callbacks passed to renderDeviceItem
from availableMicrophones.map and availableSpeakers.map (currently using () =>
setSelectedMicrophone(device) and () => setSelectedSpeaker(device)) create new
function references each render; replace them with stable callbacks: create a
memoized handler factory (e.g., a useCallback that returns a handler given a
device id or a reuseable onSelect handler that accepts the device as an
argument) and pass that stable function to renderDeviceItem, or alternatively
pass a device id to renderDeviceItem and let renderDeviceItem call a single
memoized select function (referencing setSelectedMicrophone, setSelectedSpeaker
and renderDeviceItem) so the onSelect prop remains referentially stable across
renders.
In `@src/components/settings/bluetooth-device-item.tsx`:
- Around line 23-25: Replace the hard-coded sentinel 'system-audio' string with
a shared exported constant (e.g., SYSTEM_AUDIO_DEVICE_ID) and import it where
needed so all comparisons use the same source of truth; update the equality
check in BluetoothDeviceItem (the preferredDevice.id === 'system-audio' branch)
to use SYSTEM_AUDIO_DEVICE_ID and do the same in
BluetoothDeviceSelectionBottomSheet and any persistence logic that reads/writes
preferred devices to avoid typos and silent fall-throughs.
In `@src/components/status/__tests__/status-gps-integration.test.tsx`:
- Around line 68-71: The mock implementation for mockUseLocationStore currently
logs to console on every call; remove the debug console.log from the
mockImplementation so it simply returns mockLocationStore (update the
mockUseLocationStore.mockImplementation callback to only return
mockLocationStore), ensuring no noisy output from useLocationStore calls in
tests.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2265-2272: Extract the repeated cleanup block into a single
private method (e.g. private resetMonitoringState()) that performs
bluetoothStore.setIsHeadsetButtonMonitoring(false), this.pttPressActive = false,
this.clearPttReleaseFallback(), this.clearMicApplyRetry(),
this.pendingMicEnabled = null, this.stopMonitoringWatchdog(),
this.stopReadPollingFallback(), and this.stopMediaButtonFallbackMonitoring();
then replace the duplicated sequences in revertLiveKitAudioRouting,
disconnectDevice, destroy, and reset with a call to this.resetMonitoringState()
to centralize maintenance and avoid duplication.
- Around line 1210-1228: The watchdog started by startMonitoringWatchdog
currently only stops itself and never attempts to repair or restart stalled
monitoring when monitoringStartedAt is set; update startMonitoringWatchdog to
detect a stale subscription (e.g., compare Date.now() - this.monitoringStartedAt
to a threshold) and trigger a recovery path: call a recovery helper (create or
reuse a method like restartMonitoring or reinitializeHeadsetButtonMonitoring)
that re-subscribes to BLE notifications, and ensure monitoringWatchdogInterval
is cleared and restarted around that flow; keep stopMonitoringWatchdog for
cleanup and use useBluetoothAudioStore.getState().isHeadsetButtonMonitoring to
gate recovery attempts.
- Around line 1408-1419: The current loop registers every characteristic for
700ms read-polling which can overload BLE; change the logic in the block that
iterates peripheralInfo.characteristics so registerReadPollingCharacteristic is
only called for characteristics that do not support notifications/indications
(check characteristic.properties for 'notify' or 'indicate' or an explicit
characteristic.notify flag), and further restrict to a smaller set (e.g., only
known button UUIDs or a configurable whitelist) or tag non-button
characteristics to use a longer polling interval; also introduce or use a
configurable polling interval variable (instead of the hardcoded 700ms) and
enforce a maximum number of concurrently polled characteristics to avoid
saturating the link.
In `@src/services/callkeep.service.android.ts`:
- Around line 224-252: These three public methods (ignoreMuteEvents,
removeMuteListener, restoreMuteListener) call RNCallKeep and should
short-circuit on non-Android platforms like other methods; wrap their bodies
with a Platform.OS === 'android' guard (or early return when Platform.OS !==
'android') so RNCallKeep.removeEventListener/addEventListener and logger.debug
only run on Android, keeping behavior consistent with setup/startCall/endCall
and avoiding accidental RNCallKeep calls on iOS.
- Around line 24-26: The cleanup() method fails to reset timing fields
lastMuteEventTime, muteEventStormEndTime, and ignoreEventsUntil, which can leave
stale values blocking events after a re-setup; update the cleanup()
implementation in the CallKeepService (the class that declares these fields) to
explicitly set this.lastMuteEventTime = 0, this.muteEventStormEndTime = 0, and
this.ignoreEventsUntil = 0 (or the class-equivalent names) along with any
existing teardown logic so the service state is fully cleared for subsequent
initializations.
In `@src/stores/app/livekit-store.ts`:
- Around line 32-48: Replace the inline dynamic require for NativeModules with
the existing top-level react-native import: add NativeModules to the existing
import at the top of the file and remove the inline "const { NativeModules } =
require('react-native');" line; leave the rest of the logic using
inCallAudioModule (NativeModules?.InCallAudioModule), setAudioRoute,
normalizedDeviceType and logger unchanged so the try/catch and fallback to
Audio.setAudioModeAsync still work.
package.json
Outdated
There was a problem hiding this comment.
babel-plugin-transform-import-meta belongs in devDependencies, not dependencies.
This is a compile-time Babel transform. It is not needed at runtime and should not be a production dependency. Installing it as a regular dependency inflates node_modules on any environment that installs production deps only.
🔧 Proposed fix
- "babel-plugin-transform-import-meta": "^2.3.3",
"buffer": "^6.0.3",And in devDependencies:
+ "babel-plugin-transform-import-meta": "^2.3.3",
"babel-jest": "~30.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 112, The package "babel-plugin-transform-import-meta"
is listed under dependencies but it's a compile-time Babel transform and should
be a dev-only dependency; remove the "babel-plugin-transform-import-meta" entry
from the dependencies section of package.json and add the same package (same
version "^2.3.3") under devDependencies, then reinstall (npm install / yarn
install) to update lockfile so production installs won't include this plugin.
Comment on lines
+226
to
+245
| it('should fall back to legacy livekitStore when feature store is connected but has no local participant', async () => { | ||
| const service = bluetoothAudioService as any; | ||
| const featureStore = require('@/features/livekit-call/store/useLiveKitCallStore').useLiveKitCallStore; | ||
| const mockFeatureSetMicrophoneEnabled = jest.fn(); | ||
|
|
||
| featureStore.getState.mockReturnValue({ | ||
| isConnected: true, | ||
| roomInstance: null, | ||
| localParticipant: null, | ||
| actions: { | ||
| setMicrophoneEnabled: mockFeatureSetMicrophoneEnabled, | ||
| }, | ||
| }); | ||
|
|
||
| await service.setMicrophoneEnabled(true); | ||
|
|
||
| const legacyStore = require('@/stores/app/livekit-store').useLiveKitStore.getState(); | ||
| expect(mockFeatureSetMicrophoneEnabled).not.toHaveBeenCalled(); | ||
| expect(legacyStore.setMicrophoneEnabled).toHaveBeenCalledWith(true); | ||
| }); |
There was a problem hiding this comment.
Mock override on featureStore.getState leaks to subsequent tests.
jest.clearAllMocks() in beforeEach clears call history but does not reset implementations set via mockReturnValue (line 231). If tests are added after this one, they'll see isConnected: true, localParticipant: null from the feature store mock. Use mockReturnValueOnce or restore the default mock in an afterEach/afterAll block.
Proposed fix — use mockReturnValueOnce
- featureStore.getState.mockReturnValue({
+ featureStore.getState.mockReturnValueOnce({
isConnected: true,
roomInstance: null,
localParticipant: null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/__tests__/bluetooth-audio.service.test.ts` around lines 226 -
245, The featureStore.getState mock is using mockReturnValue which persists
across tests; change the test that sets featureStore.getState to use
mockReturnValueOnce (or restore the mock in an afterEach/afterAll) so the
override doesn't leak — update the test's call to
featureStore.getState.mockReturnValue(...) to
featureStore.getState.mockReturnValueOnce({...}) (or add an afterEach that calls
featureStore.getState.mockReset()/mockRestore()) to ensure
bluetoothAudioService.setMicrophoneEnabled and subsequent tests see the default
behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx (2)
122-127:⚠️ Potential issue | 🟡 MinorHardcoded
||fallbacks aftert()are unreachable dead code.
react-i18next'st()returns the key string (e.g.,'bluetooth.connection_error_title') when a translation is missing — never an empty or falsy value in default config. The|| 'Connection Failed'(and similar strings on lines 124, 263, 264) will never be used and can mislead maintainers into thinking they provide real coverage for missing keys. Remove the fallbacks or ensure the i18n keys exist.🛠️ Proposed fix
showMessage({ - message: t('bluetooth.connection_error_title') || 'Connection Failed', - description: errorMessage === 'Device disconnected' ? t('bluetooth.device_disconnected') : errorMessage || t('bluetooth.connection_error_message') || 'Could not connect to device', + message: t('bluetooth.connection_error_title'), + description: errorMessage === 'Device disconnected' ? t('bluetooth.device_disconnected') : errorMessage || t('bluetooth.connection_error_message'), type: 'danger', duration: 4000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` around lines 122 - 127, The showMessage call in bluetooth-device-selection-bottom-sheet.tsx is using unreachable "||" fallbacks after t(...) (e.g., t('bluetooth.connection_error_title') || 'Connection Failed' and the description fallbacks), so remove those redundant || fallbacks and pass the t(...) return values directly to showMessage (and any other occurrences of the same pattern in this file such as the other t(...) uses flagged) to avoid misleading dead code; locate the showMessage invocation and all t('bluetooth.*') calls in this component and delete the "||" fallback strings so the i18n key is used as-is (or ensure the translation key exists elsewhere if you want a real fallback).
181-181:⚠️ Potential issue | 🟡 Minor
&&on a numericrssiwill render0if the value is 0.When
item.rssi === 0(which some stacks emit to indicate "not available"), React Native will render the literal0character on screen. Use a ternary guard instead.🛠️ Proposed fix
- {item.rssi && <Text className="text-xs text-neutral-600 dark:text-neutral-400">RSSI: {item.rssi}dBm</Text>} + {item.rssi ? <Text className="text-xs text-neutral-600 dark:text-neutral-400">RSSI: {item.rssi}dBm</Text> : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` at line 181, The conditional rendering of RSSI using "item.rssi && <Text...>" will render "0" when item.rssi === 0; update the guard in the BluetoothDeviceSelectionBottomSheet component to use an explicit null/undefined check (e.g., item.rssi != null or typeof item.rssi === 'number') and render <Text className="...">RSSI: {item.rssi}dBm</Text> only when the value is present, replacing the && expression with a ternary or explicit check to avoid showing the literal 0.src/services/bluetooth-audio.service.ts (1)
2393-2394:⚠️ Potential issue | 🟡 MinorDuplicate assignment:
this.isInitialized = falseappears twice.Line 2394 is a redundant copy of line 2393.
Proposed fix
this.isInitialized = false; - this.isInitialized = false; this.hasAttemptedPreferredDeviceConnection = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2393 - 2394, In the constructor/initialization block where the BluetoothAudioService (or the class containing this.isInitialized) is being set up, remove the duplicate assignment so that this.isInitialized = false only appears once; locate the two identical lines setting this.isInitialized and delete the redundant one, leaving a single initialization statement (e.g., in the constructor or initialize method).
🧹 Nitpick comments (5)
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx (3)
318-318: Extract the nested ternary for Bluetooth state message into a variable.The triple-branch expression is difficult to scan at a glance. Moving it to a named variable before the return improves readability.
♻️ Proposed refactor
+ const bluetoothStateMessage = + bluetoothState === State.PoweredOff + ? t('bluetooth.poweredOff') + : bluetoothState === State.Unauthorized + ? t('bluetooth.unauthorized') + : t('bluetooth.bluetooth_not_ready', { state: bluetoothState }); {bluetoothState !== State.PoweredOn && ( <Box ...> - <Text ...> - {bluetoothState === State.PoweredOff ? t('bluetooth.poweredOff') : bluetoothState === State.Unauthorized ? t('bluetooth.unauthorized') : t('bluetooth.bluetooth_not_ready', { state: bluetoothState })} - </Text> + <Text ...>{bluetoothStateMessage}</Text> </Box> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` at line 318, The JSX contains a nested ternary for choosing the Bluetooth status string which is hard to read; extract that expression into a named variable (e.g., bluetoothStatusMessage) above the return in the BluetoothDeviceSelectionBottomSheet component, compute it using bluetoothState and t (handle State.PoweredOff, State.Unauthorized, and the default t('bluetooth.bluetooth_not_ready', { state: bluetoothState })), and then replace the inline ternary in the JSX with the bluetoothStatusMessage variable.
229-229: Replace&&with ternary for conditional rendering throughout — coding guideline violation.Lines 229, 283, 315, and 324 all use
&&for conditional rendering. While these specific expressions use object or boolean operands (so the0-rendering bug doesn't apply), they still violate the project rule. As per coding guidelines, "Use the conditional operator (?:) for conditional rendering instead of &&."♻️ Example fix pattern (apply to all four sites)
- {preferredDevice && ( - <Box ...> ... </Box> - )} + {preferredDevice ? ( + <Box ...> ... </Box> + ) : null}Also applies to: 283-283, 315-316, 324-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` at line 229, Replace all JSX conditional renderings that use the logical AND operator with the conditional (?:) operator: for each occurrence around preferredDevice (the JSX expression currently written as "{preferredDevice && (...)}" and the other three similar boolean/object checks at the same component), change them to use "{preferredDevice ? (...) : null}" (and analogous "{condition ? (...) : null}") so the component renders via the ternary operator; update the expressions at the four places referenced (the JSX blocks currently gated by && at the bluetooth-device-selection-bottom-sheet component) to follow this pattern.
246-271: Extract the System AudioonPresshandler to a nameduseCallback.The anonymous async function creates a new reference on every render, which prevents React from bailing out on the
Pressablere-render. Extract it to auseCallbackalongside the other handlers.♻️ Proposed refactor
+ const handleSystemAudioSelect = React.useCallback(async () => { + try { + useBluetoothAudioStore.getState().setIsConnecting(true); + setConnectingDeviceId('system-audio'); + await bluetoothAudioService.connectToSystemAudio(); + await setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }); + onClose(); + } catch (error) { + logger.error({ message: 'Failed to select System Audio', context: { error } }); + showMessage({ + message: t('bluetooth.connection_error_title'), + description: t('bluetooth.system_audio_error'), + type: 'danger', + }); + } finally { + useBluetoothAudioStore.getState().setIsConnecting(false); + setConnectingDeviceId(null); + } + }, [setPreferredDevice, onClose, t]); <Pressable - onPress={async () => { ... }} + onPress={handleSystemAudioSelect} disabled={!!connectingDeviceId} ... >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` around lines 246 - 271, The inline async onPress handler for the System Audio Pressable creates a new function each render; extract it into a memoized useCallback named e.g. handleSelectSystemAudio to prevent unnecessary re-renders. Move the try/catch/finally logic (calls to useBluetoothAudioStore.getState().setIsConnecting, setConnectingDeviceId, bluetoothAudioService.connectToSystemAudio, setPreferredDevice, onClose, logger.error, showMessage) into that callback and reference it from the Pressable onPress, and include all external dependencies (t, setConnectingDeviceId, onClose, bluetoothAudioService, setPreferredDevice, logger, showMessage, and any store setters) in the useCallback dependency array so behavior stays correct.src/services/bluetooth-audio.service.ts (2)
1415-1434: Specialized devices register every characteristic for read polling — consider capping.For specialized devices, the loop at lines 1415–1426 registers all characteristics for read polling regardless of capability. Combined with the 700ms polling interval, a device advertising 15–20 characteristics would result in 15–20 sequential BLE reads per cycle, which can saturate the BLE radio and drain battery.
Consider filtering to only characteristics with read capability, or capping the registered list.
Proposed guard
if (isSpecializedDevice && peripheralInfo?.characteristics?.length) { for (const characteristic of peripheralInfo.characteristics) { const serviceUuid = characteristic?.service; const characteristicUuid = characteristic?.characteristic; if (!serviceUuid || !characteristicUuid) { continue; } + if (!this.hasReadCapability(characteristic?.properties)) { + continue; + } + this.registerReadPollingCharacteristic(serviceUuid, characteristicUuid); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1415 - 1434, The loop in registerReadPollingCharacteristic registration currently registers every characteristic from peripheralInfo.characteristics (inside the block guarded by isSpecializedDevice) which can cause many BLE reads; change the logic in the block that iterates peripheralInfo.characteristics to first filter characteristics that are actually readable (e.g., characteristic.properties includes 'read' or an equivalent isReadable flag) and then limit the number registered by a configurable cap (introduce a MAX_POLL_CHARACTERISTICS constant) and only call this.registerReadPollingCharacteristic(serviceUuid, characteristicUuid) for the top N readable characteristics; reference the symbols peripheralInfo.characteristics, characteristic.properties (or isReadable), and registerReadPollingCharacteristic when making the change.
1217-1251: Monitoring watchdog is a health check with no remediation action.After the guard checks pass (device connected, monitoring active,
monitoringStartedAtset), the interval callback has no body—it performs no logging, no restart logic, or other corrective action. The watchdog only validates conditions every 4 seconds and stops itself if any condition fails.Unlike
startReadPollingFallback(which actually polls characteristics in its interval),startMonitoringWatchdogis a pure health-check mechanism. Consider whether this 4-second interval overhead is necessary, or if the guards could be consolidated into a more efficient validation pattern. If the watchdog is intended to detect and recover from stalled monitoring, that remediation logic is missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1217 - 1251, The interval callback in startMonitoringWatchdog currently only checks guards (connectedDevice, isHeadsetButtonMonitoring, monitoringStartedAt) and does nothing else, so either add remediation logic (e.g., log health events, restart monitoring by calling startMediaButtonFallbackMonitoring or restarting whatever set monitoringStartedAt, and reset monitoringStartedAt/monitoringWatchdogInterval) when the watchdog detects stalled monitoring, or remove/consolidate the interval entirely and perform these guard checks synchronously from ensurePttInputMonitoring; update references in startMonitoringWatchdog, ensurePttInputMonitoring, stopMonitoringWatchdog, and any callers (e.g., startMediaButtonFallbackMonitoring, startReadPollingFallback) to reflect the chosen approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Line 40: preferredDeviceDisplayName can become undefined when
preferredDevice.id !== 'system-audio' and preferredDevice.name is missing;
update the expression that defines preferredDeviceDisplayName to fall back to
t('bluetooth.unknown_device') when preferredDevice?.name is falsy. Locate the
const preferredDeviceDisplayName declaration and change its ternary result so
the non-system-audio branch uses preferredDevice?.name ||
t('bluetooth.unknown_device'), keeping the existing system-audio translation
branch intact.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1457-1470: The interval can re-enter pollReadCharacteristics
causing overlapping BLE reads; add a reentrancy guard (e.g., a boolean property
like isReadPollingInFlight) checked before invoking pollReadCharacteristics
inside the setInterval callback and set true when a poll starts and false in a
finally/cleanup after pollReadCharacteristics completes (including error paths),
so modify the code around readPollingInterval/pollReadCharacteristics to skip
starting a new poll when isReadPollingInFlight is true and ensure the flag is
cleared in stopReadPollingFallback and after the async poll finishes.
---
Outside diff comments:
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Around line 122-127: The showMessage call in
bluetooth-device-selection-bottom-sheet.tsx is using unreachable "||" fallbacks
after t(...) (e.g., t('bluetooth.connection_error_title') || 'Connection Failed'
and the description fallbacks), so remove those redundant || fallbacks and pass
the t(...) return values directly to showMessage (and any other occurrences of
the same pattern in this file such as the other t(...) uses flagged) to avoid
misleading dead code; locate the showMessage invocation and all t('bluetooth.*')
calls in this component and delete the "||" fallback strings so the i18n key is
used as-is (or ensure the translation key exists elsewhere if you want a real
fallback).
- Line 181: The conditional rendering of RSSI using "item.rssi && <Text...>"
will render "0" when item.rssi === 0; update the guard in the
BluetoothDeviceSelectionBottomSheet component to use an explicit null/undefined
check (e.g., item.rssi != null or typeof item.rssi === 'number') and render
<Text className="...">RSSI: {item.rssi}dBm</Text> only when the value is
present, replacing the && expression with a ternary or explicit check to avoid
showing the literal 0.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2393-2394: In the constructor/initialization block where the
BluetoothAudioService (or the class containing this.isInitialized) is being set
up, remove the duplicate assignment so that this.isInitialized = false only
appears once; locate the two identical lines setting this.isInitialized and
delete the redundant one, leaving a single initialization statement (e.g., in
the constructor or initialize method).
---
Duplicate comments:
In `@src/services/__tests__/bluetooth-audio.service.test.ts`:
- Around line 231-238: The test stubs set featureStore.getState with
mockReturnValue which persists across tests; change the setup to avoid leaking
implementations by either replacing mockReturnValue with mockReturnValueOnce
when configuring featureStore.getState in this test (referencing
featureStore.getState and the mock object used in the test) or add
jest.resetAllMocks() to the beforeEach so implementations are cleared between
tests; update the test file to use one of these approaches consistently to
prevent cross-test mock leakage.
---
Nitpick comments:
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Line 318: The JSX contains a nested ternary for choosing the Bluetooth status
string which is hard to read; extract that expression into a named variable
(e.g., bluetoothStatusMessage) above the return in the
BluetoothDeviceSelectionBottomSheet component, compute it using bluetoothState
and t (handle State.PoweredOff, State.Unauthorized, and the default
t('bluetooth.bluetooth_not_ready', { state: bluetoothState })), and then replace
the inline ternary in the JSX with the bluetoothStatusMessage variable.
- Line 229: Replace all JSX conditional renderings that use the logical AND
operator with the conditional (?:) operator: for each occurrence around
preferredDevice (the JSX expression currently written as "{preferredDevice &&
(...)}" and the other three similar boolean/object checks at the same
component), change them to use "{preferredDevice ? (...) : null}" (and analogous
"{condition ? (...) : null}") so the component renders via the ternary operator;
update the expressions at the four places referenced (the JSX blocks currently
gated by && at the bluetooth-device-selection-bottom-sheet component) to follow
this pattern.
- Around line 246-271: The inline async onPress handler for the System Audio
Pressable creates a new function each render; extract it into a memoized
useCallback named e.g. handleSelectSystemAudio to prevent unnecessary
re-renders. Move the try/catch/finally logic (calls to
useBluetoothAudioStore.getState().setIsConnecting, setConnectingDeviceId,
bluetoothAudioService.connectToSystemAudio, setPreferredDevice, onClose,
logger.error, showMessage) into that callback and reference it from the
Pressable onPress, and include all external dependencies (t,
setConnectingDeviceId, onClose, bluetoothAudioService, setPreferredDevice,
logger, showMessage, and any store setters) in the useCallback dependency array
so behavior stays correct.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1415-1434: The loop in registerReadPollingCharacteristic
registration currently registers every characteristic from
peripheralInfo.characteristics (inside the block guarded by isSpecializedDevice)
which can cause many BLE reads; change the logic in the block that iterates
peripheralInfo.characteristics to first filter characteristics that are actually
readable (e.g., characteristic.properties includes 'read' or an equivalent
isReadable flag) and then limit the number registered by a configurable cap
(introduce a MAX_POLL_CHARACTERISTICS constant) and only call
this.registerReadPollingCharacteristic(serviceUuid, characteristicUuid) for the
top N readable characteristics; reference the symbols
peripheralInfo.characteristics, characteristic.properties (or isReadable), and
registerReadPollingCharacteristic when making the change.
- Around line 1217-1251: The interval callback in startMonitoringWatchdog
currently only checks guards (connectedDevice, isHeadsetButtonMonitoring,
monitoringStartedAt) and does nothing else, so either add remediation logic
(e.g., log health events, restart monitoring by calling
startMediaButtonFallbackMonitoring or restarting whatever set
monitoringStartedAt, and reset monitoringStartedAt/monitoringWatchdogInterval)
when the watchdog detects stalled monitoring, or remove/consolidate the interval
entirely and perform these guard checks synchronously from
ensurePttInputMonitoring; update references in startMonitoringWatchdog,
ensurePttInputMonitoring, stopMonitoringWatchdog, and any callers (e.g.,
startMediaButtonFallbackMonitoring, startReadPollingFallback) to reflect the
chosen approach.
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/bluetooth-audio.service.ts (1)
2403-2404:⚠️ Potential issue | 🟡 MinorDuplicate
this.isInitialized = false— copy-paste artifact.Line 2403 and Line 2404 both assign the same value. Remove the duplicate.
🛡️ Proposed fix
// Reset initialization flags this.isInitialized = false; - this.isInitialized = false; this.hasAttemptedPreferredDeviceConnection = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2403 - 2404, In BluetoothAudioService remove the duplicate assignment to the instance flag: there are two consecutive lines setting this.isInitialized = false; (a copy-paste artifact) — delete the redundant one so only a single this.isInitialized = false remains in the constructor/initializer where it appears.
🧹 Nitpick comments (6)
src/components/settings/bluetooth-device-selection-bottom-sheet.tsx (4)
257-257: Storing a translated string as persistent devicenameis an anti-pattern.
t('bluetooth.system_audio')is locale-dependent at call time; if the user changes the app language, the storednameinpreferredDevicebecomes stale. In this component the risk is mitigated —preferredDeviceDisplayName(line 40) re-translates forid === 'system-audio'and ignores the stored name. But any other consumer that readspreferredDevice.namedirectly will display the wrong locale.Consider storing a stable sentinel instead:
- await setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }); + await setPreferredDevice({ id: 'system-audio', name: 'system-audio' });Display-layer helpers can then map
id === 'system-audio'→t('bluetooth.system_audio')consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` at line 257, The code currently stores a locale-dependent translation via setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }), which makes preferredDevice.name stale if the user changes language; instead store a stable sentinel for the special device (e.g., use name: null or a constant like 'SYSTEM_AUDIO_SENTINEL') when calling setPreferredDevice for id === 'system-audio', and update display helpers (e.g., preferredDeviceDisplayName) to map that sentinel/id to t('bluetooth.system_audio') at render time so all consumers read a locale-independent stored value and the UI always shows the current translation.
56-61:hasScannedis never reset when the sheet closes — auto-scan only fires once per component mount.Because there's no
setHasScanned(false)whenisOpenbecomesfalse, reopening the bottom sheet within the same component-lifetime won't trigger an automatic rescan. Users will see the previously scanned device list and must manually press "Scan Again". The manual button makes this recoverable, but the UX expectation is usually a fresh scan on each open.Consider resetting
hasScannedinside the existing close-time effect:♻️ Proposed fix
useEffect(() => { if (!isOpen && isScanning) { stopScan(); } + if (!isOpen) { + setHasScanned(false); + } }, [isOpen, isScanning, stopScan]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` around lines 56 - 61, The issue: hasScanned is never reset so auto-scan only runs once per mount; fix by resetting it when the sheet closes. In bluetooth-device-selection-bottom-sheet.tsx add logic (e.g. a useEffect watching isOpen) to call setHasScanned(false) when isOpen becomes false (or extend the existing useEffect that uses isOpen/hasScanned/startScan to setHasScanned(false) in the else branch), ensuring that on every open the startScan() auto-trigger can run again.
318-318: Inconsistent translation key naming within a single expression.The three keys used here follow three different conventions:
bluetooth.poweredOff(camelCase),bluetooth.unauthorized(lowercase), andbluetooth.bluetooth_not_ready(snake_case with redundantbluetooth_prefix under thebluetooth.namespace). Standardizing to one convention across the translation namespace would improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` at line 318, The three translation keys used in the expression that calls t(...) with bluetoothState (inside the bluetoothState logic in the BluetoothDeviceSelectionBottomSheet component) are inconsistent—replace them with a single naming convention and remove the redundant namespace prefix (e.g., use bluetooth.poweredOff, bluetooth.unauthorized, bluetooth.notReady or bluetooth.powered_off, bluetooth.unauthorized, bluetooth.not_ready) and update the corresponding i18n resource keys to match; ensure the variable bluetoothState and the t(...) calls remain unchanged, only standardize the keys and resource entries so the expression uses a consistent key format (and update any other usages of the old keys).
246-271: Extract System AudioonPressto a nameduseCallback— coding guideline violation.The entire
onPresshandler is an inline anonymous async function, creating a new function reference on every render. As per coding guidelines, "Avoid anonymous functions inrenderItemor event handlers to prevent re-renders."♻️ Proposed refactor
+ const handleSystemAudioSelect = React.useCallback(async () => { + try { + useBluetoothAudioStore.getState().setIsConnecting(true); + setConnectingDeviceId('system-audio'); + await bluetoothAudioService.connectToSystemAudio(); + await setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }); + onClose(); + } catch (error) { + logger.error({ message: 'Failed to select System Audio', context: { error } }); + showMessage({ + message: t('bluetooth.connection_error_title'), + description: t('bluetooth.system_audio_error'), + type: 'danger', + }); + } finally { + useBluetoothAudioStore.getState().setIsConnecting(false); + setConnectingDeviceId(null); + } + }, [setPreferredDevice, onClose, t]); <Pressable - onPress={async () => { - try { ... } catch { ... } finally { ... } - }} + onPress={handleSystemAudioSelect} ... >As per coding guidelines: "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx` around lines 246 - 271, The inline anonymous async onPress handler on the Pressable should be extracted into a named useCallback to prevent a new function reference each render; create a handler (e.g., handleSelectSystemAudio) using React.useCallback that contains the try/catch/finally logic currently in onPress and calls useBluetoothAudioStore.getState().setIsConnecting, setConnectingDeviceId('system-audio'), await bluetoothAudioService.connectToSystemAudio(), await setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }), onClose(), logger.error(...), and showMessage(...); ensure the useCallback dependency array includes any external values used (t, bluetoothAudioService, setPreferredDevice, onClose, logger, showMessage, setConnectingDeviceId) so closures remain correct, then replace the inline onPress with onPress={handleSelectSystemAudio}.src/services/bluetooth-audio.service.ts (2)
1218-1237:startMonitoringWatchdoghas an empty happy-path body — watchdog takes no corrective action.The interval only self-terminates when preconditions fail. When all conditions are healthy, the callback body is a no-op (
monitoringStartedAtis checked but unused). A watchdog that never heals is just a self-cancelling timer. Either add a stale-monitoring detection/restart action (e.g., ifDate.now() - this.monitoringStartedAt > threshold) or rename the method to reflect its actual role.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1218 - 1237, startMonitoringWatchdog currently only cancels itself when preconditions fail and never takes corrective action when monitoring becomes stale; update the interval callback inside startMonitoringWatchdog to detect stale monitoring by comparing Date.now() against this.monitoringStartedAt (e.g., if Date.now() - this.monitoringStartedAt > STALE_THRESHOLD_MS) and then perform a corrective action such as calling a restart method (e.g., this.restartMonitoring() or re-initializing the headset monitoring flow), logging the event, and/or resetting this.monitoringStartedAt; keep existing early-exit checks (this.connectedDevice.id, useBluetoothAudioStore.getState().isHeadsetButtonMonitoring) and ensure this.monitoringWatchdogInterval and stopMonitoringWatchdog are used to clear/restart the timer as needed.
2152-2154:setMicrophoneEnabledis dead code that bypasses the serialization queue.This private method is never called — all internal callers use
requestMicrophoneStateorapplyMicrophoneEnableddirectly. If it were ever called, it would bypass theisApplyingMicState/pendingMicEnabledserializer inrequestMicrophoneState, racing with any in-flight apply. Remove it to prevent accidental usage.♻️ Proposed removal
- private async setMicrophoneEnabled(enabled: boolean): Promise<void> { - await this.applyMicrophoneEnabled(enabled); - } - private async applyMicrophoneEnabled(enabled: boolean): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2152 - 2154, Remove the unused private method setMicrophoneEnabled which bypasses the requestMicrophoneState serialization logic; search for and delete the setMicrophoneEnabled method declaration and any references to it, and ensure callers continue to use requestMicrophoneState or applyMicrophoneEnabled so the isApplyingMicState / pendingMicEnabled serializer remains authoritative for microphone state changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1487-1498: The change-detection logic in
handleButtonEventFromCharacteristic (using entry.lastHexValue and nextHexValue)
treats the initial null lastHexValue as a change and emits a phantom PTT start
for generic devices because parseGenericButtonData(0x00) maps to ptt_start;
instead, on the first successful read prime entry.lastHexValue = nextHexValue
and skip calling handleButtonEventFromCharacteristic/processButtonEvent so no
event is dispatched; specifically, update the code around where
entry.lastHexValue is compared/assigned to detect the "first read" case
(entry.lastHexValue === null) and set entry.lastHexValue = nextHexValue then
continue, preserving existing behavior for subsequent reads and leaving
schedulePttReleaseFallback unchanged.
- Around line 1416-1427: The second loop unconditionally calls
registerReadPollingCharacteristic for every characteristic, which re-adds ones
that failed subscription and causes repeated read errors; fix by tracking which
characteristics were successfully subscribed in the first pass (e.g., a Set
keyed by serviceUuid+characteristicUuid populated when
startNotification/registerReadPollingCharacteristic is invoked successfully) and
then, inside this loop over peripheralInfo.characteristics, skip any
characteristic not present in that "successfullySubscribed" set (only call
registerReadPollingCharacteristic for keys that were successfully subscribed and
have valid serviceUuid and characteristicUuid).
- Around line 2108-2117: The retry currently captures the incoming enabled value
in the closure and ignores subsequent calls because of the if
(this.micApplyRetryTimeout) guard; instead update a shared pendingMicEnabled
property when scheduleMicApplyRetry(enabled) is called (even if a timeout
already exists) and, inside the setTimeout handler, read this.pendingMicEnabled
and pass that to requestMicrophoneState; also ensure the handler clears
this.micApplyRetryTimeout before calling requestMicrophoneState so behavior of
requestMicrophoneState/applyMicrophoneEnabled remains unchanged. Use the
existing fields scheduleMicApplyRetry, micApplyRetryTimeout, and
pendingMicEnabled to implement this.
---
Outside diff comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2403-2404: In BluetoothAudioService remove the duplicate
assignment to the instance flag: there are two consecutive lines setting
this.isInitialized = false; (a copy-paste artifact) — delete the redundant one
so only a single this.isInitialized = false remains in the
constructor/initializer where it appears.
---
Nitpick comments:
In `@src/components/settings/bluetooth-device-selection-bottom-sheet.tsx`:
- Line 257: The code currently stores a locale-dependent translation via
setPreferredDevice({ id: 'system-audio', name: t('bluetooth.system_audio') }),
which makes preferredDevice.name stale if the user changes language; instead
store a stable sentinel for the special device (e.g., use name: null or a
constant like 'SYSTEM_AUDIO_SENTINEL') when calling setPreferredDevice for id
=== 'system-audio', and update display helpers (e.g.,
preferredDeviceDisplayName) to map that sentinel/id to
t('bluetooth.system_audio') at render time so all consumers read a
locale-independent stored value and the UI always shows the current translation.
- Around line 56-61: The issue: hasScanned is never reset so auto-scan only runs
once per mount; fix by resetting it when the sheet closes. In
bluetooth-device-selection-bottom-sheet.tsx add logic (e.g. a useEffect watching
isOpen) to call setHasScanned(false) when isOpen becomes false (or extend the
existing useEffect that uses isOpen/hasScanned/startScan to setHasScanned(false)
in the else branch), ensuring that on every open the startScan() auto-trigger
can run again.
- Line 318: The three translation keys used in the expression that calls t(...)
with bluetoothState (inside the bluetoothState logic in the
BluetoothDeviceSelectionBottomSheet component) are inconsistent—replace them
with a single naming convention and remove the redundant namespace prefix (e.g.,
use bluetooth.poweredOff, bluetooth.unauthorized, bluetooth.notReady or
bluetooth.powered_off, bluetooth.unauthorized, bluetooth.not_ready) and update
the corresponding i18n resource keys to match; ensure the variable
bluetoothState and the t(...) calls remain unchanged, only standardize the keys
and resource entries so the expression uses a consistent key format (and update
any other usages of the old keys).
- Around line 246-271: The inline anonymous async onPress handler on the
Pressable should be extracted into a named useCallback to prevent a new function
reference each render; create a handler (e.g., handleSelectSystemAudio) using
React.useCallback that contains the try/catch/finally logic currently in onPress
and calls useBluetoothAudioStore.getState().setIsConnecting,
setConnectingDeviceId('system-audio'), await
bluetoothAudioService.connectToSystemAudio(), await setPreferredDevice({ id:
'system-audio', name: t('bluetooth.system_audio') }), onClose(),
logger.error(...), and showMessage(...); ensure the useCallback dependency array
includes any external values used (t, bluetoothAudioService, setPreferredDevice,
onClose, logger, showMessage, setConnectingDeviceId) so closures remain correct,
then replace the inline onPress with onPress={handleSelectSystemAudio}.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1218-1237: startMonitoringWatchdog currently only cancels itself
when preconditions fail and never takes corrective action when monitoring
becomes stale; update the interval callback inside startMonitoringWatchdog to
detect stale monitoring by comparing Date.now() against this.monitoringStartedAt
(e.g., if Date.now() - this.monitoringStartedAt > STALE_THRESHOLD_MS) and then
perform a corrective action such as calling a restart method (e.g.,
this.restartMonitoring() or re-initializing the headset monitoring flow),
logging the event, and/or resetting this.monitoringStartedAt; keep existing
early-exit checks (this.connectedDevice.id,
useBluetoothAudioStore.getState().isHeadsetButtonMonitoring) and ensure
this.monitoringWatchdogInterval and stopMonitoringWatchdog are used to
clear/restart the timer as needed.
- Around line 2152-2154: Remove the unused private method setMicrophoneEnabled
which bypasses the requestMicrophoneState serialization logic; search for and
delete the setMicrophoneEnabled method declaration and any references to it, and
ensure callers continue to use requestMicrophoneState or applyMicrophoneEnabled
so the isApplyingMicState / pendingMicEnabled serializer remains authoritative
for microphone state changes.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/bluetooth-audio.service.ts (1)
2421-2422:⚠️ Potential issue | 🟡 MinorDuplicate
this.isInitialized = falseassignment indestroy().🛡️ Proposed fix
- this.isInitialized = false; this.isInitialized = false; this.hasAttemptedPreferredDeviceConnection = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2421 - 2422, Duplicate assignment to this.isInitialized is present in the destroy() method; remove the redundant line so there is only a single this.isInitialized = false assignment in destroy(), ensuring the property is still reset once during cleanup (refer to the destroy() method and the this.isInitialized property to locate the change).
🧹 Nitpick comments (1)
src/services/bluetooth-audio.service.ts (1)
1416-1432: Third loop instartNotificationsForButtonControlsis redundant for specialized devices.For
isSpecializedDevice, Loop 2 (lines 1385–1411) already callsregisterReadPollingCharacteristicfor every successfully-subscribed characteristic (condition at line 1389:isSpecializedDevice || hasReadCapabilityalways satisfies whenisSpecializedDeviceis true). Loop 1 registers known button configs.registerReadPollingCharacteristicitself deduplicates (lines 1443–1446). The third loop therefore adds nothing and can be removed.♻️ Proposed removal
- if (isSpecializedDevice && peripheralInfo?.characteristics?.length) { - for (const characteristic of peripheralInfo.characteristics) { - const serviceUuid = characteristic?.service; - const characteristicUuid = characteristic?.characteristic; - - if (!serviceUuid || !characteristicUuid) { - continue; - } - - const pairKey = `${String(serviceUuid).toUpperCase()}::${String(characteristicUuid).toUpperCase()}`; - if (!successfullySubscribed.has(pairKey)) { - continue; - } - - this.registerReadPollingCharacteristic(serviceUuid, characteristicUuid); - } - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1416 - 1432, The third for-loop inside startNotificationsForButtonControls that iterates when isSpecializedDevice and calls registerReadPollingCharacteristic for each characteristic is redundant because Loop 2 already registers read-polling for successfully-subscribed characteristics when isSpecializedDevice is true and registerReadPollingCharacteristic deduplicates calls; remove this entire loop (the block referencing isSpecializedDevice, peripheralInfo?.characteristics, pairKey, successfullySubscribed, and registerReadPollingCharacteristic) to avoid duplicate iteration and dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/__tests__/bluetooth-audio.service.test.ts`:
- Around line 98-100: The afterEach unconditionally calls
bluetoothAudioService.destroy(), causing a second invocation (and duplicate
async disconnectDevice() calls and store updates) for tests that already call
destroy(); either stop calling the real destroy twice by replacing in-test
destroy assertions with a jest.spyOn(bluetoothAudioService, 'destroy') (or spy
on disconnectDevice/ store methods) to verify effects without invoking the real
implementation, or guard the afterEach so it skips calling
bluetoothAudioService.destroy() when the service is already destroyed (e.g.,
check a public isDestroyed flag or set a local test-scoped boolean when a test
calls destroy), and update the tests "should reset flags on destroy method" and
"should allow re-attempting connection after destroy" to use the spy or the
guard accordingly.
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1218-1237: The interval body in startMonitoringWatchdog currently
only cancels itself and never attempts recovery; update it so when the checks
pass (connectedDevice matches deviceId, isHeadsetButtonMonitoring is true, and
monitoringStartedAt exists) you detect stale monitoring (e.g., by comparing
monitoringStartedAt to Date.now() or just on every tick) and call a recovery
routine such as this.ensurePttInputMonitoring('watchdog') or
this.startNotificationsForButtonControls() to re-subscribe notifications; keep
stopMonitoringWatchdog, monitoringWatchdogInterval, monitoringStartedAt and
useBluetoothAudioStore.getState().isHeadsetButtonMonitoring as the referenced
symbols, and if this no-op was intentional instead add a clear TODO comment
explaining why no recovery is performed.
- Around line 2118-2168: The loop busy-wait is caused because
scheduleMicApplyRetry writes this.pendingMicEnabled before the timeout, keeping
requestMicrophoneState's while-loop active; fix by introducing a separate
sentinel (e.g., this.retryMicEnabled) and only set retryMicEnabled inside
scheduleMicApplyRetry (never pendingMicEnabled), have the timeout handler
transfer retryMicEnabled -> pendingMicEnabled and call requestMicrophoneState,
and clear retryMicEnabled in clearMicApplyRetry and in reset paths (methods like
revertLiveKitAudioRouting, disconnectDevice, destroy, reset) so the while-loop
in requestMicrophoneState can exit normally when a retry is pending but not yet
scheduled.
---
Outside diff comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2421-2422: Duplicate assignment to this.isInitialized is present
in the destroy() method; remove the redundant line so there is only a single
this.isInitialized = false assignment in destroy(), ensuring the property is
still reset once during cleanup (refer to the destroy() method and the
this.isInitialized property to locate the change).
---
Duplicate comments:
In `@src/services/__tests__/bluetooth-audio.service.test.ts`:
- Around line 227-246: The test sets a persistent mock implementation on
useLiveKitCallStore.getState via featureStore.getState.mockReturnValue which
survives other tests; change it to mockReturnValueOnce so the override only
applies to this test: update the test that references featureStore.getState
(from useLiveKitCallStore) to call mockReturnValueOnce(...) instead of
mockReturnValue(...), ensuring other tests see the default mock; keep
jest.clearAllMocks() in beforeEach as-is to clear call history but not
implementations.
---
Nitpick comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1416-1432: The third for-loop inside
startNotificationsForButtonControls that iterates when isSpecializedDevice and
calls registerReadPollingCharacteristic for each characteristic is redundant
because Loop 2 already registers read-polling for successfully-subscribed
characteristics when isSpecializedDevice is true and
registerReadPollingCharacteristic deduplicates calls; remove this entire loop
(the block referencing isSpecializedDevice, peripheralInfo?.characteristics,
pairKey, successfullySubscribed, and registerReadPollingCharacteristic) to avoid
duplicate iteration and dead code.
Comment on lines
+98
to
+100
| afterEach(() => { | ||
| bluetoothAudioService.destroy(); | ||
| }); |
There was a problem hiding this comment.
afterEach destroy double-invokes destroy() for tests that already call it.
The tests "should reset flags on destroy method" (line 151) and "should allow re-attempting connection after destroy" (line 197) call bluetoothAudioService.destroy() inside the test body. afterEach then calls it a second time. While destroy() is effectively idempotent (null-guards on all intervals/listeners), it does fire-and-forget disconnectDevice() (which is async) twice and re-invokes the Zustand store calls twice. Consider using jest.spyOn to test destroy()'s effects rather than calling the real singleton method, or move the afterEach guard to skip if already destroyed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/__tests__/bluetooth-audio.service.test.ts` around lines 98 -
100, The afterEach unconditionally calls bluetoothAudioService.destroy(),
causing a second invocation (and duplicate async disconnectDevice() calls and
store updates) for tests that already call destroy(); either stop calling the
real destroy twice by replacing in-test destroy assertions with a
jest.spyOn(bluetoothAudioService, 'destroy') (or spy on disconnectDevice/ store
methods) to verify effects without invoking the real implementation, or guard
the afterEach so it skips calling bluetoothAudioService.destroy() when the
service is already destroyed (e.g., check a public isDestroyed flag or set a
local test-scoped boolean when a test calls destroy), and update the tests
"should reset flags on destroy method" and "should allow re-attempting
connection after destroy" to use the spy or the guard accordingly.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/bluetooth-audio.service.ts (2)
2289-2342:⚠️ Potential issue | 🟠 MajorMicrophone stuck enabled when BT device disconnects mid-PTT.
Both
revertLiveKitAudioRouting(line 2316) anddisconnectDevice(line 2335) setthis.pttPressActive = falseand thenthis.pendingMicEnabled = null— but never callrequestMicrophoneState(false). If PTT was active when the BT device disconnects (or routing is reverted), LiveKit's microphone remains enabled, leaving the user unknowingly broadcasting.
requestMicrophoneState(false)can be safely called before clearingpttPressActivebecause the async IIFE inside it synchronously reads and clearspendingMicEnabledbefore its firstawait(JS single-threaded); the subsequentthis.pendingMicEnabled = nullreset in both methods is already a no-op.🛡️ Proposed fix — apply in both
revertLiveKitAudioRoutinganddisconnectDevice- this.pttPressActive = false; + if (this.pttPressActive) { + this.requestMicrophoneState(false); + } + this.pttPressActive = false; this.clearPttReleaseFallback();Apply the same change at:
revertLiveKitAudioRouting, before line 2316disconnectDevice, before line 2335🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2289 - 2342, Both revertLiveKitAudioRouting and disconnectDevice fail to turn off the LiveKit mic when a BT PTT disconnects; call requestMicrophoneState(false) in each function before clearing this.pttPressActive so the microphone is actually disabled, then proceed with clearing pendingMicEnabled and other cleanup; update revertLiveKitAudioRouting and disconnectDevice to invoke requestMicrophoneState(false) prior to setting this.pttPressActive = false (it's safe because the async IIFE in requestMicrophoneState synchronously reads/clears pendingMicEnabled before any awaits).
2439-2440:⚠️ Potential issue | 🟡 MinorRemove the duplicate
this.isInitialized = falseassignment.Line 2440 is an exact copy of line 2439 — the second assignment is a no-op and is clearly a copy-paste mistake.
🐛 Proposed fix
// Reset initialization flags this.isInitialized = false; - this.isInitialized = false; this.hasAttemptedPreferredDeviceConnection = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 2439 - 2440, Remove the duplicate no-op assignment to the isInitialized property in the BluetoothAudioService initialization code: locate the two consecutive lines setting this.isInitialized = false (both the one on startLine 2439 and the duplicate on 2440) and delete the redundant second assignment so only a single this.isInitialized = false remains; ensure no other logic depends on the removed duplicate.
🧹 Nitpick comments (1)
src/services/bluetooth-audio.service.ts (1)
1427-1443: Third pass loop is redundant.The third pass (lines 1427–1443) only iterates over
successfullySubscribedentries and callsregisterReadPollingCharacteristic. However, every entry insuccessfullySubscribedfor a specialized device was already registered in either the first loop (line 1357, explicitbuttonControlConfigs) or the second loop (line 1401, guarded byisSpecializedDevice || hasReadCapability, which always evaluates totruefor specialized devices). SinceregisterReadPollingCharacteristicdeduplicates internally (lines 1454–1458), this third pass adds nothing.♻️ Proposed cleanup
- if (isSpecializedDevice && peripheralInfo?.characteristics?.length) { - for (const characteristic of peripheralInfo.characteristics) { - const serviceUuid = characteristic?.service; - const characteristicUuid = characteristic?.characteristic; - - if (!serviceUuid || !characteristicUuid) { - continue; - } - - const pairKey = `${String(serviceUuid).toUpperCase()}::${String(characteristicUuid).toUpperCase()}`; - if (!successfullySubscribed.has(pairKey)) { - continue; - } - - this.registerReadPollingCharacteristic(serviceUuid, characteristicUuid); - } - } - if (successfullySubscribed.size > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/bluetooth-audio.service.ts` around lines 1427 - 1443, The third loop iterating over peripheralInfo.characteristics and checking successfullySubscribed before calling registerReadPollingCharacteristic is redundant for specialized devices because those entries are already handled by the first loop (buttonControlConfigs) or the second loop (guarded by isSpecializedDevice || hasReadCapability) and registerReadPollingCharacteristic already deduplicates; remove the entire third pass (the block that references peripheralInfo.characteristics and uses pairKey and successfullySubscribed) to eliminate dead code and rely on the earlier registration paths and internal deduplication in registerReadPollingCharacteristic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 2289-2342: Both revertLiveKitAudioRouting and disconnectDevice
fail to turn off the LiveKit mic when a BT PTT disconnects; call
requestMicrophoneState(false) in each function before clearing
this.pttPressActive so the microphone is actually disabled, then proceed with
clearing pendingMicEnabled and other cleanup; update revertLiveKitAudioRouting
and disconnectDevice to invoke requestMicrophoneState(false) prior to setting
this.pttPressActive = false (it's safe because the async IIFE in
requestMicrophoneState synchronously reads/clears pendingMicEnabled before any
awaits).
- Around line 2439-2440: Remove the duplicate no-op assignment to the
isInitialized property in the BluetoothAudioService initialization code: locate
the two consecutive lines setting this.isInitialized = false (both the one on
startLine 2439 and the duplicate on 2440) and delete the redundant second
assignment so only a single this.isInitialized = false remains; ensure no other
logic depends on the removed duplicate.
---
Nitpick comments:
In `@src/services/bluetooth-audio.service.ts`:
- Around line 1427-1443: The third loop iterating over
peripheralInfo.characteristics and checking successfullySubscribed before
calling registerReadPollingCharacteristic is redundant for specialized devices
because those entries are already handled by the first loop
(buttonControlConfigs) or the second loop (guarded by isSpecializedDevice ||
hasReadCapability) and registerReadPollingCharacteristic already deduplicates;
remove the entire third pass (the block that references
peripheralInfo.characteristics and uses pairKey and successfullySubscribed) to
eliminate dead code and rely on the earlier registration paths and internal
deduplication in registerReadPollingCharacteristic.
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests / Chores