mirror of
https://github.com/cline/cline.git
synced 2026-10-29 17:42:34 +08:00
refactor: simplify write_to_file error handling per review
- Simplify writeToFileMissingContentError to single-tier error following existing diffError pattern (no progressive escalation) - Use shared getLastApiReqTotalTokens() for context window awareness - Remove private getContextUsagePercent() method from handler - Add proactive skeleton + replace_in_file guidance to write_to_file tool description for all variants - Simplify tests to match new API (11 tests)
This commit is contained in:
Diego Ferreyra
@@ -3,147 +3,63 @@ import "should"
|
||||
import { formatResponse } from "../responses"
|
||||
|
||||
describe("formatResponse.writeToFileMissingContentError", () => {
|
||||
describe("first failure (consecutiveFailures = 1)", () => {
|
||||
it("should include the file path in the error message", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
result.should.containEql("src/index.ts")
|
||||
})
|
||||
|
||||
it("should include the base error explanation", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
result.should.containEql("'content' parameter was empty")
|
||||
result.should.containEql("output token limits")
|
||||
})
|
||||
|
||||
it("should include helpful suggestions", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
result.should.containEql("Suggestions")
|
||||
result.should.containEql("replace_in_file")
|
||||
result.should.containEql("skeleton")
|
||||
})
|
||||
|
||||
it("should include the tool use instructions reminder", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
result.should.containEql("Reminder: Instructions for Tool Use")
|
||||
})
|
||||
|
||||
it("should NOT include context warning when no context usage provided", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
|
||||
it("should NOT include context warning when usage is below 50%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1, 30)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
|
||||
it("should include context warning when usage is above 50%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1, 75)
|
||||
result.should.containEql("Context window is 75% full")
|
||||
result.should.containEql("MUST use a strategy that produces smaller outputs")
|
||||
})
|
||||
it("should include the file path in the error message", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("src/index.ts")
|
||||
})
|
||||
|
||||
describe("second failure (consecutiveFailures = 2)", () => {
|
||||
it("should use stronger language about changing strategy", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
result.should.containEql("2nd failed attempt")
|
||||
result.should.containEql("must use a different strategy")
|
||||
})
|
||||
|
||||
it("should recommend specific alternative approaches", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
result.should.containEql("minimal skeleton")
|
||||
result.should.containEql("replace_in_file with smaller chunks")
|
||||
result.should.containEql("Break the task into smaller steps")
|
||||
})
|
||||
|
||||
it("should explicitly tell the model not to retry write_to_file", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
result.should.containEql("Do NOT attempt to write the full file content in a single write_to_file call again")
|
||||
})
|
||||
|
||||
it("should NOT include the generic tool use instructions reminder", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
result.should.not.containEql("Reminder: Instructions for Tool Use")
|
||||
})
|
||||
|
||||
it("should include context warning at high usage", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2, 85)
|
||||
result.should.containEql("Context window is 85% full")
|
||||
})
|
||||
it("should include the base error explanation", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("'content' parameter was empty")
|
||||
result.should.containEql("output token limits")
|
||||
})
|
||||
|
||||
describe("third+ failure (consecutiveFailures >= 3)", () => {
|
||||
it("should use CRITICAL language", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
result.should.containEql("CRITICAL")
|
||||
result.should.containEql("failed to write this file 3 times in a row")
|
||||
})
|
||||
|
||||
it("should tell the model to STOP using write_to_file", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
result.should.containEql("MUST change your approach")
|
||||
result.should.containEql("do NOT retry write_to_file")
|
||||
})
|
||||
|
||||
it("should provide concrete required actions", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
result.should.containEql("Create an empty file first, then use replace_in_file")
|
||||
result.should.containEql("50-100 lines")
|
||||
})
|
||||
|
||||
it("should work for 4+ failures with updated count", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 5)
|
||||
result.should.containEql("failed to write this file 5 times in a row")
|
||||
result.should.containEql("CRITICAL")
|
||||
})
|
||||
|
||||
it("should include context warning at high usage", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3, 92)
|
||||
result.should.containEql("Context window is 92% full")
|
||||
})
|
||||
it("should include helpful suggestions", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("Suggestions")
|
||||
result.should.containEql("replace_in_file")
|
||||
result.should.containEql("skeleton")
|
||||
})
|
||||
|
||||
describe("progressive escalation", () => {
|
||||
it("should produce increasingly directive messages as failures increase", () => {
|
||||
const msg1 = formatResponse.writeToFileMissingContentError("file.ts", 1)
|
||||
const msg2 = formatResponse.writeToFileMissingContentError("file.ts", 2)
|
||||
const msg3 = formatResponse.writeToFileMissingContentError("file.ts", 3)
|
||||
|
||||
// First message should have suggestions (soft guidance)
|
||||
msg1.should.containEql("Suggestions")
|
||||
msg1.should.not.containEql("CRITICAL")
|
||||
|
||||
// Second message should have stronger language
|
||||
msg2.should.containEql("Do NOT attempt")
|
||||
msg2.should.not.containEql("CRITICAL")
|
||||
|
||||
// Third message should be the most directive
|
||||
msg3.should.containEql("CRITICAL")
|
||||
msg3.should.containEql("MUST change your approach")
|
||||
})
|
||||
it("should include the tool use instructions reminder", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("Reminder: Instructions for Tool Use")
|
||||
})
|
||||
|
||||
describe("context window awareness", () => {
|
||||
it("should not show warning at exactly 50%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("file.ts", 1, 50)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
it("should mention breaking down the task into smaller steps", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("breaking down the task into smaller steps")
|
||||
})
|
||||
|
||||
it("should show warning at 51%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("file.ts", 1, 51)
|
||||
result.should.containEql("Context window is 51% full")
|
||||
})
|
||||
it("should suggest using replace_in_file for existing files", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts")
|
||||
result.should.containEql("prefer replace_in_file to make targeted edits")
|
||||
})
|
||||
|
||||
it("should show warning at 100%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("file.ts", 1, 100)
|
||||
result.should.containEql("Context window is 100% full")
|
||||
})
|
||||
it("should work with different file paths", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("components/MyComponent.tsx")
|
||||
result.should.containEql("components/MyComponent.tsx")
|
||||
})
|
||||
|
||||
it("should handle undefined context usage gracefully", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("file.ts", 1, undefined)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
// Context window awareness tests
|
||||
it("should include context window usage note when percent is provided", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 60)
|
||||
result.should.containEql("Context window is 60% full")
|
||||
result.should.containEql("Keep responses concise")
|
||||
})
|
||||
|
||||
it("should not include context window note when percent is undefined", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", undefined)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
|
||||
it("should not include context window note when percent is 0", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 0)
|
||||
result.should.not.containEql("Context window is")
|
||||
})
|
||||
|
||||
it("should show high context window usage correctly", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 85)
|
||||
result.should.containEql("Context window is 85% full")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -47,52 +47,21 @@ Otherwise, if you have not completed the task and do not need additional informa
|
||||
`Missing value for required parameter '${paramName}'. Please retry with complete response.\n\n${toolUseInstructionsReminder}`,
|
||||
|
||||
/**
|
||||
* Specialized error for write_to_file when the 'content' parameter is missing.
|
||||
* Provides progressive guidance based on how many times this has happened consecutively,
|
||||
* and includes token budget awareness to help the model understand output constraints.
|
||||
* Error for write_to_file when the 'content' parameter is missing/empty.
|
||||
* Follows the same pattern as diffError — a clear error message with actionable guidance appended as a reminder.
|
||||
* The proactive strategy guidance (skeleton + replace_in_file) lives in the tool description itself.
|
||||
* Optionally includes context window usage info to help the model gauge available space.
|
||||
*/
|
||||
writeToFileMissingContentError: (relPath: string, consecutiveFailures: number, contextUsagePercent?: number): string => {
|
||||
const baseError = `Failed to write to '${relPath}': The 'content' parameter was empty. This typically happens when the file content is too large to generate in a single response, or when output token limits are reached before the content parameter is fully written.`
|
||||
|
||||
const contextWarning =
|
||||
contextUsagePercent !== undefined && contextUsagePercent > 50
|
||||
? `\n\n⚠️ Context window is ${contextUsagePercent}% full. The remaining output budget may be insufficient for large file writes. You MUST use a strategy that produces smaller outputs.`
|
||||
: ""
|
||||
|
||||
if (consecutiveFailures >= 3) {
|
||||
// After 3+ failures, be very directive — stop trying write_to_file entirely
|
||||
return (
|
||||
`${baseError}${contextWarning}\n\n` +
|
||||
`CRITICAL: You have failed to write this file ${consecutiveFailures} times in a row. You MUST change your approach — do NOT retry write_to_file for this file again.\n\n` +
|
||||
`Required action — choose ONE of these strategies:\n` +
|
||||
`1. **Create an empty file first, then use replace_in_file** to add content in small sections (recommended)\n` +
|
||||
`2. **Break the file into multiple smaller files** if architecturally appropriate\n` +
|
||||
`3. **Write a minimal skeleton** using write_to_file (just imports, class/function signatures, no implementations), then use replace_in_file to fill in each section one at a time\n\n` +
|
||||
`Each replace_in_file call should add no more than 50-100 lines of content at a time.`
|
||||
)
|
||||
}
|
||||
if (consecutiveFailures >= 2) {
|
||||
// After 2 failures, strongly suggest alternative approaches
|
||||
return (
|
||||
`${baseError}${contextWarning}\n\n` +
|
||||
`This is your ${consecutiveFailures}${consecutiveFailures === 2 ? "nd" : "rd"} failed attempt. The file content is likely too large to generate in one response. You must use a different strategy:\n\n` +
|
||||
`Recommended approaches:\n` +
|
||||
`1. **Use write_to_file with a minimal skeleton** (just the structure — imports, class/function signatures, no implementations), then use replace_in_file to fill in each section incrementally\n` +
|
||||
`2. **Use replace_in_file with smaller chunks** — if the file already exists, make targeted edits instead of rewriting the entire file\n` +
|
||||
`3. **Break the task into smaller steps** — write one function or section at a time\n\n` +
|
||||
`Do NOT attempt to write the full file content in a single write_to_file call again.`
|
||||
)
|
||||
}
|
||||
// First failure — provide helpful guidance
|
||||
return (
|
||||
`${baseError}${contextWarning}\n\n` +
|
||||
`Suggestions:\n` +
|
||||
`- If the file is large, try breaking down the task into smaller steps. Write a skeleton first, then fill in sections using replace_in_file.\n` +
|
||||
`- If the file already exists, prefer replace_in_file to make targeted edits instead of rewriting the entire file.\n` +
|
||||
`- Ensure the 'content' parameter contains the complete file content before closing the tool tag.\n\n` +
|
||||
toolUseInstructionsReminder
|
||||
)
|
||||
},
|
||||
writeToFileMissingContentError: (relPath: string, contextUsagePercent?: number): string =>
|
||||
`Failed to write to '${relPath}': The 'content' parameter was empty. This typically happens when the file content is too large to generate in a single response, or when output token limits are reached before the content parameter is fully written.\n\n` +
|
||||
(contextUsagePercent && contextUsagePercent > 0
|
||||
? `Note: Context window is ${contextUsagePercent}% full. Keep responses concise to avoid hitting output limits.\n\n`
|
||||
: "") +
|
||||
`Suggestions:\n` +
|
||||
`- If the file is large, try breaking down the task into smaller steps. Write a skeleton first, then fill in sections using replace_in_file.\n` +
|
||||
`- If the file already exists, prefer replace_in_file to make targeted edits instead of rewriting the entire file.\n` +
|
||||
`- Ensure the 'content' parameter contains the complete file content before closing the tool tag.\n\n` +
|
||||
toolUseInstructionsReminder,
|
||||
|
||||
invalidMcpToolArgumentError: (serverName: string, toolName: string) =>
|
||||
`Invalid JSON argument used with ${serverName} for ${toolName}. Please retry with a properly formatted JSON argument.`,
|
||||
|
||||
@@ -29,7 +29,7 @@ const GENERIC: ClineToolSpec = {
|
||||
id,
|
||||
name: "write_to_file",
|
||||
description:
|
||||
"Request to write content to a file at the specified path. If the file exists, it will be overwritten with the provided content. If the file doesn't exist, it will be created. This tool will automatically create any directories needed to write the file.",
|
||||
"Request to write content to a file at the specified path. If the file exists, it will be overwritten with the provided content. If the file doesn't exist, it will be created. This tool will automatically create any directories needed to write the file. If the file content is too large to generate in a single response, write a skeleton first (just imports, class/function signatures, no implementations), then use replace_in_file to fill in each section incrementally.",
|
||||
parameters: [
|
||||
{
|
||||
name: "path",
|
||||
@@ -53,7 +53,7 @@ const NATIVE_NEXT_GEN: ClineToolSpec = {
|
||||
id,
|
||||
name: "write_to_file",
|
||||
description:
|
||||
"[IMPORTANT: Always output the absolutePath first] Request to write content to a file at the specified path. If the file exists, it will be overwritten with the provided content. If the file doesn't exist, it will be created. This tool will automatically create any directories needed to write the file.",
|
||||
"[IMPORTANT: Always output the absolutePath first] Request to write content to a file at the specified path. If the file exists, it will be overwritten with the provided content. If the file doesn't exist, it will be created. This tool will automatically create any directories needed to write the file. If the file content is too large to generate in a single response, write a skeleton first (just imports, class/function signatures, no implementations), then use replace_in_file to fill in each section incrementally.",
|
||||
parameters: [
|
||||
{
|
||||
name: "absolutePath",
|
||||
|
||||
@@ -6,6 +6,7 @@ import { formatResponse } from "@core/prompts/responses"
|
||||
import { getWorkspaceBasename, resolveWorkspacePath } from "@core/workspace"
|
||||
import { processFilesIntoText } from "@integrations/misc/extract-text"
|
||||
import { ClineSayTool } from "@shared/ExtensionMessage"
|
||||
import { getLastApiReqTotalTokens } from "@shared/getApiMetrics"
|
||||
import { fileExistsAtPath } from "@utils/fs"
|
||||
import { arePathsEqual, getReadablePath, isLocatedInWorkspace } from "@utils/path"
|
||||
import { telemetryService } from "@/services/telemetry"
|
||||
@@ -118,22 +119,15 @@ export class WriteToFileToolHandler implements IFullyManagedTool {
|
||||
config.taskState.consecutiveMistakeCount++
|
||||
await config.services.diffViewProvider.reset()
|
||||
|
||||
// Use smarter error with progressive guidance and token budget awareness
|
||||
const relPath = rawRelPath || "unknown"
|
||||
const contextUsagePercent = this.getContextUsagePercent(config)
|
||||
const errorMessage = formatResponse.writeToFileMissingContentError(
|
||||
relPath,
|
||||
config.taskState.consecutiveMistakeCount,
|
||||
contextUsagePercent,
|
||||
)
|
||||
const contextWindow = config.api.getModel().info.contextWindow ?? 128_000
|
||||
const lastApiReqTotalTokens = getLastApiReqTotalTokens(config.messageState.getClineMessages())
|
||||
const contextUsagePercent = contextWindow > 0 ? Math.round((lastApiReqTotalTokens / contextWindow) * 100) : undefined
|
||||
const errorMessage = formatResponse.writeToFileMissingContentError(relPath, contextUsagePercent)
|
||||
|
||||
await config.callbacks.say(
|
||||
"error",
|
||||
`Cline tried to use write_to_file for '${relPath}' without value for required parameter 'content'. ${
|
||||
config.taskState.consecutiveMistakeCount >= 2
|
||||
? "This has happened multiple times — Cline will try a different approach."
|
||||
: "Retrying..."
|
||||
}`,
|
||||
`Cline tried to use write_to_file for '${relPath}' without value for required parameter 'content'. Retrying...`,
|
||||
)
|
||||
return formatResponse.toolError(errorMessage)
|
||||
}
|
||||
@@ -515,35 +509,6 @@ export class WriteToFileToolHandler implements IFullyManagedTool {
|
||||
return { relPath, absolutePath, fileExists, diff, content, newContent, workspaceContext, matchIndices }
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the approximate context window usage percentage.
|
||||
* Uses the last api_req_started message to get token counts and the model's context window size.
|
||||
* Returns undefined if the information is not available.
|
||||
*/
|
||||
private getContextUsagePercent(config: TaskConfig): number | undefined {
|
||||
try {
|
||||
const clineMessages = config.messageState.getClineMessages()
|
||||
const modelInfo = config.api.getModel()
|
||||
const contextWindow = modelInfo.info.contextWindow ?? 128_000
|
||||
|
||||
// Find the last api_req_started message with token data
|
||||
for (let i = clineMessages.length - 1; i >= 0; i--) {
|
||||
const msg = clineMessages[i]
|
||||
if (msg.say === "api_req_started" && msg.text) {
|
||||
const parsed = JSON.parse(msg.text)
|
||||
const totalTokens =
|
||||
(parsed.tokensIn || 0) + (parsed.tokensOut || 0) + (parsed.cacheWrites || 0) + (parsed.cacheReads || 0)
|
||||
if (totalTokens > 0) {
|
||||
return Math.round((totalTokens / contextWindow) * 100)
|
||||
}
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
} catch {
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
private getModelInfo(config: TaskConfig) {
|
||||
// Extract provider information for telemetry
|
||||
const apiConfig = config.services.stateManager.getApiConfiguration()
|
||||
|
||||
Reference in New Issue
Block a user