Fix decode loop aborting on EOT sampled during prompt prefill#502
Fix decode loop aborting on EOT sampled during prompt prefill#502hakanensari wants to merge 1 commit into
Conversation
When DecodingOptions.promptTokens is set, the main decode loop force-feeds the prompt (<|startofprev|> + prompt + SOT sequence) one token per step, and the token sampled at each of those steps is discarded on the next iteration. The completion check, however, still honored a sampled EOT mid-prefill: with large-v3 turbo the model reliably predicts <|endoftext|> while the SOT sequence is being forced, so the loop broke before decoding a single content token and every transcription with a prompt came back empty. Gate sampleResult.completed on !isPrefill so a throwaway EOT cannot complete the segment; a real EOT at or after the last prefill token still ends decoding as before.
There was a problem hiding this comment.
Pull request overview
This PR fixes a decoding bug in TextDecoder.decodeText where an <|endoftext|> token sampled during forced prompt prefill could prematurely terminate decoding, producing an empty transcription whenever DecodingOptions.promptTokens is set (Issue #501).
Changes:
- Prevent segment completion on sampled EOT while the decode loop is still consuming forced prefill tokens.
- Add an inline comment explaining why EOT during prefill must be ignored.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let isSegmentCompleted = | ||
| sampleResult.completed || | ||
| (sampleResult.completed && !isPrefill) || | ||
| currentTokens.count >= Constants.maxTokenContext - 1 || | ||
| isFirstTokenLogProbTooLow |
There was a problem hiding this comment.
A meaningful regression test here is less mechanical than it looks. The deterministic repro is on large-v3 turbo, while this suite's decoder tests run against the tiny model, and I have not confirmed that tiny samples EOT mid-prefill at all. A transcribe-with-promptTokens assertion on tiny could therefore pass even without the fix. The stub route (a TokenSampling returning completed=true during prefill plus canned predictLogits output) needs mock scaffolding the suite does not currently have. If the maintainers want either shape, I am happy to add it. Until then, the failure mode and decoder trace are documented in #501.
Fixes #501.
When
DecodingOptions.promptTokensis set, the decode loop force-feeds the prompt one token per step, and the token sampled at each of those steps is discarded on the next iteration. The completion check still honored a sampled<|endoftext|>during that forced prefill. Withlarge-v3 turbothe model reliably samples EOT while the SOT sequence is being forced, so the loop broke before decoding any content token, and every transcription with a prompt came back as an empty segment with no error.This change gates
sampleResult.completedon!isPrefill, so a throwaway EOT cannot complete the segment. A real EOT sampled at or after the last prefill token still ends decoding exactly as before.Verified on
openai_whisper-large-v3-v20240930_turbo(macOS 15, Apple Silicon): with this change, transcription withpromptTokensproduces correct text and the prompt biases spelling as intended. Without it, the same runs return an empty string. Full analysis and a decoder trace are in #501.