From 8fc176dd8ad2878dd5b1c362d00ea90cc5783750 Mon Sep 17 00:00:00 2001 From: Will Li Date: Thu, 31 Jul 2025 12:39:13 -0700 Subject: [PATCH] fail more gracefully --- .../tools/__tests__/contextValidator.test.ts | 139 +++++++++++++++-- src/core/tools/__tests__/readFileTool.spec.ts | 77 ++++++++++ src/core/tools/contextValidator.ts | 142 ++++++++++++------ src/core/tools/readFileTool.ts | 22 ++- 4 files changed, 321 insertions(+), 59 deletions(-) diff --git a/src/core/tools/__tests__/contextValidator.test.ts b/src/core/tools/__tests__/contextValidator.test.ts index 07f0b02545..b1c2c418ef 100644 --- a/src/core/tools/__tests__/contextValidator.test.ts +++ b/src/core/tools/__tests__/contextValidator.test.ts @@ -313,8 +313,8 @@ describe("contextValidator", () => { // With the new implementation, when content exceeds limit even after cutback, // it returns MIN_USEFUL_LINES (50) as the minimum expect(result.safeMaxLines).toBe(50) - expect(result.reason).toContain("File exceeds available context space") - expect(result.reason).toContain("Safely read 50 lines") + expect(result.reason).toContain("Very limited context space") + expect(result.reason).toContain("Limited to 50 lines") }) it("should handle negative available space gracefully", async () => { @@ -353,8 +353,8 @@ describe("contextValidator", () => { expect(result.shouldLimit).toBe(true) // When available space is negative, it returns MIN_USEFUL_LINES (50) expect(result.safeMaxLines).toBe(50) // MIN_USEFUL_LINES from the refactored code - expect(result.reason).toContain("File exceeds available context space") - expect(result.reason).toContain("Safely read 50 lines") + expect(result.reason).toContain("Very limited context space") + expect(result.reason).toContain("Limited to 50 lines") }) it("should limit file when it is too large and would be truncated", async () => { @@ -418,7 +418,7 @@ describe("contextValidator", () => { expect(result.shouldLimit).toBe(true) // With the new implementation, when space is very limited and content exceeds, // it returns the minimal safe value - expect(result.reason).toContain("File exceeds available context space") + expect(result.reason).toContain("Very limited context space") }) it("should not limit when file fits within context", async () => { @@ -677,14 +677,133 @@ describe("contextValidator", () => { // Should limit the file expect(result.shouldLimit).toBe(true) - expect(result.safeMaxLines).toBe(0) - expect(result.reason).toContain("Minified file exceeds available context space") - expect(result.reason).toContain("80000 tokens") - expect(result.reason).toContain("Consider using search_files") + expect(result.safeMaxLines).toBe(1) // Single-line files return 1 when truncated + expect(result.reason).toContain("Large single-line file") + expect(result.reason).toContain("Only the first") // Should have attempted to read and count tokens expect(readLines).toHaveBeenCalledWith(filePath, 0, 0) - expect(mockTask.api.countTokens).toHaveBeenCalledWith([{ type: "text", text: hugeMinifiedContent }]) + expect(mockTask.api.countTokens).toHaveBeenCalled() + }) + + it("should apply char/3 heuristic and 20% backoff for large single-line files", async () => { + const filePath = "/test/large-minified.js" + const totalLines = 1 + const currentMaxReadFileLine = -1 + + // Mock a large single-line file + vi.mocked(fs.stat).mockResolvedValue({ + size: 2 * 1024 * 1024, // 2MB + } as any) + + // Create a very large single line that exceeds estimated safe chars + const largeContent = "x".repeat(300000) // 300K chars + vi.mocked(readLines).mockResolvedValue(largeContent) + + // Mock token counting to always exceed limit, forcing maximum cutbacks + mockTask.api.countTokens = vi.fn().mockResolvedValue(100000) // Always exceeds ~57k limit + + const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask) + + // After maximum cutbacks, it should still limit the file + expect(result.shouldLimit).toBe(true) + + // Check that it either returns safeMaxLines: 1 (truncated) or 0 (can't fit any) + expect([0, 1]).toContain(result.safeMaxLines) + + if (result.safeMaxLines === 1) { + expect(result.reason).toContain("Large single-line file") + expect(result.reason).toContain("Only the first") + expect(result.reason).toContain("This is a hard limit") + } else { + expect(result.reason).toContain("Single-line file is too large") + expect(result.reason).toContain("This file cannot be accessed") + } + + // Should have made multiple API calls due to cutbacks + expect(mockTask.api.countTokens).toHaveBeenCalledTimes(5) // MAX_API_CALLS + }) + + it("should handle single-line files that fit after cutback", async () => { + const filePath = "/test/borderline-minified.js" + const totalLines = 1 + const currentMaxReadFileLine = -1 + + // Mock file size + vi.mocked(fs.stat).mockResolvedValue({ + size: 800 * 1024, // 800KB + } as any) + + // Create content that's just over the limit + const content = "const x=1;".repeat(20000) // ~200KB + vi.mocked(readLines).mockResolvedValue(content) + + // Mock token counting - first call exceeds, second fits + let callCount = 0 + mockTask.api.countTokens = vi.fn().mockImplementation(async (content) => { + callCount++ + const text = content[0].text + if (callCount === 1) { + return 65000 // Just over the ~57k limit + } + // After 20% cutback + return 45000 // Now fits comfortably + }) + + const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask) + + // Should limit but allow partial read + expect(result.shouldLimit).toBe(true) + expect(result.safeMaxLines).toBe(1) + expect(result.reason).toContain("Large single-line file") + + // Verify percentage calculation in reason + if (result.reason) { + const match = result.reason.match(/Only the first (\d+)%/) + expect(match).toBeTruthy() + if (match) { + const percentage = parseInt(match[1]) + expect(percentage).toBeGreaterThan(0) + expect(percentage).toBeLessThan(100) + } + } + + // Should have made 2 API calls (initial + after cutback) + expect(mockTask.api.countTokens).toHaveBeenCalledTimes(2) + }) + + it("should handle single-line files that cannot fit any content", async () => { + const filePath = "/test/impossible-minified.js" + const totalLines = 1 + const currentMaxReadFileLine = -1 + + // Mock file size + vi.mocked(fs.stat).mockResolvedValue({ + size: 10 * 1024 * 1024, // 10MB + } as any) + + // Mock very high context usage + mockTask.getTokenUsage = vi.fn().mockReturnValue({ + contextTokens: 99000, // 99% used + }) + + // Create massive content + const content = "x".repeat(1000000) + vi.mocked(readLines).mockResolvedValue(content) + + // Mock token counting - always exceeds even after cutbacks + mockTask.api.countTokens = vi.fn().mockResolvedValue(100000) + + const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask) + + // Should completely block the file + expect(result.shouldLimit).toBe(true) + expect(result.safeMaxLines).toBe(0) + expect(result.reason).toContain("Single-line file is too large to read any portion") + expect(result.reason).toContain("This file cannot be accessed") + + // Should have tried multiple times + expect(mockTask.api.countTokens).toHaveBeenCalled() }) it("should fall back to regular validation if single-line processing fails", async () => { diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index c15422ae0c..ee31598db4 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -1395,6 +1395,83 @@ describe("read_file tool XML output structure", () => { expect(result).not.toContain("Use line_range") expect(result).not.toContain("File exceeds available context space") }) + + it("should not include line_range instructions for single-line files", async () => { + // Mock a single-line file that exceeds context + vi.mocked(countFileLines).mockResolvedValue(1) + + // Mock contextValidator to return shouldLimit true with single-line file message + vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({ + shouldLimit: true, + safeMaxLines: 1, + reason: "Large single-line file (likely minified) exceeds available context space. Only the first 50% (5000 of 10000 characters) can be loaded. This is a hard limit - no additional content from this file can be accessed.", + }) + + // Mock extractTextFromFile to return truncated content + vi.mocked(extractTextFromFile).mockResolvedValue("1 | const a=1;const b=2;...truncated") + + const result = await executeReadFileTool( + { args: `minified.js` }, + { totalLines: 1, maxReadFileLine: -1 }, + ) + + // Verify the result contains the notice but NOT the line_range instructions + expect(result).toContain("") + expect(result).toContain("Large single-line file") + expect(result).toContain("This is a hard limit") + expect(result).not.toContain("tools:readFile.contextLimitInstructions") + expect(result).not.toContain("Use line_range") + }) + + it("should include line_range instructions for multi-line files that exceed context", async () => { + // Mock a multi-line file that exceeds context + vi.mocked(countFileLines).mockResolvedValue(5000) + + // Mock contextValidator to return shouldLimit true with multi-line file message + vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({ + shouldLimit: true, + safeMaxLines: 1000, + reason: "File exceeds available context space. Safely read 1000 lines out of 5000 total lines.", + }) + + // Mock readLines to return truncated content + vi.mocked(readLines).mockResolvedValue("Line 1\nLine 2\n...truncated...") + + const result = await executeReadFileTool( + { args: `large-file.ts` }, + { totalLines: 5000, maxReadFileLine: -1 }, + ) + + // Verify the result contains both the notice AND the line_range instructions + expect(result).toContain("") + expect(result).toContain("File exceeds available context space") + expect(result).toContain("tools:readFile.contextLimitInstructions") + }) + + it("should handle normal file read section for single-line files with validation notice", async () => { + // Mock a single-line file that has shouldLimit true but fits after truncation + vi.mocked(countFileLines).mockResolvedValue(1) + + // Mock contextValidator to return shouldLimit true with a single-line file notice + vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({ + shouldLimit: true, + safeMaxLines: 1, + reason: "Large single-line file (likely minified) exceeds available context space. Only the first 80% can be loaded.", + }) + + // Mock extractTextFromFile + vi.mocked(extractTextFromFile).mockResolvedValue("1 | const a=1;const b=2;const c=3;") + + const result = await executeReadFileTool( + { args: `semi-large.js` }, + { totalLines: 1, maxReadFileLine: -1 }, + ) + + // Verify single-line file notice doesn't include line_range instructions + expect(result).toContain("") + expect(result).toContain("Large single-line file") + expect(result).not.toContain("tools:readFile.contextLimitInstructions") + }) }) }) diff --git a/src/core/tools/contextValidator.ts b/src/core/tools/contextValidator.ts index 5839407f26..dfc92161ff 100644 --- a/src/core/tools/contextValidator.ts +++ b/src/core/tools/contextValidator.ts @@ -113,7 +113,7 @@ async function shouldSkipValidation(filePath: string, totalLines: number, cline: /** * Validates a single-line file (likely minified) to see if it fits in context - * NOTE: because we cannot chunk lines in file reads, we still cannot handle single-line files that do not fit in context + * Uses the same heuristic and backoff strategy as multi-line files */ async function validateSingleLineFile( filePath: string, @@ -123,25 +123,48 @@ async function validateSingleLineFile( console.log(`[validateFileSizeForContext] Single-line file detected: ${filePath} - checking if it fits in context`) try { - // Read the entire single line - const fileContent = await readLines(filePath, 0, 0) + // Phase 1: Use char/3 heuristic to estimate safe content size + const estimatedSafeChars = contextInfo.targetTokenLimit * CHARS_PER_TOKEN_ESTIMATE - // Count tokens for the single line - const actualTokens = await cline.api.countTokens([{ type: "text", text: fileContent }]) + // Read the single line + const fullContent = await readLines(filePath, 0, 0) - console.log( - `[validateFileSizeForContext] Single-line file: ${actualTokens} tokens, available: ${contextInfo.targetTokenLimit} tokens`, + // If the full content fits within our estimated safe chars, try it + let contentToValidate = fullContent + if (fullContent.length > estimatedSafeChars) { + // Content is too large, start with estimated safe portion + contentToValidate = fullContent.substring(0, estimatedSafeChars) + console.log( + `[validateFileSizeForContext] Single-line file exceeds estimated safe chars (${fullContent.length} > ${estimatedSafeChars}), starting with truncated content`, + ) + } + + // Phase 2: Use shared validation function with cutback + const { finalContent, actualTokens } = await validateAndCutbackContent( + contentToValidate, + contextInfo.targetTokenLimit, + cline, + true, ) - if (actualTokens <= contextInfo.targetTokenLimit) { - // The single line fits within context + // Determine the result based on what we could read + if (finalContent.length === fullContent.length) { + // The entire single line fits return { shouldLimit: false, safeMaxLines: -1 } + } else if (finalContent.length > 0) { + // Only a portion of the line fits + const percentageRead = Math.round((finalContent.length / fullContent.length) * 100) + return { + shouldLimit: true, + safeMaxLines: 1, // Still technically 1 line, but truncated + reason: `Large single-line file (likely minified) exceeds available context space. Only the first ${percentageRead}% (${finalContent.length} of ${fullContent.length} characters) can be loaded. The file contains ${actualTokens} tokens of the available ${contextInfo.targetTokenLimit} tokens. Context: ${contextInfo.currentlyUsed}/${contextInfo.contextWindow} tokens used (${Math.round((contextInfo.currentlyUsed / contextInfo.contextWindow) * 100)}%). This is a hard limit - no additional content from this file can be accessed.`, + } } else { - // Single line is too large for context + // Can't fit any content return { shouldLimit: true, safeMaxLines: 0, - reason: `Minified file exceeds available context space. The single line contains ${actualTokens} tokens but only ${contextInfo.targetTokenLimit} tokens are available. Context: ${contextInfo.currentlyUsed}/${contextInfo.contextWindow} tokens used (${Math.round((contextInfo.currentlyUsed / contextInfo.contextWindow) * 100)}%). Consider using search_files to find specific content.`, + reason: `Single-line file is too large to read any portion within available context space. The file would require more than ${contextInfo.targetTokenLimit} tokens, but context is already ${Math.round((contextInfo.currentlyUsed / contextInfo.contextWindow) * 100)}% full (${contextInfo.currentlyUsed}/${contextInfo.contextWindow} tokens used). This file cannot be accessed.`, } } } catch (error) { @@ -194,28 +217,28 @@ async function readFileInBatches( } /** - * Validates content with actual API and cuts back if needed + * Shared function to validate content with actual API and apply cutback if needed + * Works for both single-line and multi-line content */ -async function validateAndAdjustContent( - accumulatedContent: string, - initialLineCount: number, - lineToCharMap: Map, +async function validateAndCutbackContent( + content: string, targetTokenLimit: number, - totalLines: number, cline: Task, -): Promise<{ finalContent: string; finalLineCount: number }> { - let finalContent = accumulatedContent - let finalLineCount = initialLineCount + isSingleLine: boolean = false, +): Promise<{ finalContent: string; actualTokens: number; didCutback: boolean }> { + let finalContent = content let apiCallCount = 0 + let actualTokens = 0 + let didCutback = false while (apiCallCount < MAX_API_CALLS) { apiCallCount++ // Make the actual API call to count tokens - const actualTokens = await cline.api.countTokens([{ type: "text", text: finalContent }]) + actualTokens = await cline.api.countTokens([{ type: "text", text: finalContent }]) console.log( - `[validateFileSizeForContext] API call ${apiCallCount}: ${actualTokens} tokens for ${finalContent.length} chars (${finalLineCount} lines)`, + `[validateFileSizeForContext] API call ${apiCallCount}: ${actualTokens} tokens for ${finalContent.length} chars${isSingleLine ? " (single-line)" : ""}`, ) if (actualTokens <= targetTokenLimit) { @@ -226,35 +249,62 @@ async function validateAndAdjustContent( // We're over the limit - cut back by CUTBACK_PERCENTAGE const targetLength = Math.floor(finalContent.length * (1 - CUTBACK_PERCENTAGE)) - // Find the line that gets us closest to the target length - let cutoffLine = 0 - for (const [lineNum, charPos] of lineToCharMap.entries()) { - if (charPos > targetLength) { - break - } - cutoffLine = lineNum - } - - // Ensure we don't cut back too far - if (cutoffLine < 10) { - console.warn( - `[validateFileSizeForContext] Cutback resulted in too few lines (${cutoffLine}), using minimum`, - ) - cutoffLine = Math.min(MIN_USEFUL_LINES, totalLines) - } - - // Get the character position for the cutoff line - const cutoffCharPos = lineToCharMap.get(cutoffLine) || 0 - finalContent = accumulatedContent.substring(0, cutoffCharPos) - finalLineCount = cutoffLine - // Safety check - if (finalContent.length === 0) { + if (targetLength === 0 || targetLength === finalContent.length) { break } + + finalContent = finalContent.substring(0, targetLength) + didCutback = true } - return { finalContent, finalLineCount } + return { finalContent, actualTokens, didCutback } +} + +/** + * Validates content with actual API and cuts back if needed (for multi-line files) + */ +async function validateAndAdjustContent( + accumulatedContent: string, + initialLineCount: number, + lineToCharMap: Map, + targetTokenLimit: number, + totalLines: number, + cline: Task, +): Promise<{ finalContent: string; finalLineCount: number }> { + // Use the shared validation function + const { finalContent, didCutback } = await validateAndCutbackContent( + accumulatedContent, + targetTokenLimit, + cline, + false, + ) + + // If no cutback was needed, return original line count + if (!didCutback) { + return { finalContent, finalLineCount: initialLineCount } + } + + // Find the line that corresponds to the cut content length + let cutoffLine = 0 + for (const [lineNum, charPos] of lineToCharMap.entries()) { + if (charPos > finalContent.length) { + break + } + cutoffLine = lineNum + } + + // Ensure we don't cut back too far + if (cutoffLine < 10) { + console.warn(`[validateFileSizeForContext] Cutback resulted in too few lines (${cutoffLine}), using minimum`) + cutoffLine = Math.min(MIN_USEFUL_LINES, totalLines) + } + + // Get the character position for the cutoff line + const cutoffCharPos = lineToCharMap.get(cutoffLine) || 0 + const adjustedContent = accumulatedContent.substring(0, cutoffCharPos) + + return { finalContent: adjustedContent, finalLineCount: cutoffLine } } /** diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index e205f4527e..766d12951e 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -597,9 +597,15 @@ export async function readFileTool( // Add appropriate notice based on whether this was a preemptive limit or user setting if (validationNotice) { - // When shouldLimit is true, always provide inline instructions - const instructions = t("tools:readFile.contextLimitInstructions", { path: relPath }) - xmlInfo += `${validationNotice}\n\n${instructions}\n` + // Check if this is a single-line file + if (totalLines === 1 && validationNotice.includes("single-line file")) { + // For single-line files, don't suggest line_range tool + xmlInfo += `${validationNotice}\n` + } else { + // For multi-line files, provide inline instructions to use line_range + const instructions = t("tools:readFile.contextLimitInstructions", { path: relPath }) + xmlInfo += `${validationNotice}\n\n${instructions}\n` + } } else { xmlInfo += `${t("tools:readFile.showingOnlyLines", { shown: effectiveMaxReadFileLine, total: totalLines })}\n` } @@ -626,6 +632,16 @@ export async function readFileTool( if (totalLines === 0) { xmlInfo += `File is empty\n` + } else if (validationNotice) { + // Check if this is a single-line file + if (totalLines === 1 && validationNotice.includes("single-line file")) { + // For single-line files, don't suggest line_range tool + xmlInfo += `${validationNotice}\n` + } else { + // For multi-line files, provide inline instructions to use line_range + const instructions = t("tools:readFile.contextLimitInstructions", { path: relPath }) + xmlInfo += `${validationNotice}\n\n${instructions}\n` + } } // Track file read