mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
feat: add pre-read file checkpoint to prevent context limit issues
- Add preReadFileCheckpoint toggle setting to GlobalSettings - Implement file size validation against context limits in ReadFileTool - Show warning dialog when file exceeds context budget - Add UI toggle in context management settings - Add comprehensive tests for the new functionality Fixes #9687
This commit is contained in:
parent
f9eb6d9cf3
commit
2d791b6415
7 changed files with 279 additions and 0 deletions
|
|
@ -148,6 +148,7 @@ export const globalSettingsSchema = z.object({
|
|||
maxReadFileLine: z.number().optional(),
|
||||
maxImageFileSize: z.number().optional(),
|
||||
maxTotalImageSize: z.number().optional(),
|
||||
preReadFileCheckpoint: z.boolean().optional(),
|
||||
|
||||
terminalOutputLineLimit: z.number().optional(),
|
||||
terminalOutputCharacterLimit: z.number().optional(),
|
||||
|
|
|
|||
|
|
@ -139,6 +139,10 @@ export class ReadFileTool extends BaseTool<"read_file"> {
|
|||
try {
|
||||
const filesToApprove: FileResult[] = []
|
||||
|
||||
// Check if preReadFileCheckpoint is enabled
|
||||
const globalState = await task.providerRef.deref()?.getState()
|
||||
const preReadFileCheckpoint = (globalState as any)?.preReadFileCheckpoint ?? false
|
||||
|
||||
for (const fileResult of fileResults) {
|
||||
const relPath = fileResult.path
|
||||
const fullPath = path.resolve(task.cwd, relPath)
|
||||
|
|
@ -188,6 +192,44 @@ export class ReadFileTool extends BaseTool<"read_file"> {
|
|||
continue
|
||||
}
|
||||
|
||||
// Pre-read checkpoint: validate file size against context limits
|
||||
if (preReadFileCheckpoint) {
|
||||
const { contextTokens } = task.getTokenUsage()
|
||||
const contextWindow = modelInfo.contextWindow
|
||||
|
||||
const budgetResult = await validateFileTokenBudget(fullPath, contextWindow, contextTokens || 0)
|
||||
|
||||
// If file exceeds context budget, ask user if they want to proceed
|
||||
if (budgetResult.shouldTruncate && budgetResult.reason) {
|
||||
const warningMessage = `⚠️ File Size Warning for ${relPath}:\n${budgetResult.reason}\n\nThe file may cause context limit issues. Do you want to proceed with reading this file?`
|
||||
|
||||
const completeMessage = JSON.stringify({
|
||||
tool: "readFile",
|
||||
path: getReadablePath(task.cwd, relPath),
|
||||
content: fullPath,
|
||||
reason: warningMessage,
|
||||
} satisfies ClineSayTool)
|
||||
|
||||
const { response, text, images } = await task.ask("tool", completeMessage, false)
|
||||
|
||||
if (response !== "yesButtonClicked") {
|
||||
if (text) await task.say("user_feedback", text, images)
|
||||
task.didRejectTool = true
|
||||
updateFileResult(relPath, {
|
||||
status: "denied",
|
||||
xmlContent: `<file><path>${relPath}</path><status>Denied by user due to file size exceeding context limits</status></file>`,
|
||||
nativeContent: `File: ${relPath}\nStatus: Denied by user due to file size exceeding context limits`,
|
||||
feedbackText: text,
|
||||
feedbackImages: images,
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
// User approved despite warning
|
||||
if (text) await task.say("user_feedback", text, images)
|
||||
}
|
||||
}
|
||||
|
||||
filesToApprove.push(fileResult)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
224
src/core/tools/__tests__/ReadFileTool.preCheckpoint.spec.ts
Normal file
224
src/core/tools/__tests__/ReadFileTool.preCheckpoint.spec.ts
Normal file
|
|
@ -0,0 +1,224 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
import path from "path"
|
||||
import * as fs from "fs/promises"
|
||||
import { ReadFileTool } from "../ReadFileTool"
|
||||
import type { Task } from "../../task/Task"
|
||||
import type { ToolCallbacks } from "../BaseTool"
|
||||
import { validateFileTokenBudget } from "../helpers/fileTokenBudget"
|
||||
|
||||
// Mock the fileTokenBudget helper
|
||||
vi.mock("../helpers/fileTokenBudget", () => ({
|
||||
validateFileTokenBudget: vi.fn(),
|
||||
truncateFileContent: vi.fn(),
|
||||
}))
|
||||
|
||||
// Mock fs/promises
|
||||
vi.mock("fs/promises", () => ({
|
||||
stat: vi.fn(),
|
||||
readFile: vi.fn(),
|
||||
open: vi.fn(),
|
||||
}))
|
||||
|
||||
// Mock isBinaryFile
|
||||
vi.mock("isbinaryfile", () => ({
|
||||
isBinaryFile: vi.fn().mockResolvedValue(false),
|
||||
}))
|
||||
|
||||
// Mock extractTextFromFile
|
||||
vi.mock("../../../integrations/misc/extract-text", () => ({
|
||||
extractTextFromFile: vi.fn().mockResolvedValue("file content"),
|
||||
addLineNumbers: vi.fn((content: string) => content),
|
||||
getSupportedBinaryFormats: vi.fn().mockReturnValue([]),
|
||||
}))
|
||||
|
||||
// Mock countFileLines
|
||||
vi.mock("../../../integrations/misc/line-counter", () => ({
|
||||
countFileLines: vi.fn().mockResolvedValue(10),
|
||||
}))
|
||||
|
||||
describe("ReadFileTool - Pre-read Checkpoint", () => {
|
||||
let readFileTool: ReadFileTool
|
||||
let mockTask: Partial<Task>
|
||||
let mockCallbacks: ToolCallbacks
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
readFileTool = new ReadFileTool()
|
||||
|
||||
// Setup mock task
|
||||
mockTask = {
|
||||
cwd: "/test/workspace",
|
||||
api: {
|
||||
getModel: () => ({
|
||||
info: {
|
||||
contextWindow: 100000,
|
||||
supportsImages: false,
|
||||
},
|
||||
}),
|
||||
},
|
||||
apiConfiguration: {
|
||||
apiProvider: "anthropic",
|
||||
},
|
||||
getTokenUsage: () => ({ contextTokens: 50000 }),
|
||||
providerRef: {
|
||||
deref: () => ({
|
||||
getState: vi.fn().mockResolvedValue({
|
||||
preReadFileCheckpoint: true,
|
||||
maxReadFileLine: -1,
|
||||
}),
|
||||
}),
|
||||
},
|
||||
say: vi.fn(),
|
||||
ask: vi.fn(),
|
||||
sayAndCreateMissingParamError: vi.fn(),
|
||||
fileContextTracker: {
|
||||
trackFileContext: vi.fn(),
|
||||
},
|
||||
rooIgnoreController: {
|
||||
validateAccess: () => true,
|
||||
},
|
||||
consecutiveMistakeCount: 0,
|
||||
recordToolError: vi.fn(),
|
||||
didRejectTool: false,
|
||||
didToolFailInCurrentTurn: false,
|
||||
} as any
|
||||
|
||||
mockCallbacks = {
|
||||
handleError: vi.fn(),
|
||||
pushToolResult: vi.fn(),
|
||||
toolProtocol: "xml",
|
||||
askApproval: vi.fn(),
|
||||
removeClosingTag: vi.fn(),
|
||||
}
|
||||
|
||||
// Setup default fs mock responses
|
||||
vi.mocked(fs.stat).mockResolvedValue({ size: 500000 } as any) // 500KB file
|
||||
vi.mocked(fs.readFile).mockResolvedValue("file content")
|
||||
})
|
||||
|
||||
it("should check file size against context limits when preReadFileCheckpoint is enabled", async () => {
|
||||
// Mock validateFileTokenBudget to indicate file exceeds budget
|
||||
vi.mocked(validateFileTokenBudget).mockResolvedValue({
|
||||
shouldTruncate: true,
|
||||
maxChars: 1000,
|
||||
reason: "File requires 60000 tokens but only 30000 tokens available in context budget",
|
||||
})
|
||||
|
||||
// Mock user approval
|
||||
mockTask.ask = vi.fn().mockResolvedValue({
|
||||
response: "yesButtonClicked",
|
||||
})
|
||||
|
||||
const params = {
|
||||
files: [{ path: "test.ts", lineRanges: [] }],
|
||||
}
|
||||
|
||||
await readFileTool.execute(params, mockTask as Task, mockCallbacks)
|
||||
|
||||
// Verify that validateFileTokenBudget was called
|
||||
expect(validateFileTokenBudget).toHaveBeenCalledWith(path.resolve("/test/workspace", "test.ts"), 100000, 50000)
|
||||
|
||||
// Verify that ask was called with warning message
|
||||
expect(mockTask.ask).toHaveBeenCalledWith("tool", expect.stringContaining("File Size Warning"), false)
|
||||
|
||||
// Verify that tool result was pushed
|
||||
expect(mockCallbacks.pushToolResult).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should skip file when user denies reading file that exceeds context limits", async () => {
|
||||
// Mock validateFileTokenBudget to indicate file exceeds budget
|
||||
vi.mocked(validateFileTokenBudget).mockResolvedValue({
|
||||
shouldTruncate: true,
|
||||
maxChars: 1000,
|
||||
reason: "File requires 60000 tokens but only 30000 tokens available in context budget",
|
||||
})
|
||||
|
||||
// Mock user denial
|
||||
mockTask.ask = vi.fn().mockResolvedValue({
|
||||
response: "noButtonClicked",
|
||||
text: "File is too large",
|
||||
})
|
||||
|
||||
const params = {
|
||||
files: [{ path: "large-file.ts", lineRanges: [] }],
|
||||
}
|
||||
|
||||
await readFileTool.execute(params, mockTask as Task, mockCallbacks)
|
||||
|
||||
// Verify that validateFileTokenBudget was called
|
||||
expect(validateFileTokenBudget).toHaveBeenCalled()
|
||||
|
||||
// Verify that ask was called with warning message
|
||||
expect(mockTask.ask).toHaveBeenCalled()
|
||||
|
||||
// Verify that the tool marked rejection
|
||||
expect(mockTask.didRejectTool).toBe(true)
|
||||
|
||||
// Verify that result includes denial message
|
||||
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Denied by user due to file size exceeding context limits"),
|
||||
)
|
||||
})
|
||||
|
||||
it("should not check file size when preReadFileCheckpoint is disabled", async () => {
|
||||
// Disable preReadFileCheckpoint
|
||||
mockTask.providerRef = {
|
||||
deref: () => ({
|
||||
getState: vi.fn().mockResolvedValue({
|
||||
preReadFileCheckpoint: false, // Disabled
|
||||
maxReadFileLine: -1,
|
||||
}),
|
||||
}),
|
||||
} as any
|
||||
|
||||
const params = {
|
||||
files: [{ path: "test.ts", lineRanges: [] }],
|
||||
}
|
||||
|
||||
// Mock user approval for regular file read
|
||||
mockTask.ask = vi.fn().mockResolvedValue({
|
||||
response: "yesButtonClicked",
|
||||
})
|
||||
|
||||
await readFileTool.execute(params, mockTask as Task, mockCallbacks)
|
||||
|
||||
// Verify that validateFileTokenBudget was NOT called for pre-check
|
||||
// (it may still be called later in the normal flow)
|
||||
const askCalls = vi.mocked(mockTask.ask).mock.calls
|
||||
const warningCalls = askCalls.filter(
|
||||
(call) => call[1] && typeof call[1] === "string" && call[1].includes("File Size Warning"),
|
||||
)
|
||||
expect(warningCalls).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("should proceed without warning when file fits within context budget", async () => {
|
||||
// Mock validateFileTokenBudget to indicate file fits
|
||||
vi.mocked(validateFileTokenBudget).mockResolvedValue({
|
||||
shouldTruncate: false,
|
||||
})
|
||||
|
||||
// Mock user approval for regular file read
|
||||
mockTask.ask = vi.fn().mockResolvedValue({
|
||||
response: "yesButtonClicked",
|
||||
})
|
||||
|
||||
const params = {
|
||||
files: [{ path: "small-file.ts", lineRanges: [] }],
|
||||
}
|
||||
|
||||
await readFileTool.execute(params, mockTask as Task, mockCallbacks)
|
||||
|
||||
// Verify that validateFileTokenBudget was called
|
||||
expect(validateFileTokenBudget).toHaveBeenCalled()
|
||||
|
||||
// Verify that no warning was shown (only regular approval)
|
||||
const askCalls = vi.mocked(mockTask.ask).mock.calls
|
||||
const warningCalls = askCalls.filter(
|
||||
(call) => call[1] && typeof call[1] === "string" && call[1].includes("File Size Warning"),
|
||||
)
|
||||
expect(warningCalls).toHaveLength(0)
|
||||
|
||||
// Verify that tool result was pushed
|
||||
expect(mockCallbacks.pushToolResult).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -290,6 +290,7 @@ export type ExtensionState = Pick<
|
|||
| "includeCurrentTime"
|
||||
| "includeCurrentCost"
|
||||
| "maxGitStatusFiles"
|
||||
| "preReadFileCheckpoint"
|
||||
> & {
|
||||
version: string
|
||||
clineMessages: ClineMessage[]
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ type ContextManagementSettingsProps = HTMLAttributes<HTMLDivElement> & {
|
|||
maxReadFileLine?: number
|
||||
maxImageFileSize?: number
|
||||
maxTotalImageSize?: number
|
||||
preReadFileCheckpoint?: boolean
|
||||
maxConcurrentFileReads?: number
|
||||
profileThresholds?: Record<string, number>
|
||||
includeDiagnosticMessages?: boolean
|
||||
|
|
@ -39,6 +40,7 @@ type ContextManagementSettingsProps = HTMLAttributes<HTMLDivElement> & {
|
|||
| "maxReadFileLine"
|
||||
| "maxImageFileSize"
|
||||
| "maxTotalImageSize"
|
||||
| "preReadFileCheckpoint"
|
||||
| "maxConcurrentFileReads"
|
||||
| "profileThresholds"
|
||||
| "includeDiagnosticMessages"
|
||||
|
|
@ -61,6 +63,7 @@ export const ContextManagementSettings = ({
|
|||
maxReadFileLine,
|
||||
maxImageFileSize,
|
||||
maxTotalImageSize,
|
||||
preReadFileCheckpoint,
|
||||
maxConcurrentFileReads,
|
||||
profileThresholds = {},
|
||||
includeDiagnosticMessages,
|
||||
|
|
|
|||
|
|
@ -187,6 +187,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
maxReadFileLine,
|
||||
maxImageFileSize,
|
||||
maxTotalImageSize,
|
||||
preReadFileCheckpoint,
|
||||
terminalCompressProgressBar,
|
||||
maxConcurrentFileReads,
|
||||
condensingApiConfigId,
|
||||
|
|
@ -395,6 +396,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
maxReadFileLine: maxReadFileLine ?? -1,
|
||||
maxImageFileSize: maxImageFileSize ?? 5,
|
||||
maxTotalImageSize: maxTotalImageSize ?? 20,
|
||||
preReadFileCheckpoint: preReadFileCheckpoint ?? false,
|
||||
maxConcurrentFileReads: cachedState.maxConcurrentFileReads ?? 5,
|
||||
includeDiagnosticMessages:
|
||||
includeDiagnosticMessages !== undefined ? includeDiagnosticMessages : true,
|
||||
|
|
@ -772,6 +774,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
|
|||
maxReadFileLine={maxReadFileLine}
|
||||
maxImageFileSize={maxImageFileSize}
|
||||
maxTotalImageSize={maxTotalImageSize}
|
||||
preReadFileCheckpoint={preReadFileCheckpoint}
|
||||
maxConcurrentFileReads={maxConcurrentFileReads}
|
||||
profileThresholds={profileThresholds}
|
||||
includeDiagnosticMessages={includeDiagnosticMessages}
|
||||
|
|
|
|||
|
|
@ -140,6 +140,8 @@ export interface ExtensionStateContextType extends ExtensionState {
|
|||
setMaxImageFileSize: (value: number) => void
|
||||
maxTotalImageSize: number
|
||||
setMaxTotalImageSize: (value: number) => void
|
||||
preReadFileCheckpoint: boolean
|
||||
setPreReadFileCheckpoint: (value: boolean) => void
|
||||
machineId?: string
|
||||
pinnedApiConfigs?: Record<string, boolean>
|
||||
setPinnedApiConfigs: (value: Record<string, boolean>) => void
|
||||
|
|
@ -243,6 +245,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
|
|||
maxReadFileLine: -1, // Default max read file line limit
|
||||
maxImageFileSize: 5, // Default max image file size in MB
|
||||
maxTotalImageSize: 20, // Default max total image size in MB
|
||||
preReadFileCheckpoint: false, // Default pre-read file checkpoint disabled
|
||||
pinnedApiConfigs: {}, // Empty object for pinned API configs
|
||||
terminalZshOhMy: false, // Default Oh My Zsh integration setting
|
||||
maxConcurrentFileReads: 5, // Default concurrent file reads
|
||||
|
|
@ -452,6 +455,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
|
|||
const contextValue: ExtensionStateContextType = {
|
||||
...state,
|
||||
reasoningBlockCollapsed: state.reasoningBlockCollapsed ?? true,
|
||||
preReadFileCheckpoint: state.preReadFileCheckpoint ?? false,
|
||||
didHydrateState,
|
||||
showWelcome,
|
||||
theme,
|
||||
|
|
@ -549,6 +553,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
|
|||
setMaxReadFileLine: (value) => setState((prevState) => ({ ...prevState, maxReadFileLine: value })),
|
||||
setMaxImageFileSize: (value) => setState((prevState) => ({ ...prevState, maxImageFileSize: value })),
|
||||
setMaxTotalImageSize: (value) => setState((prevState) => ({ ...prevState, maxTotalImageSize: value })),
|
||||
setPreReadFileCheckpoint: (value) => setState((prevState) => ({ ...prevState, preReadFileCheckpoint: value })),
|
||||
setPinnedApiConfigs: (value) => setState((prevState) => ({ ...prevState, pinnedApiConfigs: value })),
|
||||
setTerminalCompressProgressBar: (value) =>
|
||||
setState((prevState) => ({ ...prevState, terminalCompressProgressBar: value })),
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue