From c19f53b8624a31b01bdd1dea3def0a895de480bb Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 23 Dec 2025 18:16:58 -0700 Subject: [PATCH] fix: use edit_file_gemini for tool analytics and standardize editToolVariant - Change EditFileGeminiTool to use 'edit_file_gemini' as tool name for proper analytics tracking instead of generic 'edit_file' - Fix EditFileAnthropicTool handleError to use 'edit_file_anthropic' instead of 'search and replace' - Remove duplicate recordToolUsage call from EditFileAnthropicTool (already recorded by presentAssistantMessage) - Standardize editToolVariant to 'gemini' for Minimax, xAI/Grok, and ZAI models - Add missing editToolVariant to ZAI model configs - Update tests to use correct tool variant names --- packages/types/src/providers/minimax.ts | 6 ++--- packages/types/src/providers/xai.ts | 16 ++++++------- packages/types/src/providers/zai.ts | 17 +++++++++++++ .../providers/fetchers/__tests__/roo.spec.ts | 4 ++-- .../presentAssistantMessage.ts | 2 +- src/core/tools/EditFileAnthropicTool.ts | 22 ++++++++--------- src/core/tools/EditFileGeminiTool.ts | 24 +++++++++---------- src/core/tools/__tests__/editFileTool.spec.ts | 18 +++++++------- 8 files changed, 62 insertions(+), 47 deletions(-) diff --git a/packages/types/src/providers/minimax.ts b/packages/types/src/providers/minimax.ts index b824c270fd..834deba1f1 100644 --- a/packages/types/src/providers/minimax.ts +++ b/packages/types/src/providers/minimax.ts @@ -15,7 +15,7 @@ export const minimaxModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", - editToolVariant: "anthropic", + editToolVariant: "gemini", preserveReasoning: true, inputPrice: 0.3, outputPrice: 1.2, @@ -31,7 +31,7 @@ export const minimaxModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", - editToolVariant: "anthropic", + editToolVariant: "gemini", preserveReasoning: true, inputPrice: 0.3, outputPrice: 1.2, @@ -47,7 +47,7 @@ export const minimaxModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", - editToolVariant: "anthropic", + editToolVariant: "gemini", preserveReasoning: true, inputPrice: 0.3, outputPrice: 1.2, diff --git a/packages/types/src/providers/xai.ts b/packages/types/src/providers/xai.ts index 2a3b786f61..b6cf125d69 100644 --- a/packages/types/src/providers/xai.ts +++ b/packages/types/src/providers/xai.ts @@ -18,7 +18,7 @@ export const xaiModels = { cacheWritesPrice: 0.02, cacheReadsPrice: 0.02, description: "xAI's Grok Code Fast model with 256K context window", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-4-1-fast-reasoning": { maxTokens: 65_536, @@ -33,7 +33,7 @@ export const xaiModels = { cacheReadsPrice: 0.05, description: "xAI's Grok 4.1 Fast model with 2M context window, optimized for high-performance agentic tool calling with reasoning", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-4-1-fast-non-reasoning": { maxTokens: 65_536, @@ -48,7 +48,7 @@ export const xaiModels = { cacheReadsPrice: 0.05, description: "xAI's Grok 4.1 Fast model with 2M context window, optimized for high-performance agentic tool calling", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-4-fast-reasoning": { maxTokens: 65_536, @@ -63,7 +63,7 @@ export const xaiModels = { cacheReadsPrice: 0.05, description: "xAI's Grok 4 Fast model with 2M context window, optimized for high-performance agentic tool calling with reasoning", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-4-fast-non-reasoning": { maxTokens: 65_536, @@ -78,7 +78,7 @@ export const xaiModels = { cacheReadsPrice: 0.05, description: "xAI's Grok 4 Fast model with 2M context window, optimized for high-performance agentic tool calling", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-4-0709": { maxTokens: 8192, @@ -92,7 +92,7 @@ export const xaiModels = { cacheWritesPrice: 0.75, cacheReadsPrice: 0.75, description: "xAI's Grok-4 model with 256K context window", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-3-mini": { maxTokens: 8192, @@ -108,7 +108,7 @@ export const xaiModels = { description: "xAI's Grok-3 mini model with 128K context window", supportsReasoningEffort: ["low", "high"], reasoningEffort: "low", - editToolVariant: "grok", + editToolVariant: "gemini", }, "grok-3": { maxTokens: 8192, @@ -122,6 +122,6 @@ export const xaiModels = { cacheWritesPrice: 0.75, cacheReadsPrice: 0.75, description: "xAI's Grok-3 model with 128K context window", - editToolVariant: "grok", + editToolVariant: "gemini", }, } as const satisfies Record diff --git a/packages/types/src/providers/zai.ts b/packages/types/src/providers/zai.ts index 93cf9bb23b..045f988094 100644 --- a/packages/types/src/providers/zai.ts +++ b/packages/types/src/providers/zai.ts @@ -18,6 +18,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.6, outputPrice: 2.2, cacheWritesPrice: 0, @@ -32,6 +33,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.2, outputPrice: 1.1, cacheWritesPrice: 0, @@ -46,6 +48,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 2.2, outputPrice: 8.9, cacheWritesPrice: 0, @@ -60,6 +63,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 1.1, outputPrice: 4.5, cacheWritesPrice: 0, @@ -73,6 +77,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0, outputPrice: 0, cacheWritesPrice: 0, @@ -86,6 +91,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.6, outputPrice: 1.8, cacheWritesPrice: 0, @@ -100,6 +106,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.6, outputPrice: 2.2, cacheWritesPrice: 0, @@ -114,6 +121,7 @@ export const internationalZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", supportsReasoningEffort: ["disable", "medium"], reasoningEffort: "medium", preserveReasoning: true, @@ -131,6 +139,7 @@ export const internationalZAiModels = { supportsPromptCache: false, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.1, outputPrice: 0.1, cacheWritesPrice: 0, @@ -149,6 +158,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.29, outputPrice: 1.14, cacheWritesPrice: 0, @@ -163,6 +173,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.1, outputPrice: 0.6, cacheWritesPrice: 0, @@ -177,6 +188,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.29, outputPrice: 1.14, cacheWritesPrice: 0, @@ -191,6 +203,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.1, outputPrice: 0.6, cacheWritesPrice: 0, @@ -204,6 +217,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0, outputPrice: 0, cacheWritesPrice: 0, @@ -217,6 +231,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.29, outputPrice: 0.93, cacheWritesPrice: 0, @@ -231,6 +246,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", inputPrice: 0.29, outputPrice: 1.14, cacheWritesPrice: 0, @@ -245,6 +261,7 @@ export const mainlandZAiModels = { supportsPromptCache: true, supportsNativeTools: true, defaultToolProtocol: "native", + editToolVariant: "gemini", supportsReasoningEffort: ["disable", "medium"], reasoningEffort: "medium", preserveReasoning: true, diff --git a/src/api/providers/fetchers/__tests__/roo.spec.ts b/src/api/providers/fetchers/__tests__/roo.spec.ts index f52ee853c1..f18cad3d2e 100644 --- a/src/api/providers/fetchers/__tests__/roo.spec.ts +++ b/src/api/providers/fetchers/__tests__/roo.spec.ts @@ -830,7 +830,7 @@ describe("getRooModels", () => { // Versioned settings keyed by version number (low version - always met) versionedSettings: { "1.0.0": { - editToolVariant: "grok", + editToolVariant: "gemini", excludedTools: ["write_to_file", "browser_action"], }, }, @@ -875,7 +875,7 @@ describe("getRooModels", () => { // Versioned settings keyed by very high version - never met versionedSettings: { "99.0.0": { - editToolVariant: "grok", + editToolVariant: "gemini", }, }, }, diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 037f14ccce..8dea21a840 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -944,7 +944,7 @@ export async function presentAssistantMessage(cline: Task) { break case "gemini": // Route to Gemini variant (search/replace with expected_replacements) - await editFileGeminiTool.handle(cline, block as ToolUse<"edit_file">, { + await editFileGeminiTool.handle(cline, block as ToolUse<"edit_file_gemini">, { askApproval, handleError, pushToolResult, diff --git a/src/core/tools/EditFileAnthropicTool.ts b/src/core/tools/EditFileAnthropicTool.ts index b0902a7f42..2a16ca7a0f 100644 --- a/src/core/tools/EditFileAnthropicTool.ts +++ b/src/core/tools/EditFileAnthropicTool.ts @@ -245,7 +245,8 @@ export class SearchAndReplaceTool extends BaseTool<"edit_file_anthropic"> { } task.didEditFile = true - task.recordToolUsage("edit_file_anthropic") + // Tool usage metrics are recorded by presentAssistantMessage(), which also derives provider variants. + // Recording here would double-count successful runs and skew metrics. // Get the formatted response message const message = await task.diffViewProvider.pushToolWriteResult(task, task.cwd, false) @@ -262,25 +263,22 @@ export class SearchAndReplaceTool extends BaseTool<"edit_file_anthropic"> { // Process any queued messages after file edit completes task.processQueuedMessages() } catch (error) { - await handleError("search and replace", error as Error) + await handleError("edit_file_anthropic", error as Error) await task.diffViewProvider.reset() } } override async handlePartial(task: Task, block: ToolUse<"edit_file_anthropic">): Promise { const relPath: string | undefined = block.params.path - const editsStr: string | undefined = block.params.edits + // For native protocol, nativeArgs contains the edits array + // For XML protocol, edits would be in params (but this tool is native-only) + const nativeArgs = block.nativeArgs as + | { path: string; edits: Array<{ old_text: string; new_text: string }> } + | undefined let editsPreview: string | undefined - if (editsStr) { - try { - const ops = JSON.parse(editsStr) - if (Array.isArray(ops) && ops.length > 0) { - editsPreview = `${ops.length} edit(s)` - } - } catch { - editsPreview = "parsing..." - } + if (nativeArgs?.edits && Array.isArray(nativeArgs.edits)) { + editsPreview = `${nativeArgs.edits.length} edit(s)` } const absolutePath = relPath ? path.resolve(task.cwd, relPath) : "" diff --git a/src/core/tools/EditFileGeminiTool.ts b/src/core/tools/EditFileGeminiTool.ts index 3072c6f86e..6532cd9d86 100644 --- a/src/core/tools/EditFileGeminiTool.ts +++ b/src/core/tools/EditFileGeminiTool.ts @@ -90,8 +90,8 @@ function applyReplacement( return safeLiteralReplace(currentContent, oldString, newString) } -export class EditFileTool extends BaseTool<"edit_file"> { - readonly name = "edit_file" as const +export class EditFileTool extends BaseTool<"edit_file_gemini"> { + readonly name = "edit_file_gemini" as const parseLegacy(params: Partial>): EditFileParams { return { @@ -112,8 +112,8 @@ export class EditFileTool extends BaseTool<"edit_file"> { // Validate required parameters if (!file_path) { task.consecutiveMistakeCount++ - task.recordToolError("edit_file") - pushToolResult(await task.sayAndCreateMissingParamError("edit_file", "file_path")) + task.recordToolError("edit_file_gemini") + pushToolResult(await task.sayAndCreateMissingParamError("edit_file_gemini", "file_path")) return } @@ -150,7 +150,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { currentContent = currentContent.replace(/\r\n/g, "\n") } catch (error) { task.consecutiveMistakeCount++ - task.recordToolError("edit_file") + task.recordToolError("edit_file_gemini") const errorMessage = `Failed to read file '${relPath}'. Please verify file permissions and try again.` await task.say("error", errorMessage) pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) @@ -160,7 +160,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { // Check if trying to create a file that already exists if (old_string === "") { task.consecutiveMistakeCount++ - task.recordToolError("edit_file") + task.recordToolError("edit_file_gemini") const errorMessage = `File '${relPath}' already exists. Cannot create a new file with empty old_string when file exists.` await task.say("error", errorMessage) pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) @@ -174,7 +174,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { } else { // Trying to replace in non-existent file task.consecutiveMistakeCount++ - task.recordToolError("edit_file") + task.recordToolError("edit_file_gemini") const errorMessage = `File not found: ${relPath}. Cannot perform replacement on a non-existent file. Use an empty old_string to create a new file.` await task.say("error", errorMessage) pushToolResult(formatResponse.toolError(errorMessage, toolProtocol)) @@ -189,7 +189,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { if (occurrences === 0) { task.consecutiveMistakeCount++ - task.recordToolError("edit_file", "no_match") + task.recordToolError("edit_file_gemini", "no_match") pushToolResult( formatResponse.toolError( `No match found for the specified 'old_string'. Please ensure it matches the file contents exactly, including all whitespace and indentation.`, @@ -201,7 +201,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { if (occurrences !== expected_replacements) { task.consecutiveMistakeCount++ - task.recordToolError("edit_file", "occurrence_mismatch") + task.recordToolError("edit_file_gemini", "occurrence_mismatch") pushToolResult( formatResponse.toolError( `Expected ${expected_replacements} occurrence(s) but found ${occurrences}. Please adjust your old_string to match exactly ${expected_replacements} occurrence(s), or set expected_replacements to ${occurrences}.`, @@ -214,7 +214,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { // Validate that old_string and new_string are different if (old_string === new_string) { task.consecutiveMistakeCount++ - task.recordToolError("edit_file") + task.recordToolError("edit_file_gemini") pushToolResult( formatResponse.toolError( "No changes to apply. The old_string and new_string are identical.", @@ -329,12 +329,12 @@ export class EditFileTool extends BaseTool<"edit_file"> { // Process any queued messages after file edit completes task.processQueuedMessages() } catch (error) { - await handleError("edit_file", error as Error) + await handleError("edit_file_gemini", error as Error) await task.diffViewProvider.reset() } } - override async handlePartial(task: Task, block: ToolUse<"edit_file">): Promise { + override async handlePartial(task: Task, block: ToolUse<"edit_file_gemini">): Promise { const filePath: string | undefined = block.params.file_path const oldString: string | undefined = block.params.old_string diff --git a/src/core/tools/__tests__/editFileTool.spec.ts b/src/core/tools/__tests__/editFileTool.spec.ts index 59bff54f0f..b925f7139a 100644 --- a/src/core/tools/__tests__/editFileTool.spec.ts +++ b/src/core/tools/__tests__/editFileTool.spec.ts @@ -179,7 +179,7 @@ describe("editFileTool", () => { const toolUse: ToolUse = { type: "tool_use", - name: "edit_file", + name: "edit_file_gemini", params: { file_path: testFilePath, old_string: testOldString, @@ -193,7 +193,7 @@ describe("editFileTool", () => { toolResult = result }) - await editFileTool.handle(mockTask, toolUse as ToolUse<"edit_file">, { + await editFileTool.handle(mockTask, toolUse as ToolUse<"edit_file_gemini">, { askApproval: mockAskApproval, handleError: mockHandleError, pushToolResult: mockPushToolResult, @@ -210,7 +210,7 @@ describe("editFileTool", () => { expect(result).toBe("Missing param error") expect(mockTask.consecutiveMistakeCount).toBe(1) - expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file") + expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file_gemini") }) it("treats undefined new_string as empty string (deletion)", async () => { @@ -268,7 +268,7 @@ describe("editFileTool", () => { expect(result).toContain("Error:") expect(result).toContain("No match found") expect(mockTask.consecutiveMistakeCount).toBe(1) - expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file", "no_match") + expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file_gemini", "no_match") }) it("returns error when occurrence count does not match expected_replacements", async () => { @@ -280,7 +280,7 @@ describe("editFileTool", () => { expect(result).toContain("Error:") expect(result).toContain("Expected 1 occurrence(s) but found 3") expect(mockTask.consecutiveMistakeCount).toBe(1) - expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file", "occurrence_mismatch") + expect(mockTask.recordToolError).toHaveBeenCalledWith("edit_file_gemini", "occurrence_mismatch") }) it("succeeds when occurrence count matches expected_replacements", async () => { @@ -380,9 +380,9 @@ describe("editFileTool", () => { it("handles file read errors gracefully", async () => { mockedFsReadFile.mockRejectedValueOnce(new Error("Read failed")) - const toolUse: ToolUse = { + const toolUse: ToolUse<"edit_file_gemini"> = { type: "tool_use", - name: "edit_file", + name: "edit_file_gemini", params: { file_path: testFilePath, old_string: testOldString, @@ -396,7 +396,7 @@ describe("editFileTool", () => { capturedResult = result }) - await editFileTool.handle(mockTask, toolUse as ToolUse<"edit_file">, { + await editFileTool.handle(mockTask, toolUse, { askApproval: mockAskApproval, handleError: mockHandleError, pushToolResult: localPushToolResult, @@ -414,7 +414,7 @@ describe("editFileTool", () => { await executeEditFileTool() - expect(mockHandleError).toHaveBeenCalledWith("edit_file", expect.any(Error)) + expect(mockHandleError).toHaveBeenCalledWith("edit_file_gemini", expect.any(Error)) expect(mockTask.diffViewProvider.reset).toHaveBeenCalled() }) })