feat: terminate model output loops with split content/reasoning thresholds - #994
Open
sofwanwork wants to merge 1 commit into
Open
feat: terminate model output loops with split content/reasoning thresholds#994sofwanwork wants to merge 1 commit into
sofwanwork wants to merge 1 commit into
Conversation
…holds
A looping upstream stream burns account quota and the client context window
until the response budget is exhausted. Track consecutive identical deltas
and terminate the stream once a channel exceeds its ceiling.
Content and reasoning keep separate counters. high/xhigh effort reasoning
legitimately repeats the same short marker ("so", "hmm", "wait", list
bullets) far more often than visible output does, so a single shared
counter truncates valid deep-thinking responses. Reasoning is therefore
allowed a higher ceiling (256) than visible content (128).
The content ceiling also has to clear legitimate visible repetition:
markdown horizontal rules and ASCII table borders stream as long runs of
an identical single-character delta ("-", "=", "|").
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A looping upstream stream currently runs until the response budget is exhausted. There is no repetition guard in
ConvertResponseStream, so a model that gets stuck emitting the same delta burns account quota and fills the client context window before anything stops it.The naive fix — one shared repeat counter — is worse than no fix for thinking models.
high/xhigheffort reasoning legitimately repeats the same short marker ("so","hmm","wait", list bullets) many times in a row while thinking. A single counter tuned low enough to catch a real content loop truncates valid deep-thinking responses instead.What changed
Consecutive identical deltas are tracked per channel, and the stream terminates once a channel exceeds its own ceiling:
textDelta)reasoningSummaryDelta,emitReasoningDelta)Two details worth reviewing:
reasoningSummaryDelta(buffered summaries) andemitReasoningDelta(raw reasoning) both route throughtrackReasoningRepeat, so a loop that straddles summary and raw events is still caught. The error messages stay distinct for diagnosis."-","=","|"), and wide tables with empty cells repeat the same separator delta. A ceiling near typical rule width would kill correct output, which is why 128 rather than something tighter.Counters reset whenever the delta changes, so alternating or interleaved output cannot accumulate into a false positive. Empty deltas are ignored.
Tests
backend/internal/infra/provider/conversation/stream_doomloop_test.gocovers both directions:model output loop detectedThe threshold-relative tests derive their repeat counts from the constants and assert the invariant, so they stay meaningful if the ceilings are retuned.
Verified on this branch:
go build ./...,go vet, and the fullgo test ./...suite (62 packages, 0 failures).