feat(rn): remove peer connection usage in speech detection#2200
feat(rn): remove peer connection usage in speech detection#2200santhoshvai merged 4 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved the RNSpeechDetector implementation and the useSpeechDetection hook; replaced detector logic with a native event subscription model via Changes
Sequence Diagram(s)sequenceDiagram
participant MM as MicrophoneManager
participant SDK as streamRNVideoSDK.nativeEvents
participant Native as audioDeviceModuleEvents (native)
MM->>SDK: subscribe(callback)
SDK->>Native: register addSpeechActivityListener(handler)
Native-->>SDK: emit event ("started"/"stopped")
SDK-->>MM: callback({ isSoundDetected: true/false })
MM->>MM: update internal sound state / emit to consumers
MM->>SDK: unsubscribe() / SDK->>Native: remove listener
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (1)
packages/client/src/devices/__tests__/MicrophoneManagerRN.test.ts (1)
55-80: Add one regression test for missingnativeEvents.speechActivity.The new positive-path tests are good, but the fallback branch in
MicrophoneManager(whennativeEvents.speechActivityis absent) is currently untested. Please add a test that setsglobalThis.streamRNVideoSDK.nativeEventstoundefinedand asserts no throw plus warning behavior.As per coding guidelines:
**/*.{test,spec}.{ts,tsx}: Add regression tests for bug fixes.Also applies to: 146-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/devices/__tests__/MicrophoneManagerRN.test.ts` around lines 55 - 80, Add a regression test in MicrophoneManagerRN.test.ts that covers the fallback when globalThis.streamRNVideoSDK.nativeEvents is missing: in the beforeEach for this specific test set globalThis.streamRNVideoSDK.nativeEvents = undefined (or delete it), instantiate/use MicrophoneManager methods that would normally read nativeEvents.speechActivity, and assert that no exception is thrown, that subscribeMock is not called and that a warning was emitted (spy on console.warn or the module logger). Ensure the test references the existing speechActivityCallback/subscribeMock/unsubscribeMocks setup so it verifies the fallback path and cleanup behavior for MicrophoneManager.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sample-apps/react-native/dogfood/package.json`:
- Line 25: The dependency bump to "@stream-io/react-native-webrtc":
"137.1.4-alpha.8" is a required, not exploratory, change because the feature in
packages/react-native-sdk/src/utils/internal/registerSDKGlobals.ts depends on
audioDeviceModuleEvents.addSpeechActivityListener(); update the PR description
and changelog to state that this alpha version is a hard requirement for that
feature (mention the specific function
audioDeviceModuleEvents.addSpeechActivityListener and file
registerSDKGlobals.ts) so reviewers understand the upgrade is intentional and
isolated from other feature changes.
---
Nitpick comments:
In `@packages/client/src/devices/__tests__/MicrophoneManagerRN.test.ts`:
- Around line 55-80: Add a regression test in MicrophoneManagerRN.test.ts that
covers the fallback when globalThis.streamRNVideoSDK.nativeEvents is missing: in
the beforeEach for this specific test set
globalThis.streamRNVideoSDK.nativeEvents = undefined (or delete it),
instantiate/use MicrophoneManager methods that would normally read
nativeEvents.speechActivity, and assert that no exception is thrown, that
subscribeMock is not called and that a warning was emitted (spy on console.warn
or the module logger). Ensure the test references the existing
speechActivityCallback/subscribeMock/unsubscribeMocks setup so it verifies the
fallback path and cleanup behavior for MicrophoneManager.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d733528b-4df5-402d-8c34-57196ac18973
⛔ Files ignored due to path filters (2)
sample-apps/react-native/dogfood/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
packages/client/index.tspackages/client/src/devices/MicrophoneManager.tspackages/client/src/devices/__tests__/MicrophoneManagerRN.test.tspackages/client/src/helpers/RNSpeechDetector.tspackages/client/src/helpers/__tests__/RNSpeechDetector.test.tspackages/client/src/types.tspackages/react-native-sdk/CLAUDE.mdpackages/react-native-sdk/package.jsonpackages/react-native-sdk/src/hooks/index.tspackages/react-native-sdk/src/hooks/useSpeechDetection.tspackages/react-native-sdk/src/utils/internal/registerSDKGlobals.tssample-apps/react-native/dogfood/package.json
💤 Files with no reviewable changes (6)
- packages/react-native-sdk/CLAUDE.md
- packages/client/index.ts
- packages/react-native-sdk/src/hooks/index.ts
- packages/react-native-sdk/src/hooks/useSpeechDetection.ts
- packages/client/src/helpers/tests/RNSpeechDetector.test.ts
- packages/client/src/helpers/RNSpeechDetector.ts
| "@stream-io/noise-cancellation-react-native": "workspace:^", | ||
| "@stream-io/react-native-callingx": "workspace:^", | ||
| "@stream-io/react-native-webrtc": "137.1.3", | ||
| "@stream-io/react-native-webrtc": "137.1.4-alpha.8", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does @stream-io/react-native-webrtc version 137.1.4-alpha.8 explicitly include audioDeviceModuleEvents.addSpeechActivityListener, and is it documented as compatible with react-native 0.83.x?
💡 Result:
No, @stream-io/react-native-webrtc version 137.1.4-alpha.8 does not explicitly include audioDeviceModuleEvents.addSpeechActivityListener, and it is not documented as compatible with React Native 0.83.x. The package is a fork of react-native-webrtc maintained by GetStream, with recent activity up to March 2026 (latest stable v137.1.3). No sources mention version 137.1.4-alpha.8 specifically, nor changelog entries for audioDeviceModuleEvents.addSpeechActivityListener (a WebRTC Android AudioDeviceModule feature for speech detection). Searches for the method in react-native-webrtc source/code yield no matches; audio levels are accessed via getStats on inbound-rtp reports, not this listener. No documentation or issues reference it being exposed in JS. No compatibility documentation for React Native 0.83.x exists; general guides assume standard RN install steps without version-specific notes. react-native-webrtc (upstream) supports recent RN versions implicitly via examples/community use, but no explicit statement for 0.83.x.
Citations:
- 1: https://github.com/GetStream/react-native-webrtc
- 2: https://www.npmjs.com/package/@stream-io/react-native-webrtc
- 3: https://snyk.io/advisor/npm-package/%40stream-io%2Freact-native-webrtc
- 4: https://cdn.jsdelivr.net/npm/@stream-io/react-native-webrtc@118.1.0/android/
- 5: https://github.com/react-native-webrtc/react-native-webrtc/
- 6: https://github.com/react-native-webrtc/react-native-webrtc/tree/master
- 7: https://github.com/react-native-webrtc/react-native-webrtc/blob/master/android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java
- 8: Difficulty Implementing "Who is Speaking" Feature with react-native-simple-peer and react-native-webrtc react-native-webrtc/react-native-webrtc#1638
🏁 Script executed:
# First, find and examine the registerSDKGlobals.ts file mentioned in the review comment
fd -t f "registerSDKGlobals.ts"Repository: GetStream/stream-video-js
Length of output: 135
🏁 Script executed:
# Read the full registerSDKGlobals.ts file to check imports and usage
cat -n packages/react-native-sdk/src/utils/internal/registerSDKGlobals.tsRepository: GetStream/stream-video-js
Length of output: 4226
🏁 Script executed:
# Check the git diff or status to see what changed in this PR
# First, let's look at the dog-food package.json in current state and see what the old version was
git log --oneline -20 sample-apps/react-native/dogfood/package.jsonRepository: GetStream/stream-video-js
Length of output: 92
🏁 Script executed:
# Check the actual git diff for the package.json file to see what changed
git diff HEAD~1 sample-apps/react-native/dogfood/package.json 2>/dev/null || git show HEAD:sample-apps/react-native/dogfood/package.json | head -30Repository: GetStream/stream-video-js
Length of output: 1272
🏁 Script executed:
# Get the previous version by looking at the parent commit
git show HEAD~1:sample-apps/react-native/dogfood/package.json 2>/dev/null | grep -A 1 -B 1 "react-native-webrtc" || echo "Could not find previous version"Repository: GetStream/stream-video-js
Length of output: 100
🏁 Script executed:
# Try to get the diff showing what changed
git diff HEAD~1 HEAD -- sample-apps/react-native/dogfood/package.json 2>/dev/null | head -50Repository: GetStream/stream-video-js
Length of output: 51
🏁 Script executed:
# Let's check the git log with more detail to understand the change history
git log -p --follow -2 -- sample-apps/react-native/dogfood/package.json | grep -A 5 -B 5 "react-native-webrtc" | head -30Repository: GetStream/stream-video-js
Length of output: 51
🏁 Script executed:
# Let's check if registerSDKGlobals.ts itself was recently modified
git log --oneline -10 packages/react-native-sdk/src/utils/internal/registerSDKGlobals.tsRepository: GetStream/stream-video-js
Length of output: 92
🏁 Script executed:
# Check the commit to understand what changed
git show aadf81bd --statRepository: GetStream/stream-video-js
Length of output: 50382
The alpha dependency is functionally required but bundled with feature code.
Line 25 introduces version 137.1.4-alpha.8, which is needed for audioDeviceModuleEvents.addSpeechActivityListener() used in packages/react-native-sdk/src/utils/internal/registerSDKGlobals.ts (lines 92–96). While the version bump appears necessary, it's bundled with feature code in the same commit. Following the principle of keeping dependency upgrades isolated, consider documenting this hard requirement explicitly in the PR description or changelog to clarify that the alpha version is a hard blocker for this feature rather than an exploratory upgrade.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sample-apps/react-native/dogfood/package.json` at line 25, The dependency
bump to "@stream-io/react-native-webrtc": "137.1.4-alpha.8" is a required, not
exploratory, change because the feature in
packages/react-native-sdk/src/utils/internal/registerSDKGlobals.ts depends on
audioDeviceModuleEvents.addSpeechActivityListener(); update the PR description
and changelog to state that this alpha version is a hard requirement for that
feature (mention the specific function
audioDeviceModuleEvents.addSpeechActivityListener and file
registerSDKGlobals.ts) so reviewers understand the upgrade is intentional and
isolated from other feature changes.
# Conflicts: # packages/react-native-sdk/package.json # sample-apps/react-native/dogfood/ios/Podfile.lock # sample-apps/react-native/dogfood/package.json # yarn.lock
3 participants
💡 Overview
Old mechanism - polling through rtc stats
Now - speech events pushed from native side
Depends on GetStream/react-native-webrtc#29
Summary by CodeRabbit