Fix meeting echo - #2542
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Code Review Summary
PR #2542 adds a layered defense against speaker bleed in Clips meetings: an acoustic Rust guard suppresses likely playback from the mic before Whisper inference, while the TypeScript transcript engine handles cross-stream echo, late-arriving finals, historical transcript loading, and live partial rendering. The overall architecture is sound, and the native capture changes preserve pending audio timing and keep the guard fail-open for double-talk. The transcript persistence refactor also keeps speaker labels, timestamps, and stored segments together in a clearer model. Risk level: Standard, because this changes core meeting transcription behavior and persistence.
Key Findings
- 🟡 MEDIUM — The text fallback can discard a legitimate user reply that repeats a recent system phrase within the broad 15-second window.
- 🟡 MEDIUM — A mixed echo-plus-interruption mic final can be removed wholesale when its matching remote words exceed the similarity threshold, despite the acoustic layer intentionally preserving double-talk.
The PR includes strong unit coverage for mangled echo, overlap, partials, history, timing, and acoustic thresholds. These two cases need additional safeguards/tests before approval.
🧪 Browser testing: Will run after this review (PR touches UI code)
| if (line.source === "mic") { | ||
| if (isMicEcho(line.text, lines, line.startMs)) return false; |
There was a problem hiding this comment.
🟡 Mic replies that repeat recent system speech are silently discarded
isMicEcho classifies any mic final matching a recent system final as echo when their start times are within 15 seconds; it does not require actual overlap or acoustic evidence. A user repeating a four-word instruction a few seconds later can therefore be dropped from the persisted transcript, contradicting the stated quick-reply behavior. Require stronger temporal/echo evidence for this fallback or retain completed mic finals, and add a delayed identical-reply test.
| const sameUtterance = | ||
| matched / Math.max(words.length, run.length) >= ECHO_MATCH_RATIO; |
There was a problem hiding this comment.
🟡 Text fallback deletes mixed echo-and-interruption mic utterances
The text fallback can remove an entire mic line containing both speaker bleed and a real interruption when the matching remote words reach the 65% threshold. This loses the user’s words even though the Rust guard deliberately keeps double-talk; the text layer cannot separate the mixed utterance, so it should retain the line unless the match is near-complete, with a regression test for a matching remote prefix plus new user words.
Visual recap — generation failedThe visual recap could not be generated for this pull request. This is informational only and does not block the PR. Diagnostic: No plan URL: Repair changed too much of targeted file plan.mdx; expected a localized parser fix. Agent output: Repaired the malformed line-9 |
Stops Clips from adding the other person's speech to meeting notes twice when audio is playing through speakers. It also makes sure the user's response is captured when they speak over someone or reply without waiting for a pause.
What changed