diff --git a/.changeset/seven-flowers-lay.md b/.changeset/seven-flowers-lay.md new file mode 100644 index 0000000000..b051dfff08 --- /dev/null +++ b/.changeset/seven-flowers-lay.md @@ -0,0 +1,5 @@ +--- +"claude-dev": patch +--- + +Fix a bug where cline crashes when reading large data from files diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 38093d5de7..1783d92cdb 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -62,6 +62,7 @@ import { ClineHandler } from "../api/providers/cline" import { ClineProvider, GlobalFileNames } from "./webview/ClineProvider" import { DEFAULT_LANGUAGE_SETTINGS, getLanguageKey, LanguageDisplay, LanguageKey } from "../shared/Languages" import { telemetryService } from "../services/telemetry/TelemetryService" +import { getMaxAllowedSize } from "../utils/content-size" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -1353,25 +1354,8 @@ export class Cline { if (previousRequest && previousRequest.text) { const { tokensIn, tokensOut, cacheWrites, cacheReads }: ClineApiReqInfo = JSON.parse(previousRequest.text) const totalTokens = (tokensIn || 0) + (tokensOut || 0) + (cacheWrites || 0) + (cacheReads || 0) - let contextWindow = this.api.getModel().info.contextWindow || 128_000 - // FIXME: hack to get anyone using openai compatible with deepseek to have the proper context window instead of the default 128k. We need a way for the user to specify the context window for models they input through openai compatible - if (this.api instanceof OpenAiHandler && this.api.getModel().id.toLowerCase().includes("deepseek")) { - contextWindow = 64_000 - } - let maxAllowedSize: number - switch (contextWindow) { - case 64_000: // deepseek models - maxAllowedSize = contextWindow - 27_000 - break - case 128_000: // most models - maxAllowedSize = contextWindow - 30_000 - break - case 200_000: // claude models - maxAllowedSize = contextWindow - 40_000 - break - default: - maxAllowedSize = Math.max(contextWindow - 40_000, contextWindow * 0.8) // for deepseek, 80% of 64k meant only ~10k buffer which was too small and resulted in users getting context window errors. - } + let contextWindow = this.api.getModel().info.contextWindow || 64_000 // minimum context (Deepseek) + const maxAllowedSize = getMaxAllowedSize(contextWindow) // This is the most reliable way to know when we're close to hitting the context window. if (totalTokens >= maxAllowedSize) { @@ -2015,15 +1999,17 @@ export class Cline { } telemetryService.captureToolUsage(this.taskId, block.name, false, true) } - // now execute the tool like normal - const content = await extractTextFromFile(absolutePath) + // Get context window and used context from API model + const contextWindow = this.api.getModel().info.contextWindow + + // Pass the raw context window size - extractTextFromFile will calculate the appropriate limit + const content = await extractTextFromFile(absolutePath, contextWindow) pushToolResult(content) break } } catch (error) { await handleError("reading file", error) - break } } @@ -3390,9 +3376,10 @@ export class Cline { block.text.includes("") || block.text.includes("") ) { + let contextWindow = this.api.getModel().info.contextWindow return { ...block, - text: await parseMentions(block.text, cwd, this.urlContentFetcher), + text: await parseMentions(block.text, cwd, this.urlContentFetcher, contextWindow), } } } @@ -3497,11 +3484,12 @@ export class Cline { } } } + // only show inactive terminals if there's output to show if (inactiveTerminals.length > 0) { const inactiveTerminalOutputs = new Map() for (const inactiveTerminal of inactiveTerminals) { - const newOutput = this.terminalManager.getUnretrievedOutput(inactiveTerminal.id) + const newOutput = await this.terminalManager.getUnretrievedOutput(inactiveTerminal.id) if (newOutput) { inactiveTerminalOutputs.set(inactiveTerminal.id, newOutput) } diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index 146e823f34..14ff22d923 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -38,7 +38,12 @@ export function openMention(mention?: string): void { } } -export async function parseMentions(text: string, cwd: string, urlContentFetcher: UrlContentFetcher): Promise { +export async function parseMentions( + text: string, + cwd: string, + urlContentFetcher: UrlContentFetcher, + contextWindow?: number, +): Promise { const mentions: Set = new Set() let parsedText = text.replace(mentionRegexGlobal, (match, mention) => { mentions.add(mention) @@ -90,7 +95,7 @@ export async function parseMentions(text: string, cwd: string, urlContentFetcher } else if (mention.startsWith("/")) { const mentionPath = mention.slice(1) try { - const content = await getFileOrFolderContent(mentionPath, cwd) + const content = await getFileOrFolderContent(mentionPath, cwd, contextWindow) if (mention.endsWith("/")) { parsedText += `\n\n\n${content}\n` } else { @@ -145,7 +150,7 @@ export async function parseMentions(text: string, cwd: string, urlContentFetcher return parsedText } -async function getFileOrFolderContent(mentionPath: string, cwd: string): Promise { +async function getFileOrFolderContent(mentionPath: string, cwd: string, contextWindow?: number): Promise { const absPath = path.resolve(cwd, mentionPath) try { @@ -156,7 +161,7 @@ async function getFileOrFolderContent(mentionPath: string, cwd: string): Promise if (isBinary) { return "(Binary file, unable to display content)" } - const content = await extractTextFromFile(absPath) + const content = await extractTextFromFile(absPath, contextWindow) return content } else if (stats.isDirectory()) { const entries = await fs.readdir(absPath, { withFileTypes: true }) @@ -177,7 +182,7 @@ async function getFileOrFolderContent(mentionPath: string, cwd: string): Promise if (isBinary) { return undefined } - const content = await extractTextFromFile(absoluteFilePath) + const content = await extractTextFromFile(absoluteFilePath, contextWindow) return `\n${content}\n` } catch (error) { return undefined diff --git a/src/integrations/misc/extract-text.test.ts b/src/integrations/misc/extract-text.test.ts new file mode 100644 index 0000000000..046c71f0b3 --- /dev/null +++ b/src/integrations/misc/extract-text.test.ts @@ -0,0 +1,67 @@ +import { expect } from "chai" +import { extractTextFromFile } from "./extract-text" +import fs from "fs/promises" +import path from "path" +import os from "os" +import { ContentTooLargeError } from "../../shared/errors" + +const CONTEXT_LIMIT = 1000 // Context limit of 1000 tokens means max allowed size is 500 tokens + +describe("extract-text", () => { + let tempFilePath: string + + beforeEach(async () => { + tempFilePath = path.join(os.tmpdir(), "test-file.txt") + }) + + afterEach(async () => { + await fs.unlink(tempFilePath).catch(() => {}) + }) + + it("throws error for non-existent file", async () => { + const nonExistentPath = path.join(os.tmpdir(), "non-existent.txt") + try { + await extractTextFromFile(nonExistentPath, CONTEXT_LIMIT) + throw new Error("Should have thrown error") + } catch (error) { + expect(error.message).to.include("File not found") + } + }) + + it("throws ContentTooLargeError when file would exceed max allowed size", async () => { + // Create content that would exceed max allowed size (37k tokens) + const largeContent = "x".repeat(148000) // 37k tokens + await fs.writeFile(tempFilePath, largeContent) + + try { + await extractTextFromFile(tempFilePath, 37_000) // Pass pre-processed maxAllowedSize + throw new Error("Should have thrown error") + } catch (error) { + expect(error).to.be.instanceOf(ContentTooLargeError) + expect(error.details.type).to.equal("file") + expect(error.details.path).to.equal(tempFilePath) + expect(error.details.size.wouldExceedLimit).to.equal(true) + } + }) + + it("reads text file content when within size limit", async () => { + const content = "Hello world" + await fs.writeFile(tempFilePath, content) + + const result = await extractTextFromFile(tempFilePath, CONTEXT_LIMIT) + expect(result).to.equal(content) + }) + + it("throws error for binary files", async () => { + // Create a simple binary file + const buffer = new Uint8Array([0x89, 0x50, 0x4e, 0x47]) // PNG file header + await fs.writeFile(tempFilePath, buffer, { encoding: "binary" }) + + try { + await extractTextFromFile(tempFilePath, CONTEXT_LIMIT) + throw new Error("Should have thrown error") + } catch (error) { + expect(error.message).to.include("Cannot read text for file type") + } + }) +}) diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index 67a580af9b..3c122b7249 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -4,29 +4,102 @@ import pdf from "pdf-parse/lib/pdf-parse" import mammoth from "mammoth" import fs from "fs/promises" import { isBinaryFile } from "isbinaryfile" +import { estimateContentSize, estimateFileSize, wouldExceedSizeLimit, getMaxAllowedSize } from "../../utils/content-size" +import { ContentTooLargeError } from "../../shared/errors" -export async function extractTextFromFile(filePath: string): Promise { +/** + * Checks if terminal output would exceed size limits and returns the content if safe + * @param content The terminal output content to check + * @param contextWindow Context window limit in tokens + * @param command The command that generated this output (for error reporting) + * @returns The validated content + * @throws ContentTooLargeError if content exceeds size limit + */ +export async function extractTextFromTerminal(content: string | Buffer, contextWindow: number, command: string): Promise { + console.debug(`[TERMINAL_SIZE_CHECK] Checking size for command output: ${command}`) + + // Convert to string but don't trim yet + const rawContent = content.toString() + console.debug(`[TERMINAL_SIZE_CHECK] Raw content length: ${rawContent.length}`) + + // Check size before trimming + const sizeEstimate = estimateContentSize(rawContent, contextWindow) + console.debug(`[TERMINAL_SIZE_CHECK] Content size: ${sizeEstimate.bytes} bytes`) + console.debug(`[TERMINAL_SIZE_CHECK] Estimated tokens: ${sizeEstimate.estimatedTokens}`) + console.debug(`[TERMINAL_SIZE_CHECK] Context window: ${contextWindow}`) + + if (sizeEstimate.wouldExceedLimit) { + console.debug(`[TERMINAL_SIZE_CHECK] Output exceeds size limit`) + throw new ContentTooLargeError({ + type: "terminal", + command, + size: sizeEstimate, + }) + } + + // Only trim after size check passes + const cleanContent = rawContent.trim() + console.debug(`[TERMINAL_SIZE_CHECK] Clean content length: ${cleanContent.length}`) + console.debug(`[TERMINAL_SIZE_CHECK] Size check passed`) + return cleanContent +} + +export async function extractTextFromFile( + filePath: string, + contextWindow: number = 64_000 /* minimum context (Deepseek) */, +): Promise { try { await fs.access(filePath) } catch (error) { throw new Error(`File not found: ${filePath}`) } + + console.debug(`[FILE_READ_CHECK] Checking size for file: ${filePath}`) + + // Get file stats to check size + const stats = await fs.stat(filePath) + console.debug(`[FILE_SIZE_CHECK] File size: ${stats.size} bytes`) + + // Calculate max allowed size from context window + const maxAllowedSize = getMaxAllowedSize(contextWindow) + console.debug(`[FILE_SIZE_CHECK] Max allowed size: ${maxAllowedSize} tokens`) + + // Check if file size would exceed limit before attempting to read + // This is more efficient than creating a full SizeEstimate object when we just need a boolean check + if (wouldExceedSizeLimit(stats.size, contextWindow)) { + console.debug(`[FILE_SIZE_CHECK] File exceeds size limit`) + // Only create the full size estimate when we need it for the error + const sizeEstimate = await estimateFileSize(filePath, maxAllowedSize) + throw new ContentTooLargeError({ + type: "file", + path: filePath, + size: sizeEstimate, + }) + } + console.debug(`[FILE_SIZE_CHECK] File size check passed`) const fileExtension = path.extname(filePath).toLowerCase() + console.debug(`[FILE_READ] Reading file: ${filePath}`) + let content: string switch (fileExtension) { case ".pdf": - return extractTextFromPDF(filePath) + content = await extractTextFromPDF(filePath) + break case ".docx": - return extractTextFromDOCX(filePath) + content = await extractTextFromDOCX(filePath) + break case ".ipynb": - return extractTextFromIPYNB(filePath) + content = await extractTextFromIPYNB(filePath) + break default: const isBinary = await isBinaryFile(filePath).catch(() => false) if (!isBinary) { - return await fs.readFile(filePath, "utf8") + content = await fs.readFile(filePath, "utf8") } else { throw new Error(`Cannot read text for file type: ${fileExtension}`) } } + console.debug(`[FILE_READ_COMPLETE] File read complete. Content length: ${content.length} chars`) + return content } async function extractTextFromPDF(filePath: string): Promise { diff --git a/src/shared/errors.ts b/src/shared/errors.ts new file mode 100644 index 0000000000..741e749383 --- /dev/null +++ b/src/shared/errors.ts @@ -0,0 +1,18 @@ +import { SizeEstimate } from "../utils/content-size" + +/** + * Error thrown when content would exceed the model's context window limit + */ +export class ContentTooLargeError extends Error { + constructor( + public details: { + type: "file" | "terminal" + path?: string + command?: string + size: SizeEstimate + }, + ) { + super("Content too large for context window") + this.name = "ContentTooLargeError" + } +} diff --git a/src/utils/content-size.test.ts b/src/utils/content-size.test.ts new file mode 100644 index 0000000000..67f47a5a52 --- /dev/null +++ b/src/utils/content-size.test.ts @@ -0,0 +1,90 @@ +import { expect } from "chai" +import { estimateContentSize, estimateFileSize, estimateTokens, wouldExceedSizeLimit } from "./content-size" +import fs from "fs/promises" +import path from "path" +import os from "os" + +const CONTEXT_LIMIT = 1000 // Context limit of 1000 tokens means max allowed size is 500 tokens + +describe("content-size", () => { + describe("estimateTokens", () => { + it("estimates tokens based on byte count", () => { + expect(estimateTokens(100)).to.equal(50) // 100 bytes / 2 chars per token = 50 tokens + expect(estimateTokens(7)).to.equal(4) // Should round up for partial tokens + }) + }) + + describe("wouldExceedSizeLimit", () => { + it("checks if byte count would exceed max allowed size", () => { + // For deepseek (64k - 27k = 37k tokens) + expect(wouldExceedSizeLimit(100, 37_000)).to.equal(false) // 50 tokens < 37k tokens + expect(wouldExceedSizeLimit(148000, 37_000)).to.equal(true) // 74k tokens > 37k tokens + // For standard models (128k - 30k = 98k tokens) + expect(wouldExceedSizeLimit(392000, 98_000)).to.equal(true) // 196k tokens > 98k tokens + }) + }) + + describe("estimateContentSize", () => { + it("estimates size for string content", () => { + const content = "Hello world" // 11 bytes + const result = estimateContentSize(content, CONTEXT_LIMIT) + + expect(result.bytes).to.equal(11) + expect(result.estimatedTokens).to.equal(6) + expect(result.wouldExceedLimit).to.equal(false) + }) + + it("estimates size for buffer content", () => { + const content = Buffer.from("Hello world") // 11 bytes + const result = estimateContentSize(content, CONTEXT_LIMIT) + + expect(result.bytes).to.equal(11) + expect(result.estimatedTokens).to.equal(6) + expect(result.wouldExceedLimit).to.equal(false) + }) + + it("detects when content would exceed max allowed size", () => { + // Create content that would exceed max allowed size for deepseek (64k - 27k tokens) + const largeContent = "x".repeat(148000) // 74k tokens > (64k - 27k) tokens + const result = estimateContentSize(largeContent, 64_000) + + expect(result.wouldExceedLimit).to.equal(true) + }) + }) + + describe("estimateFileSize", () => { + let tempFilePath: string + + beforeEach(async () => { + tempFilePath = path.join(os.tmpdir(), "test-file.txt") + await fs.writeFile(tempFilePath, "Hello world") // 11 bytes + }) + + afterEach(async () => { + await fs.unlink(tempFilePath).catch(() => {}) + }) + + it("estimates size for existing file", async () => { + const result = await estimateFileSize(tempFilePath, CONTEXT_LIMIT) + + expect(result.bytes).to.equal(11) + expect(result.estimatedTokens).to.equal(6) + expect(result.wouldExceedLimit).to.equal(false) + }) + + /** + * Verifies that estimateFileSize properly throws an error when given a + * non-existent file path, which is important for error handling in the application. + * The test expects fs.stat() inside estimateFileSize to fail and throw. + */ + it("throws error for non-existent file", async () => { + const nonExistentPath = path.join(os.tmpdir(), "non-existent.txt") + try { + await estimateFileSize(nonExistentPath, CONTEXT_LIMIT) + throw new Error("Should have thrown error") + } catch (error) { + expect(error).to.be.instanceOf(Error) + } + }) + }) +}) diff --git a/src/utils/content-size.ts b/src/utils/content-size.ts new file mode 100644 index 0000000000..d74f216ae7 --- /dev/null +++ b/src/utils/content-size.ts @@ -0,0 +1,79 @@ +import { stat } from "fs/promises" + +// Rough approximation: 1 token ≈ 2 characters for English text +const CHARS_PER_TOKEN = 2 + +export interface SizeEstimate { + bytes: number + estimatedTokens: number + wouldExceedLimit: boolean +} + +/** + * Estimates tokens from byte count using a simple character ratio + * This is a rough approximation - actual token count may vary + */ +export function estimateTokens(bytes: number): number { + return Math.ceil(bytes / CHARS_PER_TOKEN) +} + +/** + * Checks if the given byte count would exceed the size limit + * More efficient than creating a buffer just to check size + */ +export function wouldExceedSizeLimit(byteCount: number, maxAllowedSize: number): boolean { + const estimatedTokenCount = estimateTokens(byteCount) + console.debug(`[WOULD_EXCEED_SIZE_CHECK] Estimated Token Count: ${estimatedTokenCount} tokens`) + return estimatedTokenCount >= maxAllowedSize +} + +/** + * Estimates size metrics for a string or buffer without loading entire content + */ +export function estimateContentSize(content: string | Buffer, contextLimit: number): SizeEstimate { + const bytes = Buffer.isBuffer(content) ? content.length : Buffer.from(content).length + const estimatedTokenCount = estimateTokens(bytes) + + return { + bytes, + estimatedTokens: estimatedTokenCount, + wouldExceedLimit: estimatedTokenCount >= getMaxAllowedSize(contextLimit), + } +} + +/** + * Gets size metrics for a file without reading its contents + */ +export async function estimateFileSize(filePath: string, contextLimit: number): Promise { + const stats = await stat(filePath) + const bytes = stats.size + const estimatedTokenCount = estimateTokens(bytes) + + return { + bytes, + estimatedTokens: estimatedTokenCount, + wouldExceedLimit: estimatedTokenCount >= getMaxAllowedSize(contextLimit), + } +} + +/** + * Gets the maximum allowed size for the API context window + */ +export function getMaxAllowedSize(contextWindow: number): number { + // Get context window and used context from API model + let maxAllowedSize: number + switch (contextWindow) { + case 64_000: // deepseek models + maxAllowedSize = contextWindow - 27_000 + break + case 128_000: // most models + maxAllowedSize = contextWindow - 30_000 + break + case 200_000: // claude models + maxAllowedSize = contextWindow - 40_000 + break + default: + maxAllowedSize = Math.max(contextWindow - 40_000, contextWindow * 0.8) + } + return maxAllowedSize +} diff --git a/src/utils/fs.ts b/src/utils/fs.ts index 489154961b..77b3e53ae5 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -1,5 +1,6 @@ import fs from "fs/promises" import * as path from "path" +import { extractTextFromFile } from "../integrations/misc/extract-text" /** * Asynchronously creates all non-existing subdirectories for a given file path @@ -32,10 +33,37 @@ export async function createDirectoriesForFile(filePath: string): Promise { + return await extractTextFromFile(filePath, contextWindow) +} + +/** + * Safely reads and parses a JSON configuration file + * @param filePath Path to the JSON configuration file + * @param contextWindow Context window limit in tokens + * @returns The parsed configuration object + */ +export async function readJsonConfigFile(filePath: string, contextWindow: number): Promise { + try { + const content = await readConfigFile(filePath, contextWindow) + return JSON.parse(content) as T + } catch (error) { + if (error instanceof SyntaxError) { + throw new Error(`Invalid JSON in config file ${filePath}: ${error.message}`, { cause: error }) + } + throw new Error(`Failed to read config file ${filePath}: ${error instanceof Error ? error.message : String(error)}`) + } +} + +/** + * Helper function to check if a path exists + * @param filePath The path to check + * @returns A promise that resolves to true if the path exists, false otherwise */ export async function fileExistsAtPath(filePath: string): Promise { try {