Add bidirectional realtime translation#132
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds bidirectional translation: persisted settings and UI, RealtimeTranslation and AdvancedPanel wiring, TypeScript contract updates, and iOS WebRTC dual-client orchestration with per-stream role and noise-reduction config. ChangesBidirectional Translation Feature
Sequence DiagramsequenceDiagram
participant User
participant App as App (index.tsx)
participant ConfigUI as ConfigureTranslation
participant RTComponent as RealtimeTranslation
participant AdvPanel as AdvancedPanel
participant Settings as translationSettings
participant iOS as iOS Module (primaryClient, reverseClient)
User->>ConfigUI: Opens settings
ConfigUI->>Settings: loadBidirectionalLanguage
Settings-->>ConfigUI: persisted language
User->>ConfigUI: Selects language
ConfigUI->>Settings: saveBidirectionalLanguage
ConfigUI->>App: onBidirectionalLanguageChange
App->>App: update bidirectionalLanguage state
User->>RTComponent: Start translation session
RTComponent->>Settings: loadBidirectionalEnabled
Settings-->>RTComponent: persisted enabled flag
RTComponent->>RTComponent: compute intendedStreams
RTComponent->>iOS: openTranslationConnectionAsync(..., bidirectionalLanguage?)
iOS->>iOS: open primaryClient(outputLanguage)
iOS->>iOS: open reverseClient(bidirectionalLanguage, skipAudioSession=true)
iOS-->>RTComponent: connection established
AdvPanel->>RTComponent: onToggleBidirectional
RTComponent->>Settings: saveBidirectionalEnabled
iOS->>RTComponent: emit transcript delta (role=primary/reverse)
RTComponent->>User: display transcript
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
OSSF Scorecard (PR vs base)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/RealtimeTranslation.tsx (2)
243-302:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce
near_fieldbefore opening a bidirectional session.Per the PR objective, bidirectional translation only works with Near Field noise reduction. This path still opens bidirectional mode with whatever
loadTranslationNoiseReductionType()returns, so a user can switch tofar_fieldordisabledand land in a broken/echo-prone session. Guard this beforeopenTranslationConnectionAsync()or force the supported mode.Possible guard
const [noiseReductionType, transcriptionEnabled, transcriptionModel] = await Promise.all([ loadTranslationNoiseReductionType(), loadTranslationInputTranscriptionEnabled(), loadTranslationInputTranscriptionModel(), ]); + + if (isBidirectional && noiseReductionType !== "near_field") { + Alert.alert( + "Bidirectional translation", + "Bidirectional mode requires Near Field noise reduction. Update this in Configure before starting the session.", + ); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/RealtimeTranslation.tsx` around lines 243 - 302, The bidirectional flow must enforce Near Field noise reduction before opening the connection: after loading noiseReductionType (from loadTranslationNoiseReductionType()) and before calling openTranslationConnectionAsync(), check if isBidirectional is true and noiseReductionType !== "near_field" and then either (a) override noiseReductionType to "near_field" and log an info/warn noting the enforced change, or (b) reject/start-fail with a clear error log and avoid calling openTranslationConnectionAsync(); apply this change around the existing symbols noiseReductionType, isBidirectional, and openTranslationConnectionAsync so the connection is only opened with near_field when bidirectional is requested.
141-177:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRoute transcript deltas by
payload.role.In bidirectional mode, these listeners still append every delta into the same
inputTranscript/outputTranscriptbuffers even though the payload now carriesprimaryvssecondary. That will interleave the forward and reverse conversations into one transcript stream. Please keep separate state per role, or explicitly ignore the secondary stream until the UI can render it cleanly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/RealtimeTranslation.tsx` around lines 141 - 177, The onTranslationInputTranscript and onTranslationOutputTranscript listeners currently append every payload.delta into the shared setInputTranscript/setOutputTranscript buffers regardless of payload.role; update those listeners to route by payload.role (e.g., check payload.role === "primary" vs "secondary") and either (a) only append to the existing primary buffers when role === "primary" (ignore secondary until UI supports it) or (b) maintain separate state vars (e.g., inputTranscriptPrimary, inputTranscriptSecondary, outputTranscriptPrimary, outputTranscriptSecondary) and update the appropriate setter based on payload.role; keep the same separator logic and call resetIdleTimer as before, and refer to VmWebrtcTranslatorModule.addListener, onTranslationInputTranscript, onTranslationOutputTranscript, setInputTranscript, setOutputTranscript, and TranslationTranscriptEventPayload when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/RealtimeTranslation.tsx`:
- Around line 373-376: The bidirectional toggle currently only updates state via
handleToggleBidirectional (setIsBidirectional + saveBidirectionalEnabled) but
does not reconfigure the live native connection, so it must not imply immediate
effect during an active session; update the UI logic to disable the switch when
isSessionActive is true (or show a persistent inline message like "applies next
session") and ensure handleToggleBidirectional remains a no-op for live
reconfigure; locate the toggle rendering and references around
handleToggleBidirectional and the related UI code (also change the same behavior
in the code block around lines where the other toggle logic lives ~401-409) to
prevent changing the control while isSessionActive.
---
Outside diff comments:
In `@app/RealtimeTranslation.tsx`:
- Around line 243-302: The bidirectional flow must enforce Near Field noise
reduction before opening the connection: after loading noiseReductionType (from
loadTranslationNoiseReductionType()) and before calling
openTranslationConnectionAsync(), check if isBidirectional is true and
noiseReductionType !== "near_field" and then either (a) override
noiseReductionType to "near_field" and log an info/warn noting the enforced
change, or (b) reject/start-fail with a clear error log and avoid calling
openTranslationConnectionAsync(); apply this change around the existing symbols
noiseReductionType, isBidirectional, and openTranslationConnectionAsync so the
connection is only opened with near_field when bidirectional is requested.
- Around line 141-177: The onTranslationInputTranscript and
onTranslationOutputTranscript listeners currently append every payload.delta
into the shared setInputTranscript/setOutputTranscript buffers regardless of
payload.role; update those listeners to route by payload.role (e.g., check
payload.role === "primary" vs "secondary") and either (a) only append to the
existing primary buffers when role === "primary" (ignore secondary until UI
supports it) or (b) maintain separate state vars (e.g., inputTranscriptPrimary,
inputTranscriptSecondary, outputTranscriptPrimary, outputTranscriptSecondary)
and update the appropriate setter based on payload.role; keep the same separator
logic and call resetIdleTimer as before, and refer to
VmWebrtcTranslatorModule.addListener, onTranslationInputTranscript,
onTranslationOutputTranscript, setInputTranscript, setOutputTranscript, and
TranslationTranscriptEventPayload when making the change.
🪄 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 Plus
Run ID: bcfb229f-e010-4cf6-a753-117fca774865
📒 Files selected for processing (11)
README.mdapp/RealtimeTranslation.tsxapp/index.tsxcomponents/realtime/AdvancedPanel.tsxcomponents/settings/ConfigureTranslation.tsxlib/translationSettings.tsmodules/vm-webrtc/ios/OpenAIWebRTCCore.swiftmodules/vm-webrtc/ios/OpenAIWebRTCTranslatorClient.swiftmodules/vm-webrtc/ios/WebRTCTranslatorModule.swiftmodules/vm-webrtc/ios/WebRtcClientHelpers.swiftmodules/vm-webrtc/src/VmWebrtcTranslatorModule.ts
Comment on lines
+373
to
+376
| const handleToggleBidirectional = useCallback((nextValue: boolean) => { | ||
| setIsBidirectional(nextValue); | ||
| void saveBidirectionalEnabled(nextValue); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Don't let the bidirectional switch imply a live reconfigure.
handleToggleBidirectional() only updates React state and persisted prefs; it never reopens or reconfigures the current native connection. During an active session, the switch can move to ON/OFF while the live stream topology stays unchanged until the next start. Disable this control while isSessionActive, or show explicit "applies next session" feedback.
Also applies to: 401-409
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/RealtimeTranslation.tsx` around lines 373 - 376, The bidirectional toggle
currently only updates state via handleToggleBidirectional (setIsBidirectional +
saveBidirectionalEnabled) but does not reconfigure the live native connection,
so it must not imply immediate effect during an active session; update the UI
logic to disable the switch when isSessionActive is true (or show a persistent
inline message like "applies next session") and ensure handleToggleBidirectional
remains a no-op for live reconfigure; locate the toggle rendering and references
around handleToggleBidirectional and the related UI code (also change the same
behavior in the code block around lines where the other toggle logic lives
~401-409) to prevent changing the control while isSessionActive.
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.
Only works when using Near Field noise reduction, so I made that the default setting
Summary by CodeRabbit
New Features
Documentation
Behavior