fix(core): stop streaming request on loop detection#8377
Merged
SandyTao520 merged 3 commits intomainfrom Sep 12, 2025
Merged
Conversation
|
Size Change: +161 B (0%) Total Size: 13.2 MB ℹ️ View Unchanged
|
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. |
jacob314
approved these changes
Sep 12, 2025
Contributor
jacob314
left a comment
There was a problem hiding this comment.
Can you add a regression test as a follow up?
cc65e0c to
db347c8
Compare
jacob314
reviewed
Sep 13, 2025
| expect(mockCheckNextSpeaker).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should create linked abort signal and pass it to turn.run', async () => { |
Contributor
There was a problem hiding this comment.
nit: this is a low value test as it checks the setup not the behavior. I would remove to leave more room for useful tests.
jacob314
reviewed
Sep 13, 2025
|
|
||
| // Assert | ||
| expect(events).toContainEqual({ type: GeminiEventType.LoopDetected }); | ||
| expect(client['loopDetector'].addAndCheck).toHaveBeenCalledTimes(2); |
Contributor
There was a problem hiding this comment.
sorry ELI5, how does this test detect that the abortSignal was actually called as expected with the desired result?
hoteye
added a commit
to hoteye/gemini-cli
that referenced
this pull request
Sep 13, 2025
- Integrate PR google-gemini#8377: fix(core): stop streaming request on loop detection - Integrate PR google-gemini#8373: JSON errors in non-interactive auth validation - Update i18n tests to match new translation string format with {label} placeholders - All tests passing after merge This brings the i18n branch up to date with latest upstream changes while maintaining the complete Phase 1 internationalization implementation.
reconsumeralization
pushed a commit
to reconsumeralization/gemini-cli
that referenced
this pull request
Sep 15, 2025
WangWanyue
pushed a commit
to WangWanyue/gemini-cli
that referenced
this pull request
Sep 15, 2025
thacio
added a commit
to thacio/auditaria
that referenced
this pull request
Oct 2, 2025
giraffe-tree
pushed a commit
to giraffe-tree/gemini-cli
that referenced
this pull request
Oct 10, 2025
cocosheng-g
pushed a commit
that referenced
this pull request
May 6, 2026
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
2 participants
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
This pull request fixes a bug where the streaming request to the model would continue even after a loop was detected. Now, when the
LoopDetectionServiceidentifies a repetitive pattern, it immediately aborts the underlying streaming request, preventing further unnecessary processing and token consumption.Dive Deeper
Previously, when
loopDetector.addAndCheck(event)returnedtrue, the system would yield aLoopDetectedevent and exit thesendMessageStreamgenerator. However, this action did not terminate the in-flight streaming request initiated byturn.run(). The model would continue to generate content, and the client would continue to receive events, even though they were no longer being processed.This change introduces an
AbortControllerwithin thesendMessageStreamfunction. The signal from this controller is combined with the original signal usingAbortSignal.any(), creating alinkedSignal. This new signal is then passed down to theturn.run()method, which handles the API call.When a loop is detected,
controller.abort()is now called. This immediately triggers thelinkedSignal, causing the underlyingfetchrequest in the@google/genaiSDK to be aborted. This ensures that the connection is closed promptly and resources are released, making the loop detection mechanism more efficient and robust.Reviewer Test Plan
Use network proxy and trigger a loop by asking it to say something non-stop, observe the network request is stopped when a loop is detected in the UI.
Testing Matrix
Linked issues / bugs
#8231