fix(core): ensure loop detection respects session disable flag#12347
Conversation
Summary of ChangesHello @SandyTao520, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue within the core Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a logic flaw in the loop detection service where disabling loop detection for a session did not take effect if a loop had already been detected. The fix correctly reorders the checks to prioritize the disabledForSession flag, ensuring that loop detection can be successfully disabled at any point during a session. The change is accompanied by a new test case that specifically validates this corrected behavior. The implementation is sound and effectively resolves the issue.
|
Size Change: +44 B (0%) Total Size: 20.3 MB ℹ️ View Unchanged
|
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
thanks for fixing! I've been hitting this issue a lot tonight.
|
Thanks for the update |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
4 participants
TLDR
Fixes an issue where loop detection would continue to report true even after being disabled for the session, if a loop had already been detected.
Dive Deeper
The
addAndCheckmethod inLoopDetectionServicewas checkingthis.loopDetected || this.disabledForSessionand returningthis.loopDetectedif it was true, ignoring thedisabledForSessionflag. The fix reorders these checks to prioritizedisabledForSession.Reviewer Test Plan
Run the newly added test case in
packages/core/src/services/loopDetectionService.test.tswhich specifically reproduces this scenario.Testing Matrix
Linked issues / bugs
N/A