Skip to content

Commit 8fc176d

Browse files
committed
fail more gracefully
1 parent fe60b11 commit 8fc176d

File tree

4 files changed

+318
-56
lines changed

4 files changed

+318
-56
lines changed

src/core/tools/__tests__/contextValidator.test.ts

Lines changed: 129 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,8 @@ describe("contextValidator", () => {
313313
// With the new implementation, when content exceeds limit even after cutback,
314314
// it returns MIN_USEFUL_LINES (50) as the minimum
315315
expect(result.safeMaxLines).toBe(50)
316-
expect(result.reason).toContain("File exceeds available context space")
317-
expect(result.reason).toContain("Safely read 50 lines")
316+
expect(result.reason).toContain("Very limited context space")
317+
expect(result.reason).toContain("Limited to 50 lines")
318318
})
319319

320320
it("should handle negative available space gracefully", async () => {
@@ -353,8 +353,8 @@ describe("contextValidator", () => {
353353
expect(result.shouldLimit).toBe(true)
354354
// When available space is negative, it returns MIN_USEFUL_LINES (50)
355355
expect(result.safeMaxLines).toBe(50) // MIN_USEFUL_LINES from the refactored code
356-
expect(result.reason).toContain("File exceeds available context space")
357-
expect(result.reason).toContain("Safely read 50 lines")
356+
expect(result.reason).toContain("Very limited context space")
357+
expect(result.reason).toContain("Limited to 50 lines")
358358
})
359359

360360
it("should limit file when it is too large and would be truncated", async () => {
@@ -418,7 +418,7 @@ describe("contextValidator", () => {
418418
expect(result.shouldLimit).toBe(true)
419419
// With the new implementation, when space is very limited and content exceeds,
420420
// it returns the minimal safe value
421-
expect(result.reason).toContain("File exceeds available context space")
421+
expect(result.reason).toContain("Very limited context space")
422422
})
423423

424424
it("should not limit when file fits within context", async () => {
@@ -677,14 +677,133 @@ describe("contextValidator", () => {
677677

678678
// Should limit the file
679679
expect(result.shouldLimit).toBe(true)
680-
expect(result.safeMaxLines).toBe(0)
681-
expect(result.reason).toContain("Minified file exceeds available context space")
682-
expect(result.reason).toContain("80000 tokens")
683-
expect(result.reason).toContain("Consider using search_files")
680+
expect(result.safeMaxLines).toBe(1) // Single-line files return 1 when truncated
681+
expect(result.reason).toContain("Large single-line file")
682+
expect(result.reason).toContain("Only the first")
684683

685684
// Should have attempted to read and count tokens
686685
expect(readLines).toHaveBeenCalledWith(filePath, 0, 0)
687-
expect(mockTask.api.countTokens).toHaveBeenCalledWith([{ type: "text", text: hugeMinifiedContent }])
686+
expect(mockTask.api.countTokens).toHaveBeenCalled()
687+
})
688+
689+
it("should apply char/3 heuristic and 20% backoff for large single-line files", async () => {
690+
const filePath = "/test/large-minified.js"
691+
const totalLines = 1
692+
const currentMaxReadFileLine = -1
693+
694+
// Mock a large single-line file
695+
vi.mocked(fs.stat).mockResolvedValue({
696+
size: 2 * 1024 * 1024, // 2MB
697+
} as any)
698+
699+
// Create a very large single line that exceeds estimated safe chars
700+
const largeContent = "x".repeat(300000) // 300K chars
701+
vi.mocked(readLines).mockResolvedValue(largeContent)
702+
703+
// Mock token counting to always exceed limit, forcing maximum cutbacks
704+
mockTask.api.countTokens = vi.fn().mockResolvedValue(100000) // Always exceeds ~57k limit
705+
706+
const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask)
707+
708+
// After maximum cutbacks, it should still limit the file
709+
expect(result.shouldLimit).toBe(true)
710+
711+
// Check that it either returns safeMaxLines: 1 (truncated) or 0 (can't fit any)
712+
expect([0, 1]).toContain(result.safeMaxLines)
713+
714+
if (result.safeMaxLines === 1) {
715+
expect(result.reason).toContain("Large single-line file")
716+
expect(result.reason).toContain("Only the first")
717+
expect(result.reason).toContain("This is a hard limit")
718+
} else {
719+
expect(result.reason).toContain("Single-line file is too large")
720+
expect(result.reason).toContain("This file cannot be accessed")
721+
}
722+
723+
// Should have made multiple API calls due to cutbacks
724+
expect(mockTask.api.countTokens).toHaveBeenCalledTimes(5) // MAX_API_CALLS
725+
})
726+
727+
it("should handle single-line files that fit after cutback", async () => {
728+
const filePath = "/test/borderline-minified.js"
729+
const totalLines = 1
730+
const currentMaxReadFileLine = -1
731+
732+
// Mock file size
733+
vi.mocked(fs.stat).mockResolvedValue({
734+
size: 800 * 1024, // 800KB
735+
} as any)
736+
737+
// Create content that's just over the limit
738+
const content = "const x=1;".repeat(20000) // ~200KB
739+
vi.mocked(readLines).mockResolvedValue(content)
740+
741+
// Mock token counting - first call exceeds, second fits
742+
let callCount = 0
743+
mockTask.api.countTokens = vi.fn().mockImplementation(async (content) => {
744+
callCount++
745+
const text = content[0].text
746+
if (callCount === 1) {
747+
return 65000 // Just over the ~57k limit
748+
}
749+
// After 20% cutback
750+
return 45000 // Now fits comfortably
751+
})
752+
753+
const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask)
754+
755+
// Should limit but allow partial read
756+
expect(result.shouldLimit).toBe(true)
757+
expect(result.safeMaxLines).toBe(1)
758+
expect(result.reason).toContain("Large single-line file")
759+
760+
// Verify percentage calculation in reason
761+
if (result.reason) {
762+
const match = result.reason.match(/Only the first (\d+)%/)
763+
expect(match).toBeTruthy()
764+
if (match) {
765+
const percentage = parseInt(match[1])
766+
expect(percentage).toBeGreaterThan(0)
767+
expect(percentage).toBeLessThan(100)
768+
}
769+
}
770+
771+
// Should have made 2 API calls (initial + after cutback)
772+
expect(mockTask.api.countTokens).toHaveBeenCalledTimes(2)
773+
})
774+
775+
it("should handle single-line files that cannot fit any content", async () => {
776+
const filePath = "/test/impossible-minified.js"
777+
const totalLines = 1
778+
const currentMaxReadFileLine = -1
779+
780+
// Mock file size
781+
vi.mocked(fs.stat).mockResolvedValue({
782+
size: 10 * 1024 * 1024, // 10MB
783+
} as any)
784+
785+
// Mock very high context usage
786+
mockTask.getTokenUsage = vi.fn().mockReturnValue({
787+
contextTokens: 99000, // 99% used
788+
})
789+
790+
// Create massive content
791+
const content = "x".repeat(1000000)
792+
vi.mocked(readLines).mockResolvedValue(content)
793+
794+
// Mock token counting - always exceeds even after cutbacks
795+
mockTask.api.countTokens = vi.fn().mockResolvedValue(100000)
796+
797+
const result = await validateFileSizeForContext(filePath, totalLines, currentMaxReadFileLine, mockTask)
798+
799+
// Should completely block the file
800+
expect(result.shouldLimit).toBe(true)
801+
expect(result.safeMaxLines).toBe(0)
802+
expect(result.reason).toContain("Single-line file is too large to read any portion")
803+
expect(result.reason).toContain("This file cannot be accessed")
804+
805+
// Should have tried multiple times
806+
expect(mockTask.api.countTokens).toHaveBeenCalled()
688807
})
689808

690809
it("should fall back to regular validation if single-line processing fails", async () => {

src/core/tools/__tests__/readFileTool.spec.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,83 @@ describe("read_file tool XML output structure", () => {
13951395
expect(result).not.toContain("Use line_range")
13961396
expect(result).not.toContain("File exceeds available context space")
13971397
})
1398+
1399+
it("should not include line_range instructions for single-line files", async () => {
1400+
// Mock a single-line file that exceeds context
1401+
vi.mocked(countFileLines).mockResolvedValue(1)
1402+
1403+
// Mock contextValidator to return shouldLimit true with single-line file message
1404+
vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({
1405+
shouldLimit: true,
1406+
safeMaxLines: 1,
1407+
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.",
1408+
})
1409+
1410+
// Mock extractTextFromFile to return truncated content
1411+
vi.mocked(extractTextFromFile).mockResolvedValue("1 | const a=1;const b=2;...truncated")
1412+
1413+
const result = await executeReadFileTool(
1414+
{ args: `<file><path>minified.js</path></file>` },
1415+
{ totalLines: 1, maxReadFileLine: -1 },
1416+
)
1417+
1418+
// Verify the result contains the notice but NOT the line_range instructions
1419+
expect(result).toContain("<notice>")
1420+
expect(result).toContain("Large single-line file")
1421+
expect(result).toContain("This is a hard limit")
1422+
expect(result).not.toContain("tools:readFile.contextLimitInstructions")
1423+
expect(result).not.toContain("Use line_range")
1424+
})
1425+
1426+
it("should include line_range instructions for multi-line files that exceed context", async () => {
1427+
// Mock a multi-line file that exceeds context
1428+
vi.mocked(countFileLines).mockResolvedValue(5000)
1429+
1430+
// Mock contextValidator to return shouldLimit true with multi-line file message
1431+
vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({
1432+
shouldLimit: true,
1433+
safeMaxLines: 1000,
1434+
reason: "File exceeds available context space. Safely read 1000 lines out of 5000 total lines.",
1435+
})
1436+
1437+
// Mock readLines to return truncated content
1438+
vi.mocked(readLines).mockResolvedValue("Line 1\nLine 2\n...truncated...")
1439+
1440+
const result = await executeReadFileTool(
1441+
{ args: `<file><path>large-file.ts</path></file>` },
1442+
{ totalLines: 5000, maxReadFileLine: -1 },
1443+
)
1444+
1445+
// Verify the result contains both the notice AND the line_range instructions
1446+
expect(result).toContain("<notice>")
1447+
expect(result).toContain("File exceeds available context space")
1448+
expect(result).toContain("tools:readFile.contextLimitInstructions</notice>")
1449+
})
1450+
1451+
it("should handle normal file read section for single-line files with validation notice", async () => {
1452+
// Mock a single-line file that has shouldLimit true but fits after truncation
1453+
vi.mocked(countFileLines).mockResolvedValue(1)
1454+
1455+
// Mock contextValidator to return shouldLimit true with a single-line file notice
1456+
vi.mocked(contextValidatorModule.validateFileSizeForContext).mockResolvedValue({
1457+
shouldLimit: true,
1458+
safeMaxLines: 1,
1459+
reason: "Large single-line file (likely minified) exceeds available context space. Only the first 80% can be loaded.",
1460+
})
1461+
1462+
// Mock extractTextFromFile
1463+
vi.mocked(extractTextFromFile).mockResolvedValue("1 | const a=1;const b=2;const c=3;")
1464+
1465+
const result = await executeReadFileTool(
1466+
{ args: `<file><path>semi-large.js</path></file>` },
1467+
{ totalLines: 1, maxReadFileLine: -1 },
1468+
)
1469+
1470+
// Verify single-line file notice doesn't include line_range instructions
1471+
expect(result).toContain("<notice>")
1472+
expect(result).toContain("Large single-line file")
1473+
expect(result).not.toContain("tools:readFile.contextLimitInstructions")
1474+
})
13981475
})
13991476
})
14001477

0 commit comments

Comments
 (0)