Fix issue #99: Enable silence flush regardless of hasSpeech state#137
Fix issue #99: Enable silence flush regardless of hasSpeech state#137
Conversation
📝 WalkthroughWalkthroughThe silence-based flush logic in App.tsx has been restructured to execute unconditionally before the no-speech guard. It now always queries silence duration and finalizes pending transcriptions via v4FinalizeTimeout when silence meets the threshold, followed by an early-return check if no speech is detected. Changes
Sequence DiagramsequenceDiagram
participant App as App Component
participant BC as BufferClient
participant WC as TranscriptionWorkerClient
participant W as Transcription Worker
App->>BC: getSilenceTailDuration('energyVad', 0.3)
BC-->>App: silence duration
alt Silence ≥ threshold AND hasPendingText
App->>WC: v4FinalizeTimeout()
WC->>W: V4_FINALIZE_TIMEOUT
W->>W: Promote pending text to mature
W-->>WC: Updated transcript
WC-->>App: Finalization complete
App->>App: Update cursor, transcript state
end
alt hasSpeech == false
App->>App: Return early<br/>(skip further transcription)
else hasSpeech == true
App->>App: Continue transcription flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Review NotesThe changes in this PR correctly implement the fix described in the PR description:
The code is clean, well-commented, and the debug logging is appropriate for troubleshooting. No issues found that would block merging. |
![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
🤖 Fix all issues with AI agents
In `@src/App.tsx`:
- Around line 356-358: The current hasPendingText uses getMatureCursorFrame() >
0 which checks cursor advancement rather than actual unflushed text; change the
logic to test the real pending sentence instead (e.g., set hasPendingText =
Boolean(windowBuilder.pendingSentence) or add/use a
windowBuilder.hasPendingText() accessor) and use that in the silenceDuration
check so v4FinalizeTimeout() is only triggered when there is actual pending text
to flush.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/App.tsx
🔇 Additional comments (1)
src/App.tsx (1)
353-385: Core reordering logic correctly fixes the silence-flush gating bug.Moving the silence check before the
!hasSpeechguard, and then using!hasSpeechas an early return after the flush attempt, correctly addresses the reported issue wherehasSpeechwas preventing silence flush from ever triggering. The structure is clean: flush first, then gate transcription.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const hasPendingText = windowBuilder.getMatureCursorFrame() > 0; | ||
|
|
||
| if (silenceDuration >= appStore.v4SilenceFlushSec() && hasPendingText) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find getMatureCursorFrame implementation and usage
echo "=== Searching for getMatureCursorFrame ==="
rg -n 'getMatureCursorFrame' --type=ts -B2 -A2
echo ""
echo "=== Searching for v4FinalizeTimeout ==="
rg -n 'v4FinalizeTimeout' --type=ts -B2 -A2
echo ""
echo "=== Searching for immatureText in appStore ==="
rg -n 'immatureText' --type=ts -B2 -A2Repository: ysdede/keet
Length of output: 9609
🏁 Script executed:
#!/bin/bash
# Look at the V4_FINALIZE_TIMEOUT handler in the worker
echo "=== Searching for V4_FINALIZE_TIMEOUT handler in worker ==="
rg -n 'V4_FINALIZE_TIMEOUT' --type=ts -B3 -A15
echo ""
echo "=== Check the utteranceMerger.timeoutFlush() method ==="
rg -n 'timeoutFlush' --type=ts -B2 -A10
echo ""
echo "=== Check how immatureText is cleared ==="
rg -n 'setImmatureText|immatureText = ' --type=ts -C1Repository: ysdede/keet
Length of output: 4674
🏁 Script executed:
#!/bin/bash
# Find the finalizePendingSentenceByTimeout method
echo "=== UtteranceBasedMerger.finalizePendingSentenceByTimeout ==="
rg -n 'finalizePendingSentenceByTimeout' --type=ts -B2 -A20Repository: ysdede/keet
Length of output: 6691
🏁 Script executed:
#!/bin/bash
# Check what happens to pendingSentence after finalization
echo "=== finalizePendingSentenceByTimeout full implementation ==="
rg -n 'finalizePendingSentenceByTimeout' --type=ts -A40
echo ""
echo "=== Check if pendingSentence is cleared ==="
rg -n 'this.pendingSentence = ' --type=ts -C2Repository: ysdede/keet
Length of output: 12160
hasPendingText uses incorrect semantics — consider checking actual pending text for clarity.
getMatureCursorFrame() > 0 checks whether the cursor has ever advanced, not whether pending text currently exists. While repeated calls to v4FinalizeTimeout() are guarded by the worker returning null after the first successful finalization (which clears this.pendingSentence), the condition itself is semantically misleading.
For clarity and maintainability, check actual pending text:
- const hasPendingText = windowBuilder.getMatureCursorFrame() > 0;
+ const hasPendingText = (appStore.immatureText() || '').length > 0;This makes the intent explicit: only attempt flush if there's actually unflushed text waiting to be finalized.
🤖 Prompt for AI Agents
In `@src/App.tsx` around lines 356 - 358, The current hasPendingText uses
getMatureCursorFrame() > 0 which checks cursor advancement rather than actual
unflushed text; change the logic to test the real pending sentence instead
(e.g., set hasPendingText = Boolean(windowBuilder.pendingSentence) or add/use a
windowBuilder.hasPendingText() accessor) and use that in the silenceDuration
check so v4FinalizeTimeout() is only triggered when there is actual pending text
to flush.
1 participant
Summary
Resolves an issue where the silence-based buffer flushing mechanism failed to trigger.
Problem
The logic for flushing the buffer was nested within a negated
hasSpeechconditional block. However,hasSpeechconsistently evaluated totrueeven during periods of silence because it validated the entire buffer range, including historical transcribed data. Consequently, the silence threshold was never reached.Solution
Refactored the logic within the
v4Tickfunction inApp.tsxto prioritize silence detection:hasSpeechstate. The system now monitors silence duration from the trailing buffer independently.v4SilenceFlushSecthreshold and pending text exists, the buffer is flushed to finalize the transcription.hasSpeechto determine if the transcription process should remain active.Impact:
Related Issues
Summary by CodeRabbit