Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/types/src/experiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js"
* ExperimentId
*/

export const experimentIds = ["preventFocusDisruption", "imageGeneration", "runSlashCommand", "customTools"] as const
export const experimentIds = [
"preventFocusDisruption",
"imageGeneration",
"runSlashCommand",
"customTools",
"reAnchorBeforeEdit",
] as const

export const experimentIdsSchema = z.enum(experimentIds)

Expand All @@ -21,6 +27,7 @@ export const experimentsSchema = z.object({
imageGeneration: z.boolean().optional(),
runSlashCommand: z.boolean().optional(),
customTools: z.boolean().optional(),
reAnchorBeforeEdit: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
47 changes: 19 additions & 28 deletions src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,30 @@
import { getToolUseGuidelinesSection } from "../tool-use-guidelines"

describe("getToolUseGuidelinesSection", () => {
it("should include proper numbered guidelines", () => {
const guidelines = getToolUseGuidelinesSection()

expect(guidelines).toContain("1. Assess what information")
expect(guidelines).toContain("2. Choose the most appropriate tool")
expect(guidelines).toContain("3. If multiple actions are needed")
})

it("should include multiple-tools-per-message guidance", () => {
const guidelines = getToolUseGuidelinesSection()

expect(guidelines).toContain("you may use multiple tools in a single message")
expect(guidelines).not.toContain("use one tool at a time per message")
it("returns base guidelines without re-anchor text when no experiments provided", () => {
const result = getToolUseGuidelinesSection()
expect(result).toContain("Tool Use Guidelines")
expect(result).toContain("Assess what information you already have")
expect(result).not.toContain("Re-anchor before every file edit")
})

it("should use simplified footer without step-by-step language", () => {
const guidelines = getToolUseGuidelinesSection()

expect(guidelines).toContain("carefully considering the user's response after tool executions")
expect(guidelines).not.toContain("It is crucial to proceed step-by-step")
expect(guidelines).not.toContain("ALWAYS wait for user confirmation after each tool use")
it("returns base guidelines without re-anchor text when experiment is disabled", () => {
const result = getToolUseGuidelinesSection({ reAnchorBeforeEdit: false })
expect(result).toContain("Tool Use Guidelines")
expect(result).not.toContain("Re-anchor before every file edit")
})

it("should include common guidance", () => {
const guidelines = getToolUseGuidelinesSection()
expect(guidelines).toContain("Assess what information you already have")
expect(guidelines).toContain("Choose the most appropriate tool")
expect(guidelines).not.toContain("<actual_tool_name>")
it("includes re-anchor guideline when experiment is enabled", () => {
const result = getToolUseGuidelinesSection({ reAnchorBeforeEdit: true })
expect(result).toContain("Tool Use Guidelines")
expect(result).toContain("Re-anchor before every file edit")
expect(result).toContain("re-read the file")
expect(result).toContain("read_file")
})

it("should not include per-tool confirmation guidelines", () => {
const guidelines = getToolUseGuidelinesSection()

expect(guidelines).not.toContain("After each tool use, the user will respond with the result")
it("returns base guidelines when empty experiments object provided", () => {
const result = getToolUseGuidelinesSection({})
expect(result).toContain("Tool Use Guidelines")
expect(result).not.toContain("Re-anchor before every file edit")
})
})
17 changes: 15 additions & 2 deletions src/core/prompts/sections/tool-use-guidelines.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
export function getToolUseGuidelinesSection(): string {
import type { Experiments } from "@roo-code/types"

import { EXPERIMENT_IDS, experiments } from "../../../shared/experiments"

export function getToolUseGuidelinesSection(experimentsConfig?: Experiments): string {
const reAnchorEnabled = experimentsConfig
? experiments.isEnabled(experimentsConfig, EXPERIMENT_IDS.RE_ANCHOR_BEFORE_EDIT)
: false

const reAnchorGuideline = reAnchorEnabled
? `
4. **Re-anchor before every file edit:** Before modifying any existing file, you MUST re-read the file (or the relevant section) using the read_file tool to confirm its current contents. This re-anchoring step is critical to avoid edits based on stale or hallucinated content. The re-read must happen immediately before the edit tool call, even if you have read the file earlier in the conversation. This applies to all file editing tools (apply_diff, edit_file, edit, search_replace, write_to_file for existing files, apply_patch for modifications). When the file is large, use read_file with an appropriate offset/limit or indentation mode to read the specific section you plan to modify. Skipping this step is the most common cause of failed edits.`
: ""

return `# Tool Use Guidelines

1. Assess what information you already have and what information you need to proceed with the task.
2. Choose the most appropriate tool based on the task and the tool descriptions provided. Assess if you need additional information to proceed, and which of the available tools would be most effective for gathering this information. For example using the list_files tool is more effective than running a command like \`ls\` in the terminal. It's critical that you think about each available tool and use the one that best fits the current step in the task.
3. If multiple actions are needed, you may use multiple tools in a single message when appropriate, or use tools iteratively across messages. Each tool use should be informed by the results of previous tool uses. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result.
3. If multiple actions are needed, you may use multiple tools in a single message when appropriate, or use tools iteratively across messages. Each tool use should be informed by the results of previous tool uses. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result.${reAnchorGuideline}

By carefully considering the user's response after tool executions, you can react accordingly and make informed decisions about how to proceed with the task. This iterative process helps ensure the overall success and accuracy of your work.`
}
2 changes: 1 addition & 1 deletion src/core/prompts/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ ${markdownFormattingSection()}

${getSharedToolUseSection()}${toolsCatalog}

${getToolUseGuidelinesSection()}
${getToolUseGuidelinesSection(experiments)}

${getCapabilitiesSection(cwd, shouldIncludeMcp ? mcpHub : undefined)}

Expand Down
46 changes: 42 additions & 4 deletions src/core/tools/ApplyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

import { getReadablePath } from "../../utils/path"
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
import { Task } from "../task/Task"
import { formatResponse } from "../prompts/responses"
import { fileExistsAtPath } from "../../utils/fs"
Expand All @@ -24,6 +25,9 @@ interface ApplyDiffParams {
export class ApplyDiffTool extends BaseTool<"apply_diff"> {
readonly name = "apply_diff" as const

private didSendPartialToolAsk = false
private partialToolAskRelPath: string | undefined

async execute(params: ApplyDiffParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
const { askApproval, handleError, pushToolResult } = callbacks
let { path: relPath, diff: diffContent } = params
Expand All @@ -32,24 +36,47 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
diffContent = unescapeHtmlEntities(diffContent)
}

// Finalize any outstanding partial tool ask so the UI spinner doesn't get stuck.
const finalizePartialToolAskIfNeeded = async (currentRelPath?: string): Promise<void> => {
if (!this.didSendPartialToolAsk) {
return
}

if (this.partialToolAskRelPath && currentRelPath && this.partialToolAskRelPath !== currentRelPath) {
return
}

const sharedMessageProps: ClineSayTool = {
tool: "appliedDiff",
path: getReadablePath(task.cwd, currentRelPath ?? relPath ?? ""),
diff: diffContent,
}

// Finalize the existing partial tool ask row so the UI doesn't get stuck in a spinner state.
await task.ask("tool", JSON.stringify(sharedMessageProps), false).catch(() => {})
}

try {
if (!relPath) {
task.consecutiveMistakeCount++
task.recordToolError("apply_diff")
await finalizePartialToolAskIfNeeded()
pushToolResult(await task.sayAndCreateMissingParamError("apply_diff", "path"))
return
}

if (!diffContent) {
task.consecutiveMistakeCount++
task.recordToolError("apply_diff")
await finalizePartialToolAskIfNeeded()
pushToolResult(await task.sayAndCreateMissingParamError("apply_diff", "diff"))
return
}

const accessAllowed = task.rooIgnoreController?.validateAccess(relPath)

if (!accessAllowed) {
await finalizePartialToolAskIfNeeded(relPath)
await task.say("rooignore_error", relPath)
pushToolResult(formatResponse.rooIgnoreError(relPath))
return
Expand All @@ -61,6 +88,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
if (!fileExists) {
task.consecutiveMistakeCount++
task.recordToolError("apply_diff")
await finalizePartialToolAskIfNeeded(relPath)
const formattedError = `File does not exist at path: ${absolutePath}\n\n<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>`
await task.say("error", formattedError)
task.didToolFailInCurrentTurn = true
Expand Down Expand Up @@ -107,11 +135,14 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
}

if (currentCount >= 2) {
await task.say("diff_error", formattedError)
}
// Always finalize any pending partial ask to stop the spinner.
await finalizePartialToolAskIfNeeded(relPath)

// Always show diff_error on every failure so the user gets immediate visual feedback.
await task.say("diff_error", formattedError)

task.recordToolError("apply_diff", formattedError)
task.didToolFailInCurrentTurn = true

pushToolResult(formattedError)
return
Expand Down Expand Up @@ -261,11 +292,15 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {

return
} catch (error) {
await finalizePartialToolAskIfNeeded(relPath)
await handleError("applying diff", error as Error)
await task.diffViewProvider.reset()
this.resetPartialState()
task.processQueuedMessages()
return
} finally {
this.didSendPartialToolAsk = false
this.partialToolAskRelPath = undefined
this.resetPartialState()
}
}

Expand All @@ -278,6 +313,9 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
return
}

this.didSendPartialToolAsk = true
this.partialToolAskRelPath = relPath

const sharedMessageProps: ClineSayTool = {
tool: "appliedDiff",
path: getReadablePath(task.cwd, relPath),
Expand Down
5 changes: 2 additions & 3 deletions src/core/tools/EditFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,8 @@ export class EditFileTool extends BaseTool<"edit_file"> {
const currentCount = (task.consecutiveMistakeCountForEditFile.get(relPath) || 0) + 1
task.consecutiveMistakeCountForEditFile.set(relPath, currentCount)

if (currentCount >= 2) {
await task.say("diff_error", formattedError)
}
// Always show diff_error on every failure so the user gets immediate visual feedback.
await task.say("diff_error", formattedError)
}

try {
Expand Down
15 changes: 9 additions & 6 deletions src/core/tools/__tests__/editFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,11 +434,11 @@ describe("editFileTool", () => {
})

describe("consecutive error display behavior", () => {
it("does NOT show diff_error to user on first no_match failure", async () => {
it("shows diff_error to user on first no_match failure", async () => {
await executeEditFileTool({ old_string: "NonExistent" }, { fileContent: "Line 1\nLine 2\nLine 3" })

expect(mockTask.consecutiveMistakeCountForEditFile.get(testFilePath)).toBe(1)
expect(mockTask.say).not.toHaveBeenCalledWith("diff_error", expect.any(String))
expect(mockTask.say).toHaveBeenCalledWith("diff_error", expect.stringContaining("No match found"))
expect(mockTask.recordToolError).toHaveBeenCalledWith(
"edit_file",
expect.stringContaining("No match found"),
Expand All @@ -456,14 +456,17 @@ describe("editFileTool", () => {
expect(mockTask.say).toHaveBeenCalledWith("diff_error", expect.stringContaining("No match found"))
})

it("does NOT show diff_error to user on first occurrence_mismatch failure", async () => {
it("shows diff_error to user on first occurrence_mismatch failure", async () => {
await executeEditFileTool(
{ old_string: "Line", expected_replacements: "1" },
{ fileContent: "Line 1\nLine 2\nLine 3" },
)

expect(mockTask.consecutiveMistakeCountForEditFile.get(testFilePath)).toBe(1)
expect(mockTask.say).not.toHaveBeenCalledWith("diff_error", expect.any(String))
expect(mockTask.say).toHaveBeenCalledWith(
"diff_error",
expect.stringContaining("Occurrence count mismatch"),
)
expect(mockTask.recordToolError).toHaveBeenCalledWith(
"edit_file",
expect.stringContaining("Occurrence count mismatch"),
Expand Down Expand Up @@ -522,8 +525,8 @@ describe("editFileTool", () => {
expect(mockTask.consecutiveMistakeCountForEditFile.get(testFilePath)).toBe(1)
expect(mockTask.consecutiveMistakeCountForEditFile.get(otherFilePath)).toBe(1)

// Neither should have triggered diff_error display
expect(mockTask.say).not.toHaveBeenCalledWith("diff_error", expect.any(String))
// Both should have triggered diff_error display (errors are shown on every failure now)
expect(mockTask.say).toHaveBeenCalledWith("diff_error", expect.any(String))
})
})

Expand Down
34 changes: 34 additions & 0 deletions src/shared/__tests__/experiments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,45 @@ describe("experiments", () => {
})
})

describe("RE_ANCHOR_BEFORE_EDIT", () => {
it("is configured correctly", () => {
expect(EXPERIMENT_IDS.RE_ANCHOR_BEFORE_EDIT).toBe("reAnchorBeforeEdit")
expect(experimentConfigsMap.RE_ANCHOR_BEFORE_EDIT).toMatchObject({
enabled: false,
})
})

it("returns false when not enabled", () => {
const experiments: Record<ExperimentId, boolean> = {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
customTools: false,
reAnchorBeforeEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.RE_ANCHOR_BEFORE_EDIT)).toBe(false)
})

it("returns true when enabled", () => {
const experiments: Record<ExperimentId, boolean> = {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
customTools: false,
reAnchorBeforeEdit: true,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.RE_ANCHOR_BEFORE_EDIT)).toBe(true)
})
})

describe("isEnabled", () => {
it("returns false when experiment is not enabled", () => {
const experiments: Record<ExperimentId, boolean> = {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
customTools: false,
reAnchorBeforeEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(false)
})
Expand All @@ -31,6 +63,7 @@ describe("experiments", () => {
imageGeneration: false,
runSlashCommand: false,
customTools: false,
reAnchorBeforeEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(true)
})
Expand All @@ -41,6 +74,7 @@ describe("experiments", () => {
imageGeneration: false,
runSlashCommand: false,
customTools: false,
reAnchorBeforeEdit: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(false)
})
Expand Down
2 changes: 2 additions & 0 deletions src/shared/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const EXPERIMENT_IDS = {
IMAGE_GENERATION: "imageGeneration",
RUN_SLASH_COMMAND: "runSlashCommand",
CUSTOM_TOOLS: "customTools",
RE_ANCHOR_BEFORE_EDIT: "reAnchorBeforeEdit",
} as const satisfies Record<string, ExperimentId>

type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
Expand All @@ -20,6 +21,7 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
IMAGE_GENERATION: { enabled: false },
RUN_SLASH_COMMAND: { enabled: false },
CUSTOM_TOOLS: { enabled: false },
RE_ANCHOR_BEFORE_EDIT: { enabled: false },
}

export const experimentDefault = Object.fromEntries(
Expand Down
4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/en/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,10 @@
"refreshSuccess": "Tools refreshed successfully",
"refreshError": "Failed to refresh tools",
"toolParameters": "Parameters"
},
"RE_ANCHOR_BEFORE_EDIT": {
"name": "Re-anchor before file edits",
"description": "When enabled, the system prompt instructs the model to always re-read a file before editing it. This reduces hallucination-based edit failures by ensuring the model works with up-to-date file contents. Uses more tokens but significantly improves edit reliability."
}
},
"promptCaching": {
Expand Down
Loading