fail more gracefully

This commit is contained in:
Will Li 2025-07-31 12:39:13 -07:00
parent fe60b11189
commit 8fc176dd8a
4 changed files with 321 additions and 59 deletions

View file

@ -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 () => {

View file

@ -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: `<file><path>minified.js</path></file>` },
{ totalLines: 1, maxReadFileLine: -1 },
)
// Verify the result contains the notice but NOT the line_range instructions
expect(result).toContain("<notice>")
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: `<file><path>large-file.ts</path></file>` },
{ totalLines: 5000, maxReadFileLine: -1 },
)
// Verify the result contains both the notice AND the line_range instructions
expect(result).toContain("<notice>")
expect(result).toContain("File exceeds available context space")
expect(result).toContain("tools:readFile.contextLimitInstructions</notice>")
})
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: `<file><path>semi-large.js</path></file>` },
{ totalLines: 1, maxReadFileLine: -1 },
)
// Verify single-line file notice doesn't include line_range instructions
expect(result).toContain("<notice>")
expect(result).toContain("Large single-line file")
expect(result).not.toContain("tools:readFile.contextLimitInstructions")
})
})
})

View file

@ -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<number, number>,
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<number, number>,
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 }
}
/**

View file

@ -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 += `<notice>${validationNotice}\n\n${instructions}</notice>\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 += `<notice>${validationNotice}</notice>\n`
} else {
// For multi-line files, provide inline instructions to use line_range
const instructions = t("tools:readFile.contextLimitInstructions", { path: relPath })
xmlInfo += `<notice>${validationNotice}\n\n${instructions}</notice>\n`
}
} else {
xmlInfo += `<notice>${t("tools:readFile.showingOnlyLines", { shown: effectiveMaxReadFileLine, total: totalLines })}</notice>\n`
}
@ -626,6 +632,16 @@ export async function readFileTool(
if (totalLines === 0) {
xmlInfo += `<notice>File is empty</notice>\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 += `<notice>${validationNotice}</notice>\n`
} else {
// For multi-line files, provide inline instructions to use line_range
const instructions = t("tools:readFile.contextLimitInstructions", { path: relPath })
xmlInfo += `<notice>${validationNotice}\n\n${instructions}</notice>\n`
}
}
// Track file read