fix(core): prevent duplicate function-call yields from trailing stream chunks#2065
Closed
yiliang114 wants to merge 4 commits intomainfrom
Closed
fix(core): prevent duplicate function-call yields from trailing stream chunks#2065yiliang114 wants to merge 4 commits intomainfrom
yiliang114 wants to merge 4 commits intomainfrom
Conversation
- Add separate retry budget for InvalidStreamError (NO_FINISH_REASON/NO_RESPONSE_TEXT) - Fix hasFinishReason logic to prevent usage-only chunks from overwriting existing finish reason - Update tests to use fake timers for retry delay handling - Add test for trailing usage-only chunk handling Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Contributor
📋 Review SummaryThis PR introduces an independent retry budget for transient stream anomalies (NO_FINISH_REASON / NO_RESPONSE_TEXT) to address intermittent model stream failures. It also fixes an issue where 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This was referenced Mar 4, 2026
…m chunks After the merged finish+usage response was yielded, handleChunkMerging's hasPendingFinish flag stayed true due to reference-shared candidates. Trailing empty chunks would trigger another merge, yielding the same functionCall parts again and causing duplicate tool executions in the UI. Add a finishYielded guard in processStreamWithLogging so that once the merged finish response is yielded, all subsequent chunks skip merging entirely and only absorb late-arriving usage metadata. Closes #2121
Collaborator
Author
|
Split into two separate PRs for cleaner review:
|
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.
TLDR
Fix duplicate function-call yields caused by trailing stream chunks, and introduce an independent retry budget for transient stream anomalies (empty responses / missing finish reason).
Core issue: Some OpenAI-compatible providers (e.g.
bailian/glm-5) send extra trailing empty chunks after the finish+usage pair. ThehasPendingFinishstate inhandleChunkMergingwas not cleared after the merged response was yielded (due to reference-sharedcandidates), so every trailing chunk re-triggered the merge and produced another yield — resulting in duplicate tool-call executions in the UI.Dive Deeper
Bug 1: Duplicate function-call yields (pipeline.ts)
Root cause: State leak in the chunk-merging logic of
processStreamWithLogging:finishReason+functionCall→ held pending, not yieldedcollectedGeminiResponsesstill hasfinishReason(reference assignmentmergedResponse.candidates = lastResponse.candidates) →hasPendingFinishremains true → triggers another merge and yield ❌Fix: Add a
finishYieldedflag inprocessStreamWithLogging. Once the merged finish response is yielded, set the flag to true. All subsequent chunks skiphandleChunkMergingentirely, only absorbing late-arriving usage metadata without producing further yields.Bug 2: Transient stream anomalies consuming the wrong retry budget (geminiChat.ts)
Root cause:
InvalidStreamError(NO_FINISH_REASON / NO_RESPONSE_TEXT) shared the same retry budget as content validation errors, so one type of failure could exhaust retries for the other.Fix:
INVALID_STREAM_RETRY_CONFIG(max 5 retries, 2s initial delay with linear backoff)attempt--compensation)hasFinishReasonassignment to||=to prevent later usage-only chunks from overwriting a previously detected finish reasonTest improvements (geminiChat.test.ts)
vi.useFakeTimers()interacting with async generatorsReviewer Test Plan
npx vitest run packages/core/src/core/openaiContentGenerator/pipeline.test.ts— pipeline tests including the new duplicate function-call testnpx vitest run packages/core/src/core/geminiChat.test.ts— geminiChat tests including stabilized retry testsnpm run build— confirm clean buildbailian/glm-5) to verify tool calls are no longer duplicatedTesting Matrix
Linked issues / bugs
Fixes #2121