Skip to content

Comments

fix: refactor TranscriptionDisplay and fix buffer safety in App.tsx#138

Closed
ysdede wants to merge 1 commit intoaudit/perf-degradation-resource-mgmtfrom
fix/transcription-display-refactor-buffer-safety-3897330223291647915
Closed

fix: refactor TranscriptionDisplay and fix buffer safety in App.tsx#138
ysdede wants to merge 1 commit intoaudit/perf-degradation-resource-mgmtfrom
fix/transcription-display-refactor-buffer-safety-3897330223291647915

Conversation

@ysdede
Copy link
Owner

@ysdede ysdede commented Feb 14, 2026

This PR addresses two issues:

  1. Performance/Stability in UI: TranscriptionDisplay was using MutationObserver to trigger auto-scroll. This has been replaced with a SolidJS createEffect on confirmedText and pendingText props, which is more efficient and idiomatic. The requestAnimationFrame throttling for the actual scroll action is preserved.
  2. Critical Bug in Audio Pipeline: In App.tsx, the audio chunk buffer was being transferred to the TenVADWorker via processTransfer. This detached the buffer from the main thread, causing AudioEngine (which owns the buffer and uses it for visualization and ring-buffer writing) to potentially access an invalid/empty buffer. The call was changed to process, which internally copies the buffer before transferring the copy, ensuring the original buffer remains valid for AudioEngine.

PR created automatically by Jules for task 3897330223291647915 started by @ysdede

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>
@kiloconnect
Copy link

kiloconnect bot commented Feb 14, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • src/App.tsx - TEN-VAD buffer handling fix
  • src/components/TranscriptionDisplay.tsx - Auto-scroll refactor

Changes Overview:

  1. App.tsx: Changed from processTransfer() to process() for TEN-VAD worker to preserve the original audio buffer for AudioEngine visualization/ring-buffer usage. The new comment clearly explains the rationale.

  2. TranscriptionDisplay.tsx: Refactored from MutationObserver-based auto-scroll to SolidJS createEffect. This is a cleaner approach that automatically tracks dependencies and doesn't require manual cleanup (SolidJS handles effect disposal on component unmount).

Both changes appear to be valid improvements with no security vulnerabilities, runtime errors, or logic issues detected.

Repository owner deleted a comment from google-labs-jules bot Feb 17, 2026
Repository owner deleted a comment from coderabbitai bot Feb 17, 2026
@ysdede
Copy link
Owner Author

ysdede commented Feb 18, 2026

FYI: PR #143 includes related transcript/UI performance containment changes (batched v4 updates + merged-view recompute gating) and links this PR as related context.

@ysdede
Copy link
Owner Author

ysdede commented Feb 20, 2026

Superseded by PR #134 which includes optimal scrolling dependencies.

@ysdede ysdede closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant