diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 918041e459..61e7897d9b 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -165,6 +165,7 @@ export class Task extends EventEmitter { consecutiveMistakeCount: number = 0 consecutiveMistakeLimit: number consecutiveMistakeCountForApplyDiff: Map = new Map() + consecutiveMistakeCountForInsertContent: Map = new Map() toolUsage: ToolUsage = {} // Checkpoints diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index 9fbe1e1de1..569e70e94a 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -12,6 +12,20 @@ import { addLineNumbers } from "../../integrations/misc/extract-text" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { telemetryService } from "../../services/telemetry/TelemetryService" import { unescapeHtmlEntities } from "../../utils/text-normalization" +import { inspect } from "util" + +// Find the last complete REPLACE marker +function processDiffForStreaming(diffContent: string): string { + const lastReplaceIndex = diffContent.lastIndexOf(">>>>>>> REPLACE") + if (lastReplaceIndex === -1) { + // No complete sections yet + return "" + } + + // Only include content up to and including the last REPLACE marker + diffContent = diffContent.substring(0, lastReplaceIndex + ">>>>>>> REPLACE".length) + return diffContent +} export async function applyDiffTool( cline: Task, @@ -35,24 +49,39 @@ export async function applyDiffTool( } try { + // 2. Handle ongoing streaming updates if (block.partial) { - // Update GUI message let toolProgressStatus - - if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) { + if (cline.diffStrategy?.getProgressStatus) { toolProgressStatus = cline.diffStrategy.getProgressStatus(block) + if (toolProgressStatus && Object.keys(toolProgressStatus).length === 0) { + return + } } - if (toolProgressStatus && Object.keys(toolProgressStatus).length === 0) { + if (!relPath || !(await fileExistsAtPath(path.resolve(cline.cwd, relPath)))) { return } - await cline - .ask("tool", JSON.stringify(sharedMessageProps), block.partial, toolProgressStatus) - .catch(() => {}) - - return + if (diffContent) { + const processedContent = processDiffForStreaming(diffContent) + if (!processedContent) { + return + } + diffContent = processedContent + + await cline + .ask( + "tool", + JSON.stringify({ ...sharedMessageProps, diff: diffContent }), + block.partial, + toolProgressStatus, + ) + .catch(() => {}) + } } else { + // 3. For completed model output, all parameters must be valid + // Unlike streaming mode, we report errors since the model output is complete if (!relPath) { cline.consecutiveMistakeCount++ cline.recordToolError("apply_diff") @@ -86,134 +115,152 @@ export async function applyDiffTool( pushToolResult(formattedError) return } + } - const originalContent = await fs.readFile(absolutePath, "utf-8") + // 4. Apply diff to file content (shared path) + const absolutePath = path.resolve(cline.cwd, relPath) + const originalContent = await fs.readFile(absolutePath, "utf-8") + + const diffResult = (await cline.diffStrategy?.applyDiff( + originalContent, + diffContent!, // Safe to assert here since we validated above + parseInt(block.params.start_line ?? ""), + )) ?? { + success: false, + error: "No diff strategy available", + } - // Apply the diff to the original content - const diffResult = (await cline.diffStrategy?.applyDiff( - originalContent, - diffContent, - parseInt(block.params.start_line ?? ""), - )) ?? { - success: false, - error: "No diff strategy available", - } + console.debug("diffResult", inspect(diffResult, { depth: 5 })) + let partErrors = "" - if (!diffResult.success) { - cline.consecutiveMistakeCount++ - const currentCount = (cline.consecutiveMistakeCountForApplyDiff.get(relPath) || 0) + 1 - cline.consecutiveMistakeCountForApplyDiff.set(relPath, currentCount) - let formattedError = "" - telemetryService.captureDiffApplicationError(cline.taskId, currentCount) - - if (diffResult.failParts && diffResult.failParts.length > 0) { - for (const failPart of diffResult.failParts) { - if (failPart.success) { - continue - } - - const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" - - formattedError = `\n${ - failPart.error - }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` + // 5. Handle diff application failures + // During streaming, silently ignore failures since content may be incomplete + if (block.partial && !diffResult.success) { + return + } else if (!diffResult.success) { + cline.consecutiveMistakeCount++ + const currentCount = (cline.consecutiveMistakeCountForApplyDiff.get(relPath) || 0) + 1 + cline.consecutiveMistakeCountForApplyDiff.set(relPath, currentCount) + let formattedError = "" + telemetryService.captureDiffApplicationError(cline.taskId, currentCount) + + if (diffResult.failParts && diffResult.failParts.length > 0) { + for (const failPart of diffResult.failParts) { + if (failPart.success) { + continue } - } else { - const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" - formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${ - diffResult.error + const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : "" + + formattedError = `\n${ + failPart.error }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` - } - if (currentCount >= 2) { - await cline.say("diff_error", formattedError) + partErrors += formattedError } + } else { + const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : "" - cline.recordToolError("apply_diff", formattedError) + formattedError = `Unable to apply diff to file: ${absolutePath}\n\n\n${ + diffResult.error + }${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n` + } - pushToolResult(formattedError) - return + if (currentCount >= 2) { + await cline.say("diff_error", formattedError) } - cline.consecutiveMistakeCount = 0 - cline.consecutiveMistakeCountForApplyDiff.delete(relPath) + cline.recordToolError("apply_diff", formattedError) + + pushToolResult(formattedError) + return + } + + // 6. Update UI with successful diff (common path) + cline.consecutiveMistakeCount = 0 + cline.consecutiveMistakeCountForApplyDiff.delete(relPath) - // Show diff view before asking for approval - cline.diffViewProvider.editType = "modify" + // Show diff view before asking for approval + cline.diffViewProvider.editType = "modify" + if (cline.diffViewProvider.isEditing) { + await cline.diffViewProvider.update(diffResult.content, true, false) + } else { await cline.diffViewProvider.open(relPath) await cline.diffViewProvider.update(diffResult.content, true) await cline.diffViewProvider.scrollToFirstDiff() + } - const completeMessage = JSON.stringify({ - ...sharedMessageProps, - diff: diffContent, - } satisfies ClineSayTool) + // 7. For streaming updates, show preview without asking for confirmation + if (block.partial) return + const completeMessage = JSON.stringify({ + ...sharedMessageProps, + diff: diffContent, + } satisfies ClineSayTool) - let toolProgressStatus + // 8. For completed output, get user approval and finalize changes + let toolProgressStatus - if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) { - toolProgressStatus = cline.diffStrategy.getProgressStatus(block, diffResult) - } + if (cline.diffStrategy?.getProgressStatus) { + toolProgressStatus = cline.diffStrategy.getProgressStatus(block, diffResult) + } - const didApprove = await askApproval("tool", completeMessage, toolProgressStatus) + const didApprove = await askApproval("tool", completeMessage, toolProgressStatus) - if (!didApprove) { - await cline.diffViewProvider.revertChanges() // Cline likely handles closing the diff view - return - } + if (!didApprove) { + await cline.diffViewProvider.revertChanges() + return + } - const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges() + // 9. Save approved changes and provide feedback + const { newProblemsMessage, userEdits, finalContent } = await cline.diffViewProvider.saveChanges() - // Track file edit operation - if (relPath) { - await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource) - } + // Track file edit operation + if (relPath) { + await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource) + } - // Used to determine if we should wait for busy terminal to update before sending api request - cline.didEditFile = true - let partFailHint = "" + cline.didEditFile = true + let partFailHint = "" - if (diffResult.failParts && diffResult.failParts.length > 0) { - partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use tool to check newest file version and re-apply diffs\n` - } + if (diffResult.failParts && diffResult.failParts.length > 0) { + partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use tool to check newest file version and re-apply diffs\n` + } - if (userEdits) { - await cline.say( - "user_feedback_diff", - JSON.stringify({ - tool: fileExists ? "editedExistingFile" : "newFileCreated", - path: getReadablePath(cline.cwd, relPath), - diff: userEdits, - } satisfies ClineSayTool), - ) - - pushToolResult( - `The user made the following updates to your content:\n\n${userEdits}\n\n` + - partFailHint + - `The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` + - `\n${addLineNumbers( - finalContent || "", - )}\n\n\n` + - `Please note:\n` + - `1. You do not need to re-write the file with these changes, as they have already been applied.\n` + - `2. Proceed with the task using this updated file content as the new baseline.\n` + - `3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` + - `${newProblemsMessage}`, - ) - } else { - pushToolResult( - `Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint, - ) - } + if (userEdits) { + const fileExists = await fileExistsAtPath(path.resolve(cline.cwd, relPath)) - await cline.diffViewProvider.reset() + await cline.say( + "user_feedback_diff", + JSON.stringify({ + tool: fileExists ? "editedExistingFile" : "newFileCreated", + path: getReadablePath(cline.cwd, relPath), + diff: userEdits, + } satisfies ClineSayTool), + ) - return + pushToolResult( + (partErrors ? `Partial errors encountered:\n${partErrors}\n\n` : "") + + `The user made the following updates to your content:\n\n${userEdits}\n\n` + + partFailHint + + `The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` + + `\n${addLineNumbers( + finalContent || "", + )}\n\n\n` + + `Please note:\n` + + `1. You do not need to re-write the file with these changes, as they have already been applied.\n` + + `2. Proceed with the task using this updated file content as the new baseline.\n` + + `3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` + + `${newProblemsMessage}`, + ) + } else { + pushToolResult( + `Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}\n` + partFailHint, + ) } + + await cline.diffViewProvider.reset() } catch (error) { await handleError("applying diff", error) await cline.diffViewProvider.reset() - return } } diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts index bcb217326a..89aa647455 100644 --- a/src/core/tools/insertContentTool.ts +++ b/src/core/tools/insertContentTool.ts @@ -1,7 +1,5 @@ -import delay from "delay" import fs from "fs/promises" import path from "path" - import { getReadablePath } from "../../utils/path" import { Task } from "../task/Task" import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools" @@ -10,6 +8,61 @@ import { ClineSayTool } from "../../shared/ExtensionMessage" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { fileExistsAtPath } from "../../utils/fs" import { insertGroups } from "../diff/insert-groups" +import { telemetryService } from "../../services/telemetry/TelemetryService" +import { addLineNumbers } from "../../integrations/misc/extract-text" + +const CONTENT_UPDATE_DELAY = 200 + +async function prepareAndShowContent( + cline: Task, + absolutePath: string, + relPath: string, + content: string, + lineNumber: number, + sharedMessageProps: ClineSayTool, + partial: boolean, +) { + const fileContent = !cline.diffViewProvider.isEditing + ? await fs.readFile(absolutePath, "utf8") + : cline.diffViewProvider.originalContent! + + const lines = fileContent.split("\n") + const updatedContent = insertGroups(lines, [ + { + index: lineNumber - 1, + elements: content.split("\n"), + }, + ]).join("\n") + + if (!cline.diffViewProvider.isEditing) { + cline.diffViewProvider.editType = "modify" + cline.diffViewProvider.originalContent = fileContent + await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {}) + await cline.diffViewProvider.open(relPath) + await cline.diffViewProvider.update(updatedContent, true, false) + cline.diffViewProvider.scrollToFirstDiff() + } else { + await cline.diffViewProvider.update(updatedContent, true, false) + } + + const diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent) + + // Send partial message with current state + if (partial) { + const partialMessage = JSON.stringify({ + ...sharedMessageProps, + diff, + content, // Include content for CodeAccordian to show during streaming + }) + await cline.ask("tool", partialMessage, partial).catch(() => {}) + } + + // without this the diff vscode interface does not update because it + // coalesces updates and if they come too fast then nothing changes. + await new Promise((resolve) => setTimeout(resolve, CONTENT_UPDATE_DELAY)) + + return { fileContent, updatedContent, diff } +} export async function insertContentTool( cline: Task, @@ -23,16 +76,57 @@ export async function insertContentTool( const line: string | undefined = block.params.line const content: string | undefined = block.params.content + // 1-based lineNumber + const lineNumber = line ? parseInt(line, 10) : undefined + const sharedMessageProps: ClineSayTool = { tool: "insertContent", path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)), diff: content, - lineNumber: line ? parseInt(line, 10) : undefined, + lineNumber, } try { if (block.partial) { - await cline.ask("tool", JSON.stringify(sharedMessageProps), block.partial).catch(() => {}) + // Validate all required parameters exist before proceeding + if (!relPath || lineNumber === undefined || !content) { + // Wait for all parameters before proceeding + return + } + + const absolutePath = path.resolve(cline.cwd, relPath) + const fileExists = await fileExistsAtPath(absolutePath) + + const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) + if (!accessAllowed) { + await cline.say("rooignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath))) + return + } + + if (!fileExists) { + cline.consecutiveMistakeCount++ + cline.recordToolError("insert_content") + const formattedError = `File does not exist at path: ${absolutePath}\n\n\nThe specified file could not be found. Please verify the file path and try again.\n` + const currentCount = (cline.consecutiveMistakeCountForInsertContent?.get(relPath) || 0) + 1 + cline.consecutiveMistakeCountForInsertContent?.set(relPath, currentCount) + telemetryService.captureInsertContentError(cline.taskId, currentCount) + if (currentCount >= 2) { + await cline.say("error", formattedError) + } + pushToolResult(formattedError) + return + } + + await prepareAndShowContent( + cline, + absolutePath, + relPath, + content, + lineNumber, + sharedMessageProps, + block.partial, + ) return } @@ -61,17 +155,27 @@ export async function insertContentTool( const absolutePath = path.resolve(cline.cwd, relPath) const fileExists = await fileExistsAtPath(absolutePath) + const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath) + if (!accessAllowed) { + await cline.say("rooignore_error", relPath) + pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath))) + return + } + if (!fileExists) { cline.consecutiveMistakeCount++ cline.recordToolError("insert_content") const formattedError = `File does not exist at path: ${absolutePath}\n\n\nThe specified file could not be found. Please verify the file path and try again.\n` + const currentCount = (cline.consecutiveMistakeCountForInsertContent?.get(relPath) || 0) + 1 + cline.consecutiveMistakeCountForInsertContent?.set(relPath, currentCount) + telemetryService.captureInsertContentError(cline.taskId, currentCount) await cline.say("error", formattedError) pushToolResult(formattedError) return } - const lineNumber = parseInt(line, 10) - if (isNaN(lineNumber) || lineNumber < 0) { + // 0 here is append, so it is allowed: + if (lineNumber === undefined || isNaN(lineNumber) || lineNumber < 0) { cline.consecutiveMistakeCount++ cline.recordToolError("insert_content") pushToolResult(formatResponse.toolError("Invalid line number. Must be a non-negative integer.")) @@ -80,51 +184,33 @@ export async function insertContentTool( cline.consecutiveMistakeCount = 0 - // Read the file - const fileContent = await fs.readFile(absolutePath, "utf8") - cline.diffViewProvider.editType = "modify" - cline.diffViewProvider.originalContent = fileContent - const lines = fileContent.split("\n") - - const updatedContent = insertGroups(lines, [ - { - index: lineNumber - 1, - elements: content.split("\n"), - }, - ]).join("\n") - - // Show changes in diff view - if (!cline.diffViewProvider.isEditing) { - await cline.ask("tool", JSON.stringify(sharedMessageProps), true).catch(() => {}) - // First open with original content - await cline.diffViewProvider.open(relPath) - await cline.diffViewProvider.update(fileContent, false) - cline.diffViewProvider.scrollToFirstDiff() - await delay(200) - } - - const diff = formatResponse.createPrettyPatch(relPath, fileContent, updatedContent) + const { diff } = await prepareAndShowContent( + cline, + absolutePath, + relPath, + content, + lineNumber, + sharedMessageProps, + block.partial, + ) if (!diff) { pushToolResult(`No changes needed for '${relPath}'`) return } - await cline.diffViewProvider.update(updatedContent, true) - + cline.consecutiveMistakeCount = 0 + cline.consecutiveMistakeCountForInsertContent?.delete(relPath) const completeMessage = JSON.stringify({ ...sharedMessageProps, diff, - lineNumber: lineNumber, + lineNumber: lineNumber + 1, } satisfies ClineSayTool) - const didApprove = await cline - .ask("tool", completeMessage, false) - .then((response) => response.response === "yesButtonClicked") + const didApprove = await askApproval("tool", completeMessage) if (!didApprove) { await cline.diffViewProvider.revertChanges() - pushToolResult("Changes were rejected by the user.") return } @@ -158,7 +244,7 @@ export async function insertContentTool( pushToolResult( `The user made the following updates to your content:\n\n${userEdits}\n\n` + `The updated content has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file:\n\n` + - `\n${finalContent}\n\n\n` + + `\n${addLineNumbers(finalContent || "")}\n\n\n` + `Please note:\n` + `1. You do not need to re-write the file with these changes, as they have already been applied.\n` + `2. Proceed with the task using this updated file content as the new baseline.\n` + diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index b6a5b9ac3b..df0772859e 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -95,7 +95,7 @@ export class DiffViewProvider { this.streamedLines = [] } - async update(accumulatedContent: string, isFinal: boolean) { + async update(accumulatedContent: string, isFinal: boolean, autoScroll: boolean = true) { if (!this.relPath || !this.activeLineController || !this.fadedOverlayController) { throw new Error("Required values not set") } @@ -129,8 +129,11 @@ export class DiffViewProvider { // Update decorations. this.activeLineController.setActiveLine(endLine) this.fadedOverlayController.updateOverlayAfterLine(endLine, document.lineCount) - // Scroll to the current line. - this.scrollEditorToLine(endLine) + + // Scroll to the current line if enabled + if (autoScroll) { + this.scrollEditorToLine(endLine) + } // Update the streamedLines with the new accumulated content. this.streamedLines = accumulatedLines diff --git a/src/services/telemetry/PostHogClient.ts b/src/services/telemetry/PostHogClient.ts index 784c9476e8..1233a30c78 100644 --- a/src/services/telemetry/PostHogClient.ts +++ b/src/services/telemetry/PostHogClient.ts @@ -34,6 +34,7 @@ export class PostHogClient { DIFF_APPLICATION_ERROR: "Diff Application Error", SHELL_INTEGRATION_ERROR: "Shell Integration Error", CONSECUTIVE_MISTAKE_ERROR: "Consecutive Mistake Error", + INSERT_CONTENT_ERROR: "Insert Content Error", }, } diff --git a/src/services/telemetry/TelemetryService.ts b/src/services/telemetry/TelemetryService.ts index 37423542b0..63543446e1 100644 --- a/src/services/telemetry/TelemetryService.ts +++ b/src/services/telemetry/TelemetryService.ts @@ -137,6 +137,10 @@ class TelemetryService { this.captureEvent(PostHogClient.EVENTS.ERRORS.DIFF_APPLICATION_ERROR, { taskId, consecutiveMistakeCount }) } + public captureInsertContentError(taskId: string, consecutiveMistakeCount: number): void { + this.captureEvent(PostHogClient.EVENTS.ERRORS.INSERT_CONTENT_ERROR, { taskId, consecutiveMistakeCount }) + } + public captureShellIntegrationError(taskId: string): void { this.captureEvent(PostHogClient.EVENTS.ERRORS.SHELL_INTEGRATION_ERROR, { taskId }) }