fix: stop feeding running ASR output back as Whisper initial prompt (repetition loops)#154
fix: stop feeding running ASR output back as Whisper initial prompt (repetition loops)#154NaturalPsi wants to merge 1 commit into
Conversation
Conditioning Whisper on its own prior output (previousTranscription) lets a single hallucinated phrase self-reinforce into a repetition loop across decode windows — the dominant failure mode for low-volume / quiet-speaker audio in Japanese. Drop the previousTranscription injection in both the local whisper provider and the Amical Cloud provider; vocabulary and cursor context (static, non-looping) are preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughBoth the Amical Cloud and Whisper transcription providers remove prior aggregated transcription context from their request and prompt building logic. The ChangesRemove prior transcription context from providers
🎯 2 (Simple) | ⏱️ ~8 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/pipeline/providers/transcription/whisper-provider.ts (1)
284-297: ⚡ Quick winRemove the now-unused
aggregatedTranscriptionparameter from the prompt helper signature.The behavior is correct, but keeping an intentionally-unused argument here leaves stale API intent and adds noise for future edits.
Suggested cleanup diff
- private generateInitialPrompt( - vocabulary?: readonly string[], - aggregatedTranscription?: string, - accessibilityContext?: TranscribeContext["accessibilityContext"], - ): string { + private generateInitialPrompt( + vocabulary?: readonly string[], + accessibilityContext?: TranscribeContext["accessibilityContext"], + ): string { @@ - void aggregatedTranscription; const prompt = buildWhisperPrompt({ vocabulary, previousTranscription: undefined, beforeText: accessibilityContext?.context?.textSelection?.preSelectionText, });const initialPrompt = this.generateInitialPrompt( context.vocabulary, - aggregatedTranscription, context.accessibilityContext, );🤖 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 `@apps/desktop/src/pipeline/providers/transcription/whisper-provider.ts` around lines 284 - 297, The generateInitialPrompt function currently declares an unused aggregatedTranscription parameter and explicitly voids it; remove aggregatedTranscription from the parameter list and delete the `void aggregatedTranscription;` statement inside generateInitialPrompt (function name: generateInitialPrompt) and update any call sites to stop passing that argument so the signature and usage remain consistent with buildWhisperPrompt's expected inputs; ensure TypeScript types compile after removing the parameter.
🤖 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.
Nitpick comments:
In `@apps/desktop/src/pipeline/providers/transcription/whisper-provider.ts`:
- Around line 284-297: The generateInitialPrompt function currently declares an
unused aggregatedTranscription parameter and explicitly voids it; remove
aggregatedTranscription from the parameter list and delete the `void
aggregatedTranscription;` statement inside generateInitialPrompt (function name:
generateInitialPrompt) and update any call sites to stop passing that argument
so the signature and usage remain consistent with buildWhisperPrompt's expected
inputs; ensure TypeScript types compile after removing the parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fc7c6c0-855c-4059-a726-162525c2c4ce
📒 Files selected for processing (2)
apps/desktop/src/pipeline/providers/transcription/amical-cloud-provider.tsapps/desktop/src/pipeline/providers/transcription/whisper-provider.ts
|
Hey @NaturalPsi thanks for flagging this. |
Problem
Whisper can fall into a repetition loop: once it emits a phrase, that phrase reappears every window until the audio ends. In our use this wiped out large stretches of real speech — a 56‑minute Japanese recording had minutes 20–47 replaced entirely by
はいはいはい…‑style loops, even though the audio was clear and well above the noise floor (the loop region measured −25 dB RMS vs −24 dB in a clean region, so it was not a volume problem).The trigger is self‑conditioning.
generateInitialPrompt()(local Whisper) and the Amical Cloud request both feed the running ASR output (previousTranscription/currentAggregatedTranscription) back in as the next window's prompt. When a hallucinated phrase slips in, re‑injecting it as context makes Whisper far more likely to emit it again, which re‑injects it again — a self‑reinforcing loop. whisper.cpp's ownno_contextdefault istrueprecisely to avoid this; the app re‑introduces the carry‑over at the application layer.Change
Stop feeding the running transcription back as the initial prompt, in both providers:
whisper-provider.ts–generateInitialPrompt()no longer passespreviousTranscription.amical-cloud-provider.ts– the request sendspreviousTranscription: undefined.Vocabulary and cursor context (
beforeText) are preserved — those are static and don't loop, so spelling/term hints still work.Verification
Built from this branch (local Whisper, large‑v3, Metal) and re‑transcribed the same recording: the previously‑lost minutes came back as clean Japanese with no repetition loops (~5.8× more real text recovered). Equivalent to running whisper.cpp with
no_context=true, which is its default.Tradeoff / open question
This is the minimal fix and disables cross‑window context entirely. There's a small coherence cost (the model no longer "remembers" the last few words across a long dictation). If you'd prefer to keep some context, happy to rework this as:
conditionOnPreviousText, default off), orLet me know which direction you'd prefer and I'll adjust.
Summary by CodeRabbit
Bug Fixes