From ddeba12f93a2931dfa91a5427e6ac3f631d9e079 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Mon, 13 Apr 2026 21:17:38 -0400 Subject: [PATCH 1/4] feat(core): enhance loop detection with thought, read-file, and stagnation checks Adds three new detectors to LoopDetectionService, complementing the existing CONSECUTIVE_IDENTICAL_TOOL_CALLS detector: - REPETITIVE_THOUGHTS: fires when the same thought-like phrase appears THOUGHT_REPEAT_THRESHOLD times across the tracked content history. Runs on Content events (previously only on ToolCallRequest, so it could never fire). - READ_FILE_LOOP: fires when FILE_READ_THRESHOLD read-like tools (read, cat, view, list) appear within a bounded window. - ACTION_STAGNATION: fires when the same tool *name* is called STAGNATION_THRESHOLD times consecutively regardless of arguments. Distinct from CONSECUTIVE_IDENTICAL_TOOL_CALLS (same name AND args) and from READ_FILE_LOOP (catches thrashing with varying params). Shared helper isReadLikeTool() deduplicates read-category checks. --- .../src/services/loopDetectionService.test.ts | 181 ++++++++++++++++++ .../core/src/services/loopDetectionService.ts | 169 +++++++++++++++- packages/core/src/telemetry/types.ts | 3 + 3 files changed, 349 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 31a8699dc4b..4fcab68befd 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -619,4 +619,185 @@ describe('LoopDetectionService', () => { expect(service.addAndCheck(otherEvent)).toBe(false); }); }); + + describe('Repetitive Thoughts Detection', () => { + it('should detect repetitive thoughts pattern', () => { + service.reset(''); + + // Simulate repetitive thinking patterns + for (let i = 0; i < 3; i++) { + service.addAndCheck( + createContentEvent('Thinking about the problem...'), + ); + } + + const isLoop = service.addAndCheck( + createContentEvent('Still thinking about the same problem...'), + ); + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: 'repetitive_thoughts', + }), + ); + }); + + it('should not detect loop with varied thoughts', () => { + service.reset(''); + + service.addAndCheck(createContentEvent('Thinking about the problem...')); + service.addAndCheck(createContentEvent('Considering the solution...')); + service.addAndCheck(createContentEvent('Analyzing the requirements...')); + + const isLoop = service.addAndCheck( + createContentEvent('Now evaluating alternatives...'), + ); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + }); + + describe('Read File Loop Detection', () => { + it('should detect excessive file read operations', () => { + service.reset(''); + + // FILE_READ_THRESHOLD reads in the window trigger the loop. The first + // (THRESHOLD - 1) calls must not fire; the THRESHOLD-th does. + for (let i = 0; i < 4; i++) { + const event = createToolCallRequestEvent('read_file', { + path: `file${i}.txt`, + }); + const isLoop = service.addAndCheck(event); + expect(isLoop).toBe(false); + } + + const event = createToolCallRequestEvent('read_file', { + path: 'file4.txt', + }); + const isLoop = service.addAndCheck(event); + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: 'read_file_loop', + }), + ); + }); + + it('should detect other read-like operations', () => { + service.reset(''); + + // Mix of different read operations + service.addAndCheck( + createToolCallRequestEvent('cat_file', { path: 'file1.txt' }), + ); + service.addAndCheck( + createToolCallRequestEvent('view_file', { path: 'file2.txt' }), + ); + service.addAndCheck( + createToolCallRequestEvent('list_files', { dir: '.' }), + ); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file3.txt' }), + ); + + const isLoop = service.addAndCheck( + createToolCallRequestEvent('cat_file', { path: 'file4.txt' }), + ); + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: 'read_file_loop', + }), + ); + }); + + it('should not detect loop with mixed operations', () => { + service.reset(''); + + // Mix of read and non-read operations + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file1.txt' }), + ); + service.addAndCheck( + createToolCallRequestEvent('write_file', { + path: 'file2.txt', + content: 'test', + }), + ); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file3.txt' }), + ); + service.addAndCheck( + createToolCallRequestEvent('execute', { command: 'ls' }), + ); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file4.txt' }), + ); + + const isLoop = service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file5.txt' }), + ); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + }); + + describe('Action Stagnation Detection', () => { + // Stagnation fires when the same tool *name* is called STAGNATION_THRESHOLD + // times consecutively regardless of arguments. This is distinct from + // CONSECUTIVE_IDENTICAL_TOOL_CALLS (same name AND args) and from + // READ_FILE_LOOP (high proportion of read-like tools in the window), + // so we exercise it with a non-read-like tool and varying args. + it('should detect action stagnation when the same tool is repeated with varying args', () => { + service.reset(''); + + // STAGNATION_THRESHOLD - 1 calls must not fire + for (let i = 0; i < 7; i++) { + const isLoop = service.addAndCheck( + createToolCallRequestEvent('search_code', { query: `term${i}` }), + ); + expect(isLoop).toBe(false); + } + + // THRESHOLD-th consecutive same-name call triggers stagnation + const isLoop = service.addAndCheck( + createToolCallRequestEvent('search_code', { query: 'term7' }), + ); + expect(isLoop).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'action_stagnation' }), + ); + }); + + it('should reset stagnation streak when a different tool is called', () => { + service.reset(''); + + // Accumulate 5 consecutive same-name calls (below threshold) + for (let i = 0; i < 5; i++) { + service.addAndCheck( + createToolCallRequestEvent('search_code', { query: `a${i}` }), + ); + } + + // A different tool resets the streak + service.addAndCheck( + createToolCallRequestEvent('write_file', { + path: 'out.txt', + content: 'x', + }), + ); + + // 5 more calls of the original tool: streak only reaches 5, below threshold + for (let i = 0; i < 5; i++) { + const isLoop = service.addAndCheck( + createToolCallRequestEvent('search_code', { query: `b${i}` }), + ); + expect(isLoop).toBe(false); + } + }); + }); }); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index d14e4223e85..f58c3e8e0fd 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -23,6 +23,17 @@ const CONTENT_LOOP_THRESHOLD = 10; const CONTENT_CHUNK_SIZE = 50; const MAX_HISTORY_LENGTH = 1000; +// Thought tracking +const THOUGHT_REPEAT_THRESHOLD = 3; +const MAX_THOUGHT_HISTORY = 50; + +// File read tracking +const FILE_READ_THRESHOLD = 5; +const FILE_READ_WINDOW = 10; + +// Action stagnation tracking +const STAGNATION_THRESHOLD = 8; + /** * Service for detecting and preventing infinite loops in AI responses. * Monitors tool call repetitions and content sentence repetitions. @@ -45,6 +56,19 @@ export class LoopDetectionService { // Session-level disable flag private disabledForSession = false; + // Thought tracking + private thoughtHistory: string[] = []; + + // Tool call tracking (for read-file loop + stagnation detection) + private recentToolCalls: Array<{ name: string; args: object }> = []; + + // Action stagnation tracking: consecutive calls to the same tool *name* + // (regardless of args). Distinct from checkToolCallLoop, which requires + // identical name AND args. This catches parameter-thrashing loops where + // the model keeps calling one tool with varying arguments. + private sameNameStreak = 0; + private lastSeenToolName: string | null = null; + constructor(config: Config) { this.config = config; } @@ -77,15 +101,31 @@ export class LoopDetectionService { } switch (event.type) { - case GeminiEventType.ToolCallRequest: + case GeminiEventType.ToolCallRequest: { // content chanting only happens in one single stream, reset if there // is a tool call in between this.resetContentTracking(); - this.loopDetected = this.checkToolCallLoop(event.value); + + const toolCallLoop = this.checkToolCallLoop(event.value); + this.trackToolCall(event.value); + const repetitiveThoughts = this.checkRepetitiveThoughts(); + const readFileLoop = this.checkReadFileLoop(); + const actionStagnation = this.checkActionStagnation(); + + this.loopDetected = + toolCallLoop || + repetitiveThoughts || + readFileLoop || + actionStagnation; break; - case GeminiEventType.Content: - this.loopDetected = this.checkContentLoop(event.value); + } + case GeminiEventType.Content: { + this.trackThoughtPatterns(event.value); + const repetitiveThoughts = this.checkRepetitiveThoughts(); + this.loopDetected = + repetitiveThoughts || this.checkContentLoop(event.value); break; + } default: break; } @@ -291,6 +331,121 @@ export class LoopDetectionService { return originalChunk === currentChunk; } + /** + * Tracks thought patterns in content for repetitive thoughts detection. + */ + private trackThoughtPatterns(content: string): void { + // Look for thought-like patterns in the content (e.g., internal reasoning) + const thoughtPattern = + /\b(thinking|considering|let me|need to|should|could|might|perhaps|maybe|likely|probably|possibly)\b/i; + if (thoughtPattern.test(content)) { + // Record every thought-like content fragment so repetition is visible. + // Earlier versions deduped consecutive identical entries, which made + // checkRepetitiveThoughts() impossible to trigger. + const simplifiedThought = content.toLowerCase().substring(0, 100).trim(); + this.thoughtHistory.push(simplifiedThought); + if (this.thoughtHistory.length > MAX_THOUGHT_HISTORY) { + this.thoughtHistory.shift(); + } + } + } + + /** + * Checks for repetitive thoughts pattern. + */ + private checkRepetitiveThoughts(): boolean { + if (this.thoughtHistory.length < THOUGHT_REPEAT_THRESHOLD) { + return false; + } + + // Fire if any thought appears >= THRESHOLD times in the history. The + // repeated thought doesn't need to be the most recent one — the model may + // rephrase slightly while still fixating on the same underlying idea. + const counts = new Map(); + for (const thought of this.thoughtHistory) { + const next = (counts.get(thought) ?? 0) + 1; + counts.set(thought, next); + if (next >= THOUGHT_REPEAT_THRESHOLD) { + logLoopDetected( + this.config, + new LoopDetectedEvent(LoopType.REPETITIVE_THOUGHTS, this.promptId), + ); + return true; + } + } + return false; + } + + private static readonly READ_LIKE_TOKENS = ['read', 'cat', 'view', 'list']; + + private isReadLikeTool(toolName: string): boolean { + return LoopDetectionService.READ_LIKE_TOKENS.some((token) => + toolName.includes(token), + ); + } + + /** + * Tracks tool calls for subsequent loop detection. + */ + private trackToolCall(toolCall: { name: string; args: object }): void { + // Add to recent tool calls history + this.recentToolCalls.push(toolCall); + + // Keep bounded history + if (this.recentToolCalls.length > FILE_READ_WINDOW) { + this.recentToolCalls.shift(); + } + + // Track same-name streak for action stagnation. Distinct from + // checkToolCallLoop which requires identical args; this detector catches + // "thrashing" where the same tool is called with varying arguments. + if (this.lastSeenToolName === toolCall.name) { + this.sameNameStreak++; + } else { + this.lastSeenToolName = toolCall.name; + this.sameNameStreak = 1; + } + } + + /** + * Checks for excessive file read operations without meaningful progress. + */ + private checkReadFileLoop(): boolean { + if (this.recentToolCalls.length < FILE_READ_THRESHOLD) { + return false; + } + + // Count how many of the recent tool calls were file reads + const fileReadCount = this.recentToolCalls.filter((call) => + this.isReadLikeTool(call.name), + ).length; + + if (fileReadCount >= FILE_READ_THRESHOLD) { + logLoopDetected( + this.config, + new LoopDetectedEvent(LoopType.READ_FILE_LOOP, this.promptId), + ); + return true; + } + + return false; + } + + /** + * Checks for action stagnation where the model performs different but equally unproductive actions. + */ + private checkActionStagnation(): boolean { + if (this.sameNameStreak >= STAGNATION_THRESHOLD) { + logLoopDetected( + this.config, + new LoopDetectedEvent(LoopType.ACTION_STAGNATION, this.promptId), + ); + return true; + } + + return false; + } + /** * Resets all loop detection state. */ @@ -299,6 +454,12 @@ export class LoopDetectionService { this.resetToolCallCount(); this.resetContentTracking(); this.loopDetected = false; + + // Reset new tracking variables + this.thoughtHistory = []; + this.recentToolCalls = []; + this.sameNameStreak = 0; + this.lastSeenToolName = null; } private resetToolCallCount(): void { diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 9cef68ce49a..c3666ae9a94 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -383,6 +383,9 @@ export class RipgrepFallbackEvent implements BaseTelemetryEvent { export enum LoopType { CONSECUTIVE_IDENTICAL_TOOL_CALLS = 'consecutive_identical_tool_calls', CHANTING_IDENTICAL_SENTENCES = 'chanting_identical_sentences', + REPETITIVE_THOUGHTS = 'repetitive_thoughts', + READ_FILE_LOOP = 'read_file_loop', + ACTION_STAGNATION = 'action_stagnation', } export class LoopDetectedEvent implements BaseTelemetryEvent { From 98dadb346962b9c670afdfb80074fc0752b1b16e Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 18 Apr 2026 10:12:14 -0400 Subject: [PATCH 2/4] fix(core): address review feedback on loop detection - checkRepetitiveThoughts: scope to a consecutive window of the last THOUGHT_REPEAT_THRESHOLD thoughts instead of counting across the full retained history, so a long-running session that revisits an earlier phrase after genuine progress is no longer flagged as a loop. - isReadLikeTool: replace loose substring matching on 'read'/'cat'/ 'view'/'list' with an exact-name allowlist (read_file, read_many_files, list_directory) plus a read_/list_ prefix fallback for MCP-provided tools. Prevents unrelated tools like 'review' or 'listener_bind' from contributing to READ_FILE_LOOP detection. - coreToolScheduler._schedule: prune validationRetryCounts per-tool rather than wholesale. Stale counters for tools absent from the current batch were surviving and could fire RETRY LOOP DETECTED prematurely the next time an unrelated tool was used. Adds regression tests for each case. --- .../core/src/core/coreToolScheduler.test.ts | 146 ++++++++++++++++++ packages/core/src/core/coreToolScheduler.ts | 24 +-- .../src/services/loopDetectionService.test.ts | 62 +++++++- .../core/src/services/loopDetectionService.ts | 53 +++++-- 4 files changed, 250 insertions(+), 35 deletions(-) diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index f7be11c5a0b..1c2f9fa2a10 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -4110,4 +4110,150 @@ describe('CoreToolScheduler validation retry loop detection', () => { expect(msg).toBeDefined(); expect(msg).not.toContain(RETRY_LOOP_STOP_DIRECTIVE); }); + + it('should isolate retry counters per-tool across batches', async () => { + // Regression: the batch-level continues-loop check used to keep *all* + // retry state whenever any current request matched a previously failing + // tool. That let stale counts for an unrelated tool survive long enough + // to fire RETRY LOOP DETECTED prematurely the next time that tool was + // called. The correct behaviour prunes counters per-tool: keep only + // counters whose tool name actually appears in the current batch. + class StrictToolAlt extends BaseDeclarativeTool< + { other: string }, + ToolResult + > { + static readonly Name = 'strictStringToolAlt'; + constructor() { + super( + StrictToolAlt.Name, + 'StrictStringToolAlt', + 'Alt tool requiring string other param.', + Kind.Other, + { + type: 'object', + properties: { other: { type: 'string' } }, + required: ['other'], + }, + ); + } + protected createInvocation(params: { + other: string; + }): ToolInvocation<{ other: string }, ToolResult> { + return new (class extends BaseToolInvocation< + { other: string }, + ToolResult + > { + constructor(p: { other: string }) { + super(p); + } + getDescription() { + return 'strictStringToolAlt invocation'; + } + async execute(): Promise { + return { llmContent: 'ok', returnDisplay: 'ok' }; + } + })(params); + } + } + + const toolA = new StrictStringTool(); + const toolB = new StrictToolAlt(); + const mockToolRegistry = { + ensureTool: async (name: string) => + name === StrictStringTool.Name + ? toolA + : name === StrictToolAlt.Name + ? toolB + : undefined, + getTool: (name: string) => + name === StrictStringTool.Name + ? toolA + : name === StrictToolAlt.Name + ? toolB + : undefined, + getFunctionDeclarations: () => [], + tools: new Map(), + discovery: {}, + registerTool: () => {}, + getToolByName: (name: string) => + name === StrictStringTool.Name + ? toolA + : name === StrictToolAlt.Name + ? toolB + : undefined, + getToolByDisplayName: () => undefined, + getTools: () => [], + discoverTools: async () => {}, + getAllTools: () => [], + getAllToolNames: () => [StrictStringTool.Name, StrictToolAlt.Name], + getToolsByServer: () => [], + } as unknown as ToolRegistry; + + const mockConfig = { + getSessionId: () => 'test-session-id', + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + getApprovalMode: () => ApprovalMode.YOLO, + getPermissionsAllow: () => [], + getContentGeneratorConfig: () => ({ + model: 'test-model', + authType: 'gemini', + }), + getShellExecutionConfig: () => ({ + terminalWidth: 90, + terminalHeight: 30, + }), + storage: { getProjectTempDir: () => '/tmp' }, + getTruncateToolOutputThreshold: () => 100, + getTruncateToolOutputLines: () => 10, + getToolRegistry: () => mockToolRegistry, + getUseModelRouter: () => false, + getGeminiClient: () => null, + isInteractive: () => true, + getIdeMode: () => false, + getExperimentalZedIntegration: () => false, + getChatRecordingService: () => undefined, + getMessageBus: vi.fn().mockReturnValue(undefined), + getDisableAllHooks: vi.fn().mockReturnValue(true), + setApprovalMode: vi.fn(), + } as unknown as Config; + + const onToolCallsUpdate = vi.fn(); + const scheduler = new CoreToolScheduler({ + config: mockConfig, + onAllToolCallsComplete: vi.fn(), + onToolCallsUpdate, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + // Tool A fails twice, accumulating a retry count of 2. + await scheduler.schedule( + [makeRequest('a1', StrictStringTool.Name, { value: 123 })], + new AbortController().signal, + ); + await scheduler.schedule( + [makeRequest('a2', StrictStringTool.Name, { value: 123 })], + new AbortController().signal, + ); + + // Now a batch for tool B only — tool A's counter must be pruned because + // A is not present in this batch. + await scheduler.schedule( + [makeRequest('b1', StrictToolAlt.Name, { other: 456 })], + new AbortController().signal, + ); + + // Tool A fails once more. Under the old wholesale-keep behaviour this + // would be the third consecutive A failure and would trip the directive. + // Under per-tool pruning the counter starts fresh at 1 and no directive + // should be emitted. + await scheduler.schedule( + [makeRequest('a3', StrictStringTool.Name, { value: 123 })], + new AbortController().signal, + ); + const msg = getLastErrorMessage(onToolCallsUpdate); + expect(msg).toBeDefined(); + expect(msg).not.toContain(RETRY_LOOP_STOP_DIRECTIVE); + }); }); diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 99944fd5d76..7b78ee351b9 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -794,20 +794,20 @@ export class CoreToolScheduler { } const requestsToProcess = Array.isArray(request) ? request : [request]; - // Check if this batch continues a validation retry loop. - // Keys are ":"; if no request reuses a tool name - // that previously failed validation, reset the tracker. + // Prune validation retry state per-tool, not wholesale. Keys are + // ":"; retain counters only for tools actually + // present in the current batch. Keeping every tracked tool's counters + // whenever any current request matched caused stale counts for + // unrelated tools to survive and fire RETRY LOOP DETECTED prematurely + // the next time those tools were used. if (this.validationRetryCounts.size > 0) { - const prevTools = new Set(); - for (const key of this.validationRetryCounts.keys()) { + const currentToolNames = new Set(requestsToProcess.map((r) => r.name)); + for (const key of [...this.validationRetryCounts.keys()]) { const sep = key.indexOf(':'); - prevTools.add(sep === -1 ? key : key.slice(0, sep)); - } - const hasPrevFailingTool = requestsToProcess.some((r) => - prevTools.has(r.name), - ); - if (!hasPrevFailingTool) { - this.validationRetryCounts.clear(); + const toolName = sep === -1 ? key : key.slice(0, sep); + if (!currentToolNames.has(toolName)) { + this.validationRetryCounts.delete(key); + } } } diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 4fcab68befd..27d3cec1c45 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -656,6 +656,26 @@ describe('LoopDetectionService', () => { expect(isLoop).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); }); + + it('should not detect a loop when an earlier thought reappears after progress', () => { + service.reset(''); + + // Regression: earlier counting-based implementation fired as soon as + // any thought appeared >= THRESHOLD times anywhere in the retained + // history. A healthy long-running session where the model revisits + // the same phrase after making progress on unrelated steps should + // *not* trip this detector — only a sustained consecutive run does. + service.addAndCheck(createContentEvent('Thinking about the schema.')); + service.addAndCheck(createContentEvent('Considering the migration.')); + service.addAndCheck(createContentEvent('Analyzing the indexes.')); + service.addAndCheck(createContentEvent('Thinking about the schema.')); + service.addAndCheck(createContentEvent('Considering rollout risks.')); + const isLoop = service.addAndCheck( + createContentEvent('Thinking about the schema.'), + ); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); }); describe('Read File Loop Detection', () => { @@ -685,25 +705,29 @@ describe('LoopDetectionService', () => { ); }); - it('should detect other read-like operations', () => { + it('should detect other read-like operations (exact names + read_/list_ prefixes)', () => { service.reset(''); - // Mix of different read operations + // Mix of read-like tool names that either appear in the exact allowlist + // (read_file, read_many_files, list_directory) or match the read_/list_ + // prefix fallback used for MCP-provided tools. service.addAndCheck( - createToolCallRequestEvent('cat_file', { path: 'file1.txt' }), + createToolCallRequestEvent('read_many_files', { + paths: ['file1.txt'], + }), ); service.addAndCheck( - createToolCallRequestEvent('view_file', { path: 'file2.txt' }), + createToolCallRequestEvent('list_directory', { path: '.' }), ); service.addAndCheck( - createToolCallRequestEvent('list_files', { dir: '.' }), + createToolCallRequestEvent('read_resource', { uri: 'a' }), ); service.addAndCheck( createToolCallRequestEvent('read_file', { path: 'file3.txt' }), ); const isLoop = service.addAndCheck( - createToolCallRequestEvent('cat_file', { path: 'file4.txt' }), + createToolCallRequestEvent('list_projects', {}), ); expect(isLoop).toBe(true); expect(loggers.logLoopDetected).toHaveBeenCalledWith( @@ -714,6 +738,32 @@ describe('LoopDetectionService', () => { ); }); + it('should not treat tools that merely contain read-like substrings as file reads', () => { + service.reset(''); + + // Regression: the earlier substring heuristic treated any name + // containing 'read'/'cat'/'view'/'list' as a file read, so `review` + // (contains 'view') and `concat_chunks` (contains 'cat') contributed + // to READ_FILE_LOOP even though no file-read loop was happening. + const nonReadLikeNames = [ + 'review', + 'concat_chunks', + 'viewport_set', + 'listener_bind', + ]; + for (let i = 0; i < 6; i++) { + const name = nonReadLikeNames[i % nonReadLikeNames.length]; + const isLoop = service.addAndCheck( + createToolCallRequestEvent(name, { i }), + ); + expect(isLoop).toBe(false); + } + expect(loggers.logLoopDetected).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'read_file_loop' }), + ); + }); + it('should not detect loop with mixed operations', () => { service.reset(''); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index f58c3e8e0fd..10cfb2b0f65 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -352,35 +352,54 @@ export class LoopDetectionService { /** * Checks for repetitive thoughts pattern. + * + * Only fires when the last `THOUGHT_REPEAT_THRESHOLD` thoughts are the same + * string. Earlier implementations counted repeats across the full retained + * history, which caused false positives whenever the model revisited an + * earlier phrase after making progress on an unrelated step. */ private checkRepetitiveThoughts(): boolean { if (this.thoughtHistory.length < THOUGHT_REPEAT_THRESHOLD) { return false; } - // Fire if any thought appears >= THRESHOLD times in the history. The - // repeated thought doesn't need to be the most recent one — the model may - // rephrase slightly while still fixating on the same underlying idea. - const counts = new Map(); - for (const thought of this.thoughtHistory) { - const next = (counts.get(thought) ?? 0) + 1; - counts.set(thought, next); - if (next >= THOUGHT_REPEAT_THRESHOLD) { - logLoopDetected( - this.config, - new LoopDetectedEvent(LoopType.REPETITIVE_THOUGHTS, this.promptId), - ); - return true; - } + const recentThoughts = this.thoughtHistory.slice(-THOUGHT_REPEAT_THRESHOLD); + const firstThought = recentThoughts[0]; + if (recentThoughts.every((thought) => thought === firstThought)) { + logLoopDetected( + this.config, + new LoopDetectedEvent(LoopType.REPETITIVE_THOUGHTS, this.promptId), + ); + return true; } return false; } - private static readonly READ_LIKE_TOKENS = ['read', 'cat', 'view', 'list']; + // Exact tool names that read content from the filesystem. A plain substring + // match on tokens like "view" or "list" is unsafe because unrelated tools + // (e.g. "review", "checklist_update") can incidentally contain those + // tokens and get miscounted as file reads. + private static readonly READ_LIKE_TOOL_NAMES: ReadonlySet = new Set([ + 'read_file', + 'read_many_files', + 'list_directory', + ]); + + // Prefix fallback for MCP-provided tools that follow the same naming + // convention (e.g. `read_resource`, `list_projects`). The trailing + // underscore anchors the match to a name segment so "review" and + // "listener" are not treated as read-like. + private static readonly READ_LIKE_NAME_PREFIXES: readonly string[] = [ + 'read_', + 'list_', + ]; private isReadLikeTool(toolName: string): boolean { - return LoopDetectionService.READ_LIKE_TOKENS.some((token) => - toolName.includes(token), + if (LoopDetectionService.READ_LIKE_TOOL_NAMES.has(toolName)) { + return true; + } + return LoopDetectionService.READ_LIKE_NAME_PREFIXES.some((prefix) => + toolName.startsWith(prefix), ); } From 78c8f436993b3b7e5ae9f4f76b4e5cfc9e38a485 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sun, 19 Apr 2026 00:51:05 -0400 Subject: [PATCH 3/4] fix(core): hook Thought events for repetition, clear thoughts between tool calls - trackThoughtPatterns previously hooked GeminiEventType.Content and flagged hedge phrases ("should", "might", etc.) via a substring regex. That never actually sampled the model's reasoning channel, because structured thinking is emitted as GeminiEventType.Thought. Replace the Content hook with a proper Thought handler that stores a (subject, description) signature. - thoughtHistory was only cleared on reset(promptId), which runs on new user prompts. Within a single prompt, the model can drive many tool roundtrips; a thought like "Inspect the schema" reappearing three times across those roundtrips incorrectly fired REPETITIVE_THOUGHTS despite real progress in between. Clear thoughtHistory at every ToolCallRequest so repetition only fires within one contiguous reasoning stream. - Update tests to use Thought events, add a regression for the cross-roundtrip leak, and assert hedge phrases in Content no longer influence thought detection. Co-Authored-By: Claude Opus 4.7 --- .../src/services/loopDetectionService.test.ts | 84 +++++++++++++++---- .../core/src/services/loopDetectionService.ts | 49 +++++------ 2 files changed, 92 insertions(+), 41 deletions(-) diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 27d3cec1c45..15c6f280dca 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -9,6 +9,7 @@ import type { Config } from '../config/config.js'; import type { ServerGeminiContentEvent, ServerGeminiStreamEvent, + ServerGeminiThoughtEvent, ServerGeminiToolCallRequestEvent, } from '../core/turn.js'; import { GeminiEventType } from '../core/turn.js'; @@ -55,6 +56,14 @@ describe('LoopDetectionService', () => { value: content, }); + const createThoughtEvent = ( + subject: string, + description = '', + ): ServerGeminiThoughtEvent => ({ + type: GeminiEventType.Thought, + value: { subject, description }, + }); + const createRepetitiveContent = (id: number, length: number): string => { const baseString = `This is a unique sentence, id=${id}. `; let content = ''; @@ -114,7 +123,7 @@ describe('LoopDetectionService', () => { param: 'value', }); const otherEvent = { - type: 'thought', + type: GeminiEventType.UserCancelled, } as unknown as ServerGeminiStreamEvent; // Send events just below the threshold @@ -624,17 +633,12 @@ describe('LoopDetectionService', () => { it('should detect repetitive thoughts pattern', () => { service.reset(''); - // Simulate repetitive thinking patterns for (let i = 0; i < 3; i++) { service.addAndCheck( - createContentEvent('Thinking about the problem...'), + createThoughtEvent('Plan', 'Inspect the migration script.'), ); } - const isLoop = service.addAndCheck( - createContentEvent('Still thinking about the same problem...'), - ); - expect(isLoop).toBe(true); expect(loggers.logLoopDetected).toHaveBeenCalledWith( mockConfig, expect.objectContaining({ @@ -646,12 +650,16 @@ describe('LoopDetectionService', () => { it('should not detect loop with varied thoughts', () => { service.reset(''); - service.addAndCheck(createContentEvent('Thinking about the problem...')); - service.addAndCheck(createContentEvent('Considering the solution...')); - service.addAndCheck(createContentEvent('Analyzing the requirements...')); + service.addAndCheck(createThoughtEvent('Plan', 'Inspect the schema.')); + service.addAndCheck( + createThoughtEvent('Analysis', 'Check migration risks.'), + ); + service.addAndCheck( + createThoughtEvent('Plan', 'Evaluate rollout alternatives.'), + ); const isLoop = service.addAndCheck( - createContentEvent('Now evaluating alternatives...'), + createThoughtEvent('Next', 'Draft the fix.'), ); expect(isLoop).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); @@ -665,17 +673,59 @@ describe('LoopDetectionService', () => { // history. A healthy long-running session where the model revisits // the same phrase after making progress on unrelated steps should // *not* trip this detector — only a sustained consecutive run does. - service.addAndCheck(createContentEvent('Thinking about the schema.')); - service.addAndCheck(createContentEvent('Considering the migration.')); - service.addAndCheck(createContentEvent('Analyzing the indexes.')); - service.addAndCheck(createContentEvent('Thinking about the schema.')); - service.addAndCheck(createContentEvent('Considering rollout risks.')); + service.addAndCheck(createThoughtEvent('Plan', 'Inspect the schema.')); + service.addAndCheck( + createThoughtEvent('Analysis', 'Consider migration.'), + ); + service.addAndCheck(createThoughtEvent('Analysis', 'Review indexes.')); + service.addAndCheck(createThoughtEvent('Plan', 'Inspect the schema.')); + service.addAndCheck( + createThoughtEvent('Analysis', 'Consider rollout risks.'), + ); + const isLoop = service.addAndCheck( + createThoughtEvent('Plan', 'Inspect the schema.'), + ); + expect(isLoop).toBe(false); + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + + it('clears thought history across tool-call roundtrips within a turn', () => { + service.reset(''); + + // Regression: thoughtHistory previously persisted across ToolCallRequest + // events within a single prompt. Three identical thoughts separated by + // real tool-call progress would incorrectly fire REPETITIVE_THOUGHTS. + service.addAndCheck(createThoughtEvent('Plan', 'Inspect the schema.')); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'a.sql' }), + ); + service.addAndCheck(createThoughtEvent('Plan', 'Inspect the schema.')); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'b.sql' }), + ); const isLoop = service.addAndCheck( - createContentEvent('Thinking about the schema.'), + createThoughtEvent('Plan', 'Inspect the schema.'), ); expect(isLoop).toBe(false); expect(loggers.logLoopDetected).not.toHaveBeenCalled(); }); + + it('ignores hedge phrases in Content events (thought detection is Thought-only)', () => { + service.reset(''); + + // Content events used to feed a substring-matched hedge-phrase list + // into thoughtHistory, which conflated prose with the model's actual + // reasoning channel. Thought detection now runs only on Thought events. + for (let i = 0; i < 5; i++) { + service.addAndCheck( + createContentEvent('I should check the config, maybe it helps.'), + ); + } + expect(loggers.logLoopDetected).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'repetitive_thoughts' }), + ); + }); }); describe('Read File Loop Detection', () => { diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index 10cfb2b0f65..37dba9e231e 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -7,6 +7,7 @@ import { createHash } from 'node:crypto'; import type { ServerGeminiStreamEvent } from '../core/turn.js'; import { GeminiEventType } from '../core/turn.js'; +import type { ThoughtSummary } from '../utils/thoughtUtils.js'; import { logLoopDetected, logLoopDetectionDisabled, @@ -105,25 +106,26 @@ export class LoopDetectionService { // content chanting only happens in one single stream, reset if there // is a tool call in between this.resetContentTracking(); + // Thought repetition is only meaningful within a single contiguous + // reasoning stream. Once a tool call lands, the model has made + // observable progress — any prior thoughts should not carry over. + this.thoughtHistory = []; const toolCallLoop = this.checkToolCallLoop(event.value); this.trackToolCall(event.value); - const repetitiveThoughts = this.checkRepetitiveThoughts(); const readFileLoop = this.checkReadFileLoop(); const actionStagnation = this.checkActionStagnation(); - this.loopDetected = - toolCallLoop || - repetitiveThoughts || - readFileLoop || - actionStagnation; + this.loopDetected = toolCallLoop || readFileLoop || actionStagnation; break; } case GeminiEventType.Content: { - this.trackThoughtPatterns(event.value); - const repetitiveThoughts = this.checkRepetitiveThoughts(); - this.loopDetected = - repetitiveThoughts || this.checkContentLoop(event.value); + this.loopDetected = this.checkContentLoop(event.value); + break; + } + case GeminiEventType.Thought: { + this.trackThought(event.value); + this.loopDetected = this.checkRepetitiveThoughts(); break; } default: @@ -332,21 +334,20 @@ export class LoopDetectionService { } /** - * Tracks thought patterns in content for repetitive thoughts detection. + * Records a structured thought summary for repetition detection. Uses both + * subject and description so two thoughts with the same subject but + * diverging descriptions are correctly treated as distinct progress. */ - private trackThoughtPatterns(content: string): void { - // Look for thought-like patterns in the content (e.g., internal reasoning) - const thoughtPattern = - /\b(thinking|considering|let me|need to|should|could|might|perhaps|maybe|likely|probably|possibly)\b/i; - if (thoughtPattern.test(content)) { - // Record every thought-like content fragment so repetition is visible. - // Earlier versions deduped consecutive identical entries, which made - // checkRepetitiveThoughts() impossible to trigger. - const simplifiedThought = content.toLowerCase().substring(0, 100).trim(); - this.thoughtHistory.push(simplifiedThought); - if (this.thoughtHistory.length > MAX_THOUGHT_HISTORY) { - this.thoughtHistory.shift(); - } + private trackThought(summary: ThoughtSummary): void { + const subject = summary.subject.trim().toLowerCase(); + const description = summary.description + .trim() + .toLowerCase() + .substring(0, 200); + const signature = `${subject}|${description}`; + this.thoughtHistory.push(signature); + if (this.thoughtHistory.length > MAX_THOUGHT_HISTORY) { + this.thoughtHistory.shift(); } } From b1bd1a64cffe8bce73510dd84ac49010c73b5b4c Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sun, 19 Apr 2026 05:37:50 -0400 Subject: [PATCH 4/4] fix(core): exempt cold-start reads and surface loop reason in non-interactive mode Addresses PR #3236 review feedback: 1. Raise FILE_READ_THRESHOLD/FILE_READ_WINDOW to 8/15 and add a cold-start exemption (hasSeenNonReadTool gate) so opening exploration (list_directory + parallel read_file calls) no longer trips READ_FILE_LOOP on its first productive move. 2. In non-interactive TEXT mode, print a one-liner to stderr when a loop is detected, naming which detector fired. Previously the run exited silently with empty stdout. Routed via a new getLastLoopType() on LoopDetectionService and an optional value.loopType on ServerGeminiLoopDetectedEvent. Co-Authored-By: Claude Opus 4.7 --- packages/cli/src/nonInteractiveCli.ts | 39 ++++++++ packages/core/src/core/client.ts | 6 +- packages/core/src/core/turn.ts | 7 ++ packages/core/src/index.ts | 1 + .../src/services/loopDetectionService.test.ts | 98 ++++++++++++++++++- .../core/src/services/loopDetectionService.ts | 56 ++++++++++- 6 files changed, 198 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index b8f8e04f482..dc90c77756b 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -20,6 +20,7 @@ import { promptIdContext, OutputFormat, InputFormat, + LoopType, uiTelemetryService, parseAndFormatApiError, createDebugLogger, @@ -51,6 +52,38 @@ import { computeUsageFromMetrics, } from './utils/nonInteractiveHelpers.js'; +// Human-readable labels for the detectors that can fire mid-stream. +// Surfaced to stderr in TEXT mode so a headless run that halts on a loop +// doesn't exit with empty stdout and no explanation — see PR #3236 review. +const LOOP_TYPE_LABELS: Record = { + [LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS]: + 'the model repeated the same tool call with identical arguments', + [LoopType.CHANTING_IDENTICAL_SENTENCES]: + 'the model repeated the same sentence in its output', + [LoopType.REPETITIVE_THOUGHTS]: + 'the model repeated the same reasoning thought', + [LoopType.READ_FILE_LOOP]: + 'the model spent too many consecutive calls reading files without making progress', + [LoopType.ACTION_STAGNATION]: + 'the model kept calling the same tool without making progress', +}; + +function emitLoopDetectedMessage( + config: Config, + loopType: LoopType | undefined, +): void { + // In TEXT mode the adapter swallows LoopDetected, so we print here. In + // JSON modes the adapter emits a structured result, which is enough. + if (config.getOutputFormat() !== OutputFormat.TEXT) { + return; + } + const reason = loopType ? LOOP_TYPE_LABELS[loopType] : undefined; + const detail = reason ? ` (${loopType}: ${reason})` : ''; + process.stderr.write( + `Loop detection halted the run${detail}. Set the \`model.skipLoopDetection\` setting to true to disable.\n`, + ); +} + /** * Emits a final message for slash command results. * Note: systemMessage should already be emitted before calling this function. @@ -340,6 +373,9 @@ export async function runNonInteractive( if (event.type === GeminiEventType.ToolCallRequest) { toolCallRequests.push(event.value); } + if (event.type === GeminiEventType.LoopDetected) { + emitLoopDetectedMessage(config, event.value?.loopType); + } if ( outputFormat === OutputFormat.TEXT && event.type === GeminiEventType.Error @@ -506,6 +542,9 @@ export async function runNonInteractive( if (event.type === GeminiEventType.ToolCallRequest) { itemToolCallRequests.push(event.value); } + if (event.type === GeminiEventType.LoopDetected) { + emitLoopDetectedMessage(config, event.value?.loopType); + } if ( outputFormat === OutputFormat.TEXT && event.type === GeminiEventType.Error diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index e3a93e7847e..c58b3db731b 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -883,7 +883,11 @@ export class GeminiClient { for await (const event of resultStream) { if (!this.config.getSkipLoopDetection()) { if (this.loopDetector.addAndCheck(event)) { - yield { type: GeminiEventType.LoopDetected }; + const loopType = this.loopDetector.getLastLoopType(); + yield { + type: GeminiEventType.LoopDetected, + ...(loopType && { value: { loopType } }), + }; if (arenaAgentClient) { await arenaAgentClient.reportError('Loop detected'); } diff --git a/packages/core/src/core/turn.ts b/packages/core/src/core/turn.ts index 5e918c72cb2..78e8dac24f6 100644 --- a/packages/core/src/core/turn.ts +++ b/packages/core/src/core/turn.ts @@ -33,6 +33,7 @@ import { parseThought, type ThoughtSummary, } from '../utils/thoughtUtils.js'; +import type { LoopType } from '../telemetry/types.js'; // Define a structure for tools passed to the server export interface ServerTool { @@ -194,6 +195,12 @@ export type ServerGeminiFinishedEvent = { export type ServerGeminiLoopDetectedEvent = { type: GeminiEventType.LoopDetected; + // The loop type is optional so historical call sites that don't produce one + // (tests, fixtures) stay valid. Real emissions in client.ts always populate + // it so downstream consumers can surface a concrete reason to the user. + value?: { + loopType: LoopType; + }; }; export type ServerGeminiCitationEvent = { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b4eead082d3..5d3b632476c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -227,6 +227,7 @@ export { ExtensionUninstallEvent, IdeConnectionEvent, IdeConnectionType, + LoopType, ModelSlashCommandEvent, PromptSuggestionEvent, SpeculationEvent, diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 15c6f280dca..2d0ba5e3d67 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -24,6 +24,9 @@ vi.mock('../telemetry/loggers.js', () => ({ const TOOL_CALL_LOOP_THRESHOLD = 5; const CONTENT_LOOP_THRESHOLD = 10; const CONTENT_CHUNK_SIZE = 50; +// Mirrored from loopDetectionService.ts. Kept local so the test is +// self-describing and failures point to the constant that changed. +const FILE_READ_WINDOW = 15; describe('LoopDetectionService', () => { let service: LoopDetectionService; @@ -729,12 +732,26 @@ describe('LoopDetectionService', () => { }); describe('Read File Loop Detection', () => { + // Cold-start exemption: a prompt that has not yet fired any non-read-like + // tool is still in its opening-exploration phase, so the detector gives + // it an initial pass. Tests that want to exercise the detector must + // fire a non-read tool first so subsequent reads are judged normally. + const primeNonReadTool = () => { + service.addAndCheck( + createToolCallRequestEvent('write_file', { + path: 'prime.txt', + content: '', + }), + ); + }; + it('should detect excessive file read operations', () => { service.reset(''); + primeNonReadTool(); // FILE_READ_THRESHOLD reads in the window trigger the loop. The first - // (THRESHOLD - 1) calls must not fire; the THRESHOLD-th does. - for (let i = 0; i < 4; i++) { + // (THRESHOLD - 1) reads must not fire; the THRESHOLD-th does. + for (let i = 0; i < 7; i++) { const event = createToolCallRequestEvent('read_file', { path: `file${i}.txt`, }); @@ -743,7 +760,7 @@ describe('LoopDetectionService', () => { } const event = createToolCallRequestEvent('read_file', { - path: 'file4.txt', + path: 'file7.txt', }); const isLoop = service.addAndCheck(event); expect(isLoop).toBe(true); @@ -755,8 +772,65 @@ describe('LoopDetectionService', () => { ); }); + it('should exempt opening exploration from READ_FILE_LOOP (cold start)', () => { + service.reset(''); + + // Regression for PR #3236 review: a prompt like "summarize this + // project" opens with parallel read_file / list_directory calls and + // must not trip READ_FILE_LOOP before any write/execute action has + // fired. This exercises FILE_READ_WINDOW+ consecutive reads with no + // prior non-read tool — nothing should fire. + for (let i = 0; i < 20; i++) { + const name = i % 2 === 0 ? 'read_file' : 'list_directory'; + const isLoop = service.addAndCheck( + createToolCallRequestEvent(name, { path: `f${i}` }), + ); + expect(isLoop).toBe(false); + } + expect(loggers.logLoopDetected).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'read_file_loop' }), + ); + }); + + it('should activate READ_FILE_LOOP once a non-read tool lands mid-prompt', () => { + service.reset(''); + + // No firing before the cold-start gate flips. + for (let i = 0; i < 7; i++) { + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: `pre${i}.txt` }), + ); + } + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + + // A non-read tool lands — gate opens. + service.addAndCheck( + createToolCallRequestEvent('write_file', { + path: 'out.txt', + content: 'x', + }), + ); + + // Now a window of reads should eventually trip READ_FILE_LOOP. As new + // reads push the write_file out of the FILE_READ_WINDOW-sized history + // and FILE_READ_THRESHOLD read-likes accumulate, detection fires. + let detected = false; + for (let i = 0; i < FILE_READ_WINDOW + 2 && !detected; i++) { + detected = service.addAndCheck( + createToolCallRequestEvent('read_file', { path: `post${i}.txt` }), + ); + } + expect(detected).toBe(true); + expect(loggers.logLoopDetected).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'read_file_loop' }), + ); + }); + it('should detect other read-like operations (exact names + read_/list_ prefixes)', () => { service.reset(''); + primeNonReadTool(); // Mix of read-like tool names that either appear in the exact allowlist // (read_file, read_many_files, list_directory) or match the read_/list_ @@ -775,9 +849,18 @@ describe('LoopDetectionService', () => { service.addAndCheck( createToolCallRequestEvent('read_file', { path: 'file3.txt' }), ); + service.addAndCheck(createToolCallRequestEvent('list_projects', {})); + service.addAndCheck( + createToolCallRequestEvent('read_file', { path: 'file5.txt' }), + ); + service.addAndCheck( + createToolCallRequestEvent('read_many_files', { + paths: ['file6.txt'], + }), + ); const isLoop = service.addAndCheck( - createToolCallRequestEvent('list_projects', {}), + createToolCallRequestEvent('list_directory', { path: 'nested' }), ); expect(isLoop).toBe(true); expect(loggers.logLoopDetected).toHaveBeenCalledWith( @@ -790,6 +873,7 @@ describe('LoopDetectionService', () => { it('should not treat tools that merely contain read-like substrings as file reads', () => { service.reset(''); + primeNonReadTool(); // Regression: the earlier substring heuristic treated any name // containing 'read'/'cat'/'view'/'list' as a file read, so `review` @@ -816,6 +900,7 @@ describe('LoopDetectionService', () => { it('should not detect loop with mixed operations', () => { service.reset(''); + primeNonReadTool(); // Mix of read and non-read operations service.addAndCheck( @@ -841,7 +926,10 @@ describe('LoopDetectionService', () => { createToolCallRequestEvent('read_file', { path: 'file5.txt' }), ); expect(isLoop).toBe(false); - expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + expect(loggers.logLoopDetected).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ loop_type: 'read_file_loop' }), + ); }); }); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index 37dba9e231e..ed0006733b0 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -28,9 +28,18 @@ const MAX_HISTORY_LENGTH = 1000; const THOUGHT_REPEAT_THRESHOLD = 3; const MAX_THOUGHT_HISTORY = 50; -// File read tracking -const FILE_READ_THRESHOLD = 5; -const FILE_READ_WINDOW = 10; +// File read tracking. +// +// Thresholds were raised from 5/10 because a prompt like "summarize this +// project" legitimately opens with `list_directory` + several parallel +// `read_file` calls in a single turn, which previously tripped the detector +// on its first productive move. 8/15 leaves enough headroom for that shape +// while still catching pathological read-only churn. Combined with the +// cold-start exemption below (see `hasSeenNonReadTool`), a turn that has +// only ever performed read-like actions is treated as exploration, not a +// loop — once any non-read tool lands, the detector activates. +const FILE_READ_THRESHOLD = 8; +const FILE_READ_WINDOW = 15; // Action stagnation tracking const STAGNATION_THRESHOLD = 8; @@ -70,10 +79,29 @@ export class LoopDetectionService { private sameNameStreak = 0; private lastSeenToolName: string | null = null; + // Cold-start gate for READ_FILE_LOOP: the opening exploration of a prompt + // is almost always read-heavy (list + parallel reads). Until at least one + // non-read-like tool fires, a window full of reads is treated as legitimate + // exploration rather than loop evidence. Resets per-prompt in reset(). + private hasSeenNonReadTool = false; + + // Loop type of the most recent firing. Bubbled up through the + // LoopDetected event so callers (non-interactive CLI, telemetry) can tell + // the user which detector actually fired. + private lastLoopType: LoopType | null = null; + constructor(config: Config) { this.config = config; } + /** + * Returns the LoopType of the most recent detection, or null if no loop + * has been detected in the current prompt. + */ + getLastLoopType(): LoopType | null { + return this.lastLoopType; + } + /** * Disables loop detection for the current session. */ @@ -143,6 +171,7 @@ export class LoopDetectionService { this.toolCallRepetitionCount = 1; } if (this.toolCallRepetitionCount >= TOOL_CALL_LOOP_THRESHOLD) { + this.lastLoopType = LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS; logLoopDetected( this.config, new LoopDetectedEvent( @@ -256,6 +285,7 @@ export class LoopDetectionService { const chunkHash = createHash('sha256').update(currentChunk).digest('hex'); if (this.isLoopDetectedForChunk(currentChunk, chunkHash)) { + this.lastLoopType = LoopType.CHANTING_IDENTICAL_SENTENCES; logLoopDetected( this.config, new LoopDetectedEvent( @@ -367,6 +397,7 @@ export class LoopDetectionService { const recentThoughts = this.thoughtHistory.slice(-THOUGHT_REPEAT_THRESHOLD); const firstThought = recentThoughts[0]; if (recentThoughts.every((thought) => thought === firstThought)) { + this.lastLoopType = LoopType.REPETITIVE_THOUGHTS; logLoopDetected( this.config, new LoopDetectedEvent(LoopType.REPETITIVE_THOUGHTS, this.promptId), @@ -416,6 +447,13 @@ export class LoopDetectionService { this.recentToolCalls.shift(); } + // Flip the cold-start gate once any non-read-like tool has been observed. + // Opening exploration (list_directory + several read_file calls) should + // not count as loop evidence on its own. + if (!this.hasSeenNonReadTool && !this.isReadLikeTool(toolCall.name)) { + this.hasSeenNonReadTool = true; + } + // Track same-name streak for action stagnation. Distinct from // checkToolCallLoop which requires identical args; this detector catches // "thrashing" where the same tool is called with varying arguments. @@ -431,6 +469,14 @@ export class LoopDetectionService { * Checks for excessive file read operations without meaningful progress. */ private checkReadFileLoop(): boolean { + // Cold-start exemption: if no non-read-like tool has ever fired in this + // prompt, the model is still in its opening exploration phase. Treat a + // run of reads as legitimate discovery rather than a loop. Once any + // write/execute/other tool lands, normal detection resumes. + if (!this.hasSeenNonReadTool) { + return false; + } + if (this.recentToolCalls.length < FILE_READ_THRESHOLD) { return false; } @@ -441,6 +487,7 @@ export class LoopDetectionService { ).length; if (fileReadCount >= FILE_READ_THRESHOLD) { + this.lastLoopType = LoopType.READ_FILE_LOOP; logLoopDetected( this.config, new LoopDetectedEvent(LoopType.READ_FILE_LOOP, this.promptId), @@ -456,6 +503,7 @@ export class LoopDetectionService { */ private checkActionStagnation(): boolean { if (this.sameNameStreak >= STAGNATION_THRESHOLD) { + this.lastLoopType = LoopType.ACTION_STAGNATION; logLoopDetected( this.config, new LoopDetectedEvent(LoopType.ACTION_STAGNATION, this.promptId), @@ -480,6 +528,8 @@ export class LoopDetectionService { this.recentToolCalls = []; this.sameNameStreak = 0; this.lastSeenToolName = null; + this.hasSeenNonReadTool = false; + this.lastLoopType = null; } private resetToolCallCount(): void {