From 72c5c713549f225802111b4ba75345a1a6bb0478 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 01:12:05 -0500 Subject: [PATCH 1/6] Improvements to apply_diff --- CHANGELOG.md | 4 ++++ package-lock.json | 4 ++-- package.json | 2 +- src/core/Cline.ts | 29 +++++++++++++++++++++++++---- src/core/prompts/system.ts | 23 +++++++++++++++++++++-- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd0f73c69..fcb8a69440 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Roo Cline Changelog +## [2.1.14] + +- Fix bug where diffs were not being applied correctly and try Aider's unified diff prompt + ## [2.1.13] - Fix https://github.com/RooVetGit/Roo-Cline/issues/50 where sound effects were not respecting settings diff --git a/package-lock.json b/package-lock.json index e2d9044fa2..7309b5dac7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "roo-cline", - "version": "2.1.13", + "version": "2.1.14", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "roo-cline", - "version": "2.1.13", + "version": "2.1.14", "dependencies": { "@anthropic-ai/bedrock-sdk": "^0.10.2", "@anthropic-ai/sdk": "^0.26.0", diff --git a/package.json b/package.json index e62a6c30de..82a9ba5bc6 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "displayName": "Roo Cline", "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.", "publisher": "RooVeterinaryInc", - "version": "2.1.13", + "version": "2.1.14", "icon": "assets/icons/rocket.png", "galleryBanner": { "color": "#617A91", diff --git a/src/core/Cline.ts b/src/core/Cline.ts index bf45f7f9ed..cb8e68f1ee 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -1231,11 +1231,32 @@ export class Cline { break } - await fs.writeFile(absolutePath, newContent) - await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false }) + const { newProblemsMessage, userEdits, finalContent } = + await this.diffViewProvider.saveChanges() + this.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request + if (userEdits) { + await this.say( + "user_feedback_diff", + JSON.stringify({ + tool: fileExists ? "editedExistingFile" : "newFileCreated", + path: getReadablePath(cwd, relPath), + diff: userEdits, + } satisfies ClineSayTool), + ) + pushToolResult( + `The user made the following updates to your content:\n\n${userEdits}\n\n` + + `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:\n\n` + + `\n${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}`) + } await this.diffViewProvider.reset() - - pushToolResult(`Changes successfully applied to ${relPath.toPosix()}:\n\n${diffRepresentation}`) break } } catch (error) { diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 2d07d79193..ddd8f54a85 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -69,10 +69,29 @@ Your file content here ${diffEnabled ? ` ## apply_diff -Description: Apply a diff to a file at the specified path. The diff should be in unified format (diff -u) and can be used to apply changes to a file. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in a diff. +Description: Apply a diff to a file at the specified path. The diff should be in unified format ('diff -U0') and can be used to apply changes to a file. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in a diff. + +- Make sure you include the first 2 lines with the file paths. +- Don't include timestamps with the file paths. +- Start each hunk of changes with a '@@ ... @@' line. Don't include line numbers like 'diff -U0' does. The user's patch tool doesn't need them. +- The user's patch tool needs CORRECT patches that apply cleanly against the current contents of the file! +- Think carefully and make sure you include and mark all lines that need to be removed or changed as '-' lines. +- Make sure you mark all new or modified lines with '+'. +- Don't leave out any lines or the diff patch won't apply correctly. +- Indentation matters in the diffs! +- Start a new hunk for each section of the file that needs changes. +- Only output hunks that specify changes with '+' or '-' lines. +- Skip any hunks that are entirely unchanging ' ' lines. +- Output hunks in whatever order makes the most sense. +- Hunks don't need to be in any particular order. +- When editing a function, method, loop, etc use a hunk to replace the *entire* code block. +- Delete the entire existing version with '-' lines and then add a new, updated version with '+' lines. +- This will help you generate correct code and correct diffs. +- To move code within a file, use 2 hunks: 1 to delete it from its current location, 1 to insert it in the new location. + Parameters: - path: (required) The path of the file to apply the diff to (relative to the current working directory ${cwd.toPosix()}) -- diff: (required) The diff in unified format (diff -u) to apply to the file. +- diff: (required) The diff in unified format (diff -U0) to apply to the file. Usage: File path here From 9cc311d7f9faa146654daf9a10d23350ad47b3a0 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 01:24:38 -0500 Subject: [PATCH 2/6] Default is_sound_enabled to false just to be safe --- src/utils/sound.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/sound.ts b/src/utils/sound.ts index 42a1623b59..9255db4344 100644 --- a/src/utils/sound.ts +++ b/src/utils/sound.ts @@ -20,7 +20,7 @@ export const isWAV = (filepath: string): boolean => { return path.extname(filepath).toLowerCase() === ".wav" } -let isSoundEnabled = true +let isSoundEnabled = false /** * Set sound configuration From 27b4bb2bb000070211152bca10dc537931f9a589 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 01:29:41 -0500 Subject: [PATCH 3/6] Link to Aider --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcb8a69440..ec1f0035fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [2.1.14] -- Fix bug where diffs were not being applied correctly and try Aider's unified diff prompt +- Fix bug where diffs were not being applied correctly and try Aider's [unified diff prompt](https://github.com/Aider-AI/aider/blob/3995accd0ca71cea90ef76d516837f8c2731b9fe/aider/coders/udiff_prompts.py#L75-L105) ## [2.1.13] From 538fedbd521aa2097da0b2c9ee200106c14a7823 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 02:18:47 -0500 Subject: [PATCH 4/6] Add line numbers when reading files and writing files --- src/core/Cline.ts | 11 +++++----- src/core/prompts/system.ts | 3 ++- .../webview/__tests__/ClineProvider.test.ts | 15 ++++++++++++++ src/integrations/misc/extract-text.ts | 20 +++++++++++++++---- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index cb8e68f1ee..038109f37f 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -12,7 +12,7 @@ import { ApiHandler, buildApiHandler } from "../api" import { ApiStream } from "../api/transform/stream" import { DiffViewProvider } from "../integrations/editor/DiffViewProvider" import { findToolName, formatContentBlockToMarkdown } from "../integrations/misc/export-markdown" -import { extractTextFromFile } from "../integrations/misc/extract-text" +import { extractTextFromFile, addLineNumbers } from "../integrations/misc/extract-text" import { TerminalManager } from "../integrations/terminal/TerminalManager" import { UrlContentFetcher } from "../services/browser/UrlContentFetcher" import { listFiles } from "../services/glob/list-files" @@ -1133,8 +1133,8 @@ export class Cline { ) pushToolResult( `The user made the following updates to your content:\n\n${userEdits}\n\n` + - `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:\n\n` + - `\n${finalContent}\n\n\n` + + `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` + @@ -1245,8 +1245,8 @@ export class Cline { ) pushToolResult( `The user made the following updates to your content:\n\n${userEdits}\n\n` + - `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:\n\n` + - `\n${finalContent}\n\n\n` + + `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` + @@ -2263,3 +2263,4 @@ export class Cline { return `\n${details.trim()}\n` } } + diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index ddd8f54a85..395973d7b4 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -46,7 +46,7 @@ Usage: ## read_file -Description: Request to read the contents of a file at the specified path. Use this when you need to examine the contents of an existing file you do not know the contents of, for example to analyze code, review text files, or extract information from configuration files. Automatically extracts raw text from PDF and DOCX files. May not be suitable for other types of binary files, as it returns the raw content as a string. +Description: Request to read the contents of a file at the specified path. Use this when you need to examine the contents of an existing file you do not know the contents of, for example to analyze code, review text files, or extract information from configuration files. The output includes line numbers prefixed to each line (e.g. "1 | const x = 1"), making it easier to reference specific lines when creating diffs or discussing code. Automatically extracts raw text from PDF and DOCX files. May not be suitable for other types of binary files, as it returns the raw content as a string. Parameters: - path: (required) The path of the file to read (relative to the current working directory ${cwd.toPosix()}) Usage: @@ -361,3 +361,4 @@ The following additional instructions are provided by the user, and should be fo ${joinedInstructions}` : "" } + diff --git a/src/core/webview/__tests__/ClineProvider.test.ts b/src/core/webview/__tests__/ClineProvider.test.ts index d2e8773cae..b88e2f0834 100644 --- a/src/core/webview/__tests__/ClineProvider.test.ts +++ b/src/core/webview/__tests__/ClineProvider.test.ts @@ -76,6 +76,15 @@ jest.mock('../../Cline', () => { } }) +// Mock extract-text +jest.mock('../../../integrations/misc/extract-text', () => ({ + extractTextFromFile: jest.fn().mockImplementation(async (filePath: string) => { + const content = 'const x = 1;\nconst y = 2;\nconst z = 3;' + const lines = content.split('\n') + return lines.map((line, index) => `${index + 1} | ${line}`).join('\n') + }) +})) + // Spy on console.error and console.log to suppress expected messages beforeAll(() => { jest.spyOn(console, 'error').mockImplementation(() => {}) @@ -258,4 +267,10 @@ describe('ClineProvider', () => { expect(mockContext.globalState.update).toHaveBeenCalledWith('soundEnabled', false) expect(mockPostMessage).toHaveBeenCalled() }) + + test('file content includes line numbers', async () => { + const { extractTextFromFile } = require('../../../integrations/misc/extract-text') + const result = await extractTextFromFile('test.js') + expect(result).toBe('1 | const x = 1;\n2 | const y = 2;\n3 | const z = 3;') + }) }) diff --git a/src/integrations/misc/extract-text.ts b/src/integrations/misc/extract-text.ts index 67a580af9b..3f9ff1c68e 100644 --- a/src/integrations/misc/extract-text.ts +++ b/src/integrations/misc/extract-text.ts @@ -22,7 +22,7 @@ export async function extractTextFromFile(filePath: string): Promise { default: const isBinary = await isBinaryFile(filePath).catch(() => false) if (!isBinary) { - return await fs.readFile(filePath, "utf8") + return addLineNumbers(await fs.readFile(filePath, "utf8")) } else { throw new Error(`Cannot read text for file type: ${fileExtension}`) } @@ -32,12 +32,12 @@ export async function extractTextFromFile(filePath: string): Promise { async function extractTextFromPDF(filePath: string): Promise { const dataBuffer = await fs.readFile(filePath) const data = await pdf(dataBuffer) - return data.text + return addLineNumbers(data.text) } async function extractTextFromDOCX(filePath: string): Promise { const result = await mammoth.extractRawText({ path: filePath }) - return result.value + return addLineNumbers(result.value) } async function extractTextFromIPYNB(filePath: string): Promise { @@ -51,5 +51,17 @@ async function extractTextFromIPYNB(filePath: string): Promise { } } - return extractedText + return addLineNumbers(extractedText) } + +export function addLineNumbers(content: string): string { + const lines = content.split('\n') + const maxLineNumberWidth = String(lines.length).length + return lines + .map((line, index) => { + const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ') + return `${lineNumber} | ${line}` + }).join('\n') +} + + From 955660c71d6e00290cc5315ec00bb811d314246a Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 03:08:33 -0500 Subject: [PATCH 5/6] Prompt updates --- src/core/prompts/system.ts | 71 +++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index 395973d7b4..b1672eba0f 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -71,23 +71,60 @@ ${diffEnabled ? ` ## apply_diff Description: Apply a diff to a file at the specified path. The diff should be in unified format ('diff -U0') and can be used to apply changes to a file. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in a diff. -- Make sure you include the first 2 lines with the file paths. -- Don't include timestamps with the file paths. -- Start each hunk of changes with a '@@ ... @@' line. Don't include line numbers like 'diff -U0' does. The user's patch tool doesn't need them. -- The user's patch tool needs CORRECT patches that apply cleanly against the current contents of the file! -- Think carefully and make sure you include and mark all lines that need to be removed or changed as '-' lines. -- Make sure you mark all new or modified lines with '+'. -- Don't leave out any lines or the diff patch won't apply correctly. -- Indentation matters in the diffs! -- Start a new hunk for each section of the file that needs changes. -- Only output hunks that specify changes with '+' or '-' lines. -- Skip any hunks that are entirely unchanging ' ' lines. -- Output hunks in whatever order makes the most sense. -- Hunks don't need to be in any particular order. -- When editing a function, method, loop, etc use a hunk to replace the *entire* code block. -- Delete the entire existing version with '-' lines and then add a new, updated version with '+' lines. -- This will help you generate correct code and correct diffs. -- To move code within a file, use 2 hunks: 1 to delete it from its current location, 1 to insert it in the new location. +Diff Format Requirements: + +1. Header (REQUIRED): + \`\`\` + --- path/to/original/file + +++ path/to/modified/file + \`\`\` + - Must include both lines exactly as shown + - Use actual file paths + - NO timestamps after paths + +2. Hunks: + \`\`\` + @@ -lineStart,lineCount +lineStart,lineCount @@ + -removed line + +added line + \`\`\` + - Each hunk starts with @@ showing line numbers for changes + - Format: @@ -originalStart,originalCount +newStart,newCount @@ + - Use - for removed/changed lines + - Use + for new/modified lines + - Indentation must match exactly + +Complete Example: +\`\`\` +--- src/utils/helper.ts ++++ src/utils/helper.ts +@@ -10,3 +10,4 @@ +-function oldFunction(x: number): number { +- return x + 1; +-} ++function newFunction(x: number): number { ++ const result = x + 2; ++ return result; ++} +\`\`\` + +Common Pitfalls: +1. Missing or incorrect header lines +2. Incorrect line numbers in @@ lines +3. Wrong indentation in changed lines +4. Incomplete context (missing lines that need changing) +5. Not marking all modified lines with - and + + +Best Practices: +1. Replace entire code blocks: + - Remove complete old version with - lines + - Add complete new version with + lines + - Include correct line numbers +2. Moving code requires two hunks: + - First hunk: Remove from old location + - Second hunk: Add to new location +3. One hunk per logical change +4. Verify line numbers match the file Parameters: - path: (required) The path of the file to apply the diff to (relative to the current working directory ${cwd.toPosix()}) From 484679670f0f493b95890b1d6fe1ed1ed72c02d6 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Sun, 8 Dec 2024 02:07:40 -0500 Subject: [PATCH 6/6] Automatically reject truncated writes if diffs are enabled --- CHANGELOG.md | 3 +- src/core/Cline.ts | 29 +++++++++++++++++-- src/integrations/editor/detect-omission.ts | 27 +---------------- .../src/components/settings/SettingsView.tsx | 2 +- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec1f0035fe..e13f8a806f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [2.1.14] - Fix bug where diffs were not being applied correctly and try Aider's [unified diff prompt](https://github.com/Aider-AI/aider/blob/3995accd0ca71cea90ef76d516837f8c2731b9fe/aider/coders/udiff_prompts.py#L75-L105) +- If diffs are enabled, automatically reject write_to_file commands that lead to truncated output ## [2.1.13] @@ -51,4 +52,4 @@ ## [2.1.2] - Support for auto-approval of write operations and command execution -- Support for .clinerules custom instructions \ No newline at end of file +- Support for .clinerules custom instructions diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 038109f37f..3c32a35e61 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -46,7 +46,7 @@ import { formatResponse } from "./prompts/responses" import { addCustomInstructions, SYSTEM_PROMPT } from "./prompts/system" import { truncateHalfConversation } from "./sliding-window" import { ClineProvider, GlobalFileNames } from "./webview/ClineProvider" -import { showOmissionWarning } from "../integrations/editor/detect-omission" +import { detectCodeOmission } from "../integrations/editor/detect-omission" import { BrowserSession } from "../services/browser/BrowserSession" const cwd = @@ -1101,7 +1101,32 @@ export class Cline { await this.diffViewProvider.update(newContent, true) await delay(300) // wait for diff view to update this.diffViewProvider.scrollToFirstDiff() - showOmissionWarning(this.diffViewProvider.originalContent || "", newContent) + + // Check for code omissions before proceeding + if (detectCodeOmission(this.diffViewProvider.originalContent || "", newContent)) { + if (this.diffEnabled) { + await this.diffViewProvider.revertChanges() + pushToolResult(formatResponse.toolError( + "Content appears to be truncated. Found comments indicating omitted code (e.g., '// rest of code unchanged', '/* previous code */'). Please provide the complete file content without any omissions if possible, or otherwise use the 'apply_diff' tool to apply the diff to the original file." + )) + break + } else { + vscode.window + .showWarningMessage( + "Potential code truncation detected. This happens when the AI reaches its max output limit.", + "Follow this guide to fix the issue", + ) + .then((selection) => { + if (selection === "Follow this guide to fix the issue") { + vscode.env.openExternal( + vscode.Uri.parse( + "https://github.com/cline/cline/wiki/Troubleshooting-%E2%80%90-Cline-Deleting-Code-with-%22Rest-of-Code-Here%22-Comments", + ), + ) + } + }) + } + } const completeMessage = JSON.stringify({ ...sharedMessageProps, diff --git a/src/integrations/editor/detect-omission.ts b/src/integrations/editor/detect-omission.ts index 32de0aac72..565ebd3ace 100644 --- a/src/integrations/editor/detect-omission.ts +++ b/src/integrations/editor/detect-omission.ts @@ -1,12 +1,10 @@ -import * as vscode from "vscode" - /** * Detects potential AI-generated code omissions in the given file content. * @param originalFileContent The original content of the file. * @param newFileContent The new content of the file to check. * @returns True if a potential omission is detected, false otherwise. */ -function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean { +export function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean { const originalLines = originalFileContent.split("\n") const newLines = newFileContent.split("\n") const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "..."] @@ -33,26 +31,3 @@ function detectCodeOmission(originalFileContent: string, newFileContent: string) return false } -/** - * Shows a warning in VSCode if a potential code omission is detected. - * @param originalFileContent The original content of the file. - * @param newFileContent The new content of the file to check. - */ -export function showOmissionWarning(originalFileContent: string, newFileContent: string): void { - if (detectCodeOmission(originalFileContent, newFileContent)) { - vscode.window - .showWarningMessage( - "Potential code truncation detected. This happens when the AI reaches its max output limit.", - "Follow this guide to fix the issue", - ) - .then((selection) => { - if (selection === "Follow this guide to fix the issue") { - vscode.env.openExternal( - vscode.Uri.parse( - "https://github.com/cline/cline/wiki/Troubleshooting-%E2%80%90-Cline-Deleting-Code-with-%22Rest-of-Code-Here%22-Comments", - ), - ) - } - }) - } -} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 563eb0179c..2c95176b2f 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -301,7 +301,7 @@ const SettingsView = ({ onDone }: SettingsViewProps) => { marginTop: "5px", color: "var(--vscode-descriptionForeground)", }}> - When enabled, Cline will be able to apply diffs to make changes to files. + When enabled, Cline will be able to apply diffs to make changes to files and will automatically reject truncated full-file edits.