fix: refactor TranscriptionDisplay and fix buffer safety in App.tsx#138
Conversation
Refactored `TranscriptionDisplay` to use `createEffect` instead of `MutationObserver` for auto-scrolling, eliminating unnecessary DOM observation overhead and potential layout thrashing while retaining `requestAnimationFrame` for smooth scrolling. Fixed a critical safety issue in `App.tsx` where `tenVADClient.processTransfer` was detaching the audio buffer used by `AudioEngine`. Switched to `tenVADClient.process` which creates a safe copy before transfer, preventing potential crashes or data loss in the audio pipeline. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Changes Overview:
Both changes appear to be valid improvements with no security vulnerabilities, runtime errors, or logic issues detected. |
|
FYI: PR #143 includes related transcript/UI performance containment changes (batched v4 updates + merged-view recompute gating) and links this PR as related context. |
|
Superseded by PR #134 which includes optimal scrolling dependencies. |
This PR addresses two issues:
TranscriptionDisplaywas usingMutationObserverto trigger auto-scroll. This has been replaced with a SolidJScreateEffectonconfirmedTextandpendingTextprops, which is more efficient and idiomatic. TherequestAnimationFramethrottling for the actual scroll action is preserved.App.tsx, the audio chunk buffer was being transferred to theTenVADWorkerviaprocessTransfer. This detached the buffer from the main thread, causingAudioEngine(which owns the buffer and uses it for visualization and ring-buffer writing) to potentially access an invalid/empty buffer. The call was changed toprocess, which internally copies the buffer before transferring the copy, ensuring the original buffer remains valid forAudioEngine.PR created automatically by Jules for task 3897330223291647915 started by @ysdede