From 8985e489a5fd5251f3b41fe358797d7a2e90ac6a Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Tue, 29 Jul 2025 21:05:03 -0700 Subject: [PATCH] Skip and reset loop checking around code blocks (#5144) --- .../src/services/loopDetectionService.test.ts | 125 ++++++++++++++++-- .../core/src/services/loopDetectionService.ts | 20 +++ 2 files changed, 134 insertions(+), 11 deletions(-) diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 9f5d63a7..2ec32ae7 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -56,6 +56,15 @@ describe('LoopDetectionService', () => { value: content, }); + const createRepetitiveContent = (id: number, length: number): string => { + const baseString = `This is a unique sentence, id=${id}. `; + let content = ''; + while (content.length < length) { + content += baseString; + } + return content.slice(0, length); + }; + describe('Tool Call Loop Detection', () => { it(`should not detect a loop for fewer than TOOL_CALL_LOOP_THRESHOLD identical calls`, () => { const event = createToolCallRequestEvent('testTool', { param: 'value' }); @@ -149,13 +158,11 @@ describe('LoopDetectionService', () => { it('should detect a loop when a chunk of content repeats consecutively', () => { service.reset(''); - const repeatedContent = 'a'.repeat(CONTENT_CHUNK_SIZE); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); let isLoop = false; for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { - for (const char of repeatedContent) { - isLoop = service.addAndCheck(createContentEvent(char)); - } + isLoop = service.addAndCheck(createContentEvent(repeatedContent)); } expect(isLoop).toBe(true); expect(loggers.logLoopDetected).toHaveBeenCalledTimes(1); @@ -163,23 +170,119 @@ describe('LoopDetectionService', () => { it('should not detect a loop if repetitions are very far apart', () => { service.reset(''); - const repeatedContent = 'b'.repeat(CONTENT_CHUNK_SIZE); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); const fillerContent = generateRandomString(500); let isLoop = false; for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { - for (const char of repeatedContent) { - isLoop = service.addAndCheck(createContentEvent(char)); - } - for (const char of fillerContent) { - isLoop = service.addAndCheck(createContentEvent(char)); - } + isLoop = service.addAndCheck(createContentEvent(repeatedContent)); + isLoop = service.addAndCheck(createContentEvent(fillerContent)); } expect(isLoop).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); }); }); + describe('Content Loop Detection with Code Blocks', () => { + it('should not detect a loop when repetitive content is inside a code block', () => { + service.reset(''); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); + + service.addAndCheck(createContentEvent('```\n')); + + for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { + const isLoop = service.addAndCheck(createContentEvent(repeatedContent)); + expect(isLoop).toBe(false); + } + + const isLoop = service.addAndCheck(createContentEvent('\n```')); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + + it('should detect a loop when repetitive content is outside a code block', () => { + service.reset(''); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); + + service.addAndCheck(createContentEvent('```')); + service.addAndCheck(createContentEvent('\nsome code\n')); + service.addAndCheck(createContentEvent('```')); + + let isLoop = false; + for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { + isLoop = service.addAndCheck(createContentEvent(repeatedContent)); + } + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledTimes(1); + }); + + it('should handle content with multiple code blocks and no loops', () => { + service.reset(''); + service.addAndCheck(createContentEvent('```\ncode1\n```')); + service.addAndCheck(createContentEvent('\nsome text\n')); + const isLoop = service.addAndCheck(createContentEvent('```\ncode2\n```')); + + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + + it('should handle content with mixed code blocks and looping text', () => { + service.reset(''); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); + + service.addAndCheck(createContentEvent('```')); + service.addAndCheck(createContentEvent('\ncode1\n')); + service.addAndCheck(createContentEvent('```')); + + let isLoop = false; + for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { + isLoop = service.addAndCheck(createContentEvent(repeatedContent)); + } + + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledTimes(1); + }); + + it('should not detect a loop for a long code block with some repeating tokens', () => { + service.reset(''); + const repeatingTokens = + 'for (let i = 0; i < 10; i++) { console.log(i); }'; + + service.addAndCheck(createContentEvent('```\n')); + + for (let i = 0; i < 20; i++) { + const isLoop = service.addAndCheck(createContentEvent(repeatingTokens)); + expect(isLoop).toBe(false); + } + + const isLoop = service.addAndCheck(createContentEvent('\n```')); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + + it('should reset tracking when a code fence is found', () => { + service.reset(''); + const repeatedContent = createRepetitiveContent(1, CONTENT_CHUNK_SIZE); + + for (let i = 0; i < CONTENT_LOOP_THRESHOLD - 1; i++) { + service.addAndCheck(createContentEvent(repeatedContent)); + } + + // This should not trigger a loop because of the reset + service.addAndCheck(createContentEvent('```')); + + // We are now in a code block, so loop detection should be off. + // Let's add the repeated content again, it should not trigger a loop. + let isLoop = false; + for (let i = 0; i < CONTENT_LOOP_THRESHOLD; i++) { + isLoop = service.addAndCheck(createContentEvent(repeatedContent)); + expect(isLoop).toBe(false); + } + + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + }); + describe('Edge Cases', () => { it('should handle empty content', () => { const event = createContentEvent(''); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index 7b3da20b..f71b8434 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -61,6 +61,7 @@ export class LoopDetectionService { private contentStats = new Map(); private lastContentIndex = 0; private loopDetected = false; + private inCodeBlock = false; // LLM loop track tracking private turnsInCurrentPrompt = 0; @@ -156,8 +157,27 @@ export class LoopDetectionService { * 2. Truncating history if it exceeds the maximum length * 3. Analyzing content chunks for repetitive patterns using hashing * 4. Detecting loops when identical chunks appear frequently within a short distance + * 5. Disabling loop detection within code blocks to prevent false positives, + * as repetitive code structures are common and not necessarily loops. */ private checkContentLoop(content: string): boolean { + // Code blocks can often contain repetitive syntax that is not indicative of a loop. + // To avoid false positives, we detect when we are inside a code block and + // temporarily disable loop detection. + const numFences = (content.match(/```/g) ?? []).length; + if (numFences) { + // Reset tracking when a code fence is detected to avoid analyzing content + // that spans across code block boundaries. + this.resetContentTracking(); + } + + const wasInCodeBlock = this.inCodeBlock; + this.inCodeBlock = + numFences % 2 === 0 ? this.inCodeBlock : !this.inCodeBlock; + if (wasInCodeBlock) { + return false; + } + this.streamContentHistory += content; this.truncateAndUpdate();