From f7704876950fb2ec8329ce667137d9353a13d78a Mon Sep 17 00:00:00 2001 From: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com> Date: Sat, 28 Dec 2024 13:21:12 -0800 Subject: [PATCH] Improve diff editing prompts to avoid common deepseek issues --- src/core/Cline.ts | 31 +++++++++++++++--------------- src/core/assistant-message/diff.ts | 22 ++++++++++----------- src/core/prompts/system.ts | 18 +++++++++++------ src/utils/string.ts | 22 +++++++++++++++++++++ 4 files changed, 60 insertions(+), 33 deletions(-) create mode 100644 src/utils/string.ts diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 3b94b02efb..c1a8356860 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -50,6 +50,8 @@ import { addUserInstructions, SYSTEM_PROMPT } from "./prompts/system" import { truncateHalfConversation } from "./sliding-window" import { ClineProvider, GlobalFileNames } from "./webview/ClineProvider" import { showSystemNotification } from "../integrations/notifications" +import { removeInvalidChars } from "../utils/string" +import { fixModelHtmlEscaping } from "../utils/string" const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0) ?? path.join(os.homedir(), "Desktop") // may or may not exist but fs checking existence would immediately ask for permission which would be bad UX, need to come up with a better solution @@ -1131,6 +1133,11 @@ export class Cline { // Construct newContent from diff let newContent: string if (diff) { + if (!this.api.getModel().id.includes("claude")) { + // deepseek models tend to use unescaped html entities in diffs + diff = fixModelHtmlEscaping(diff) + diff = removeInvalidChars(diff) + } try { newContent = await constructNewFileContent( diff, @@ -1163,25 +1170,17 @@ export class Cline { if (newContent.endsWith("```")) { newContent = newContent.split("\n").slice(0, -1).join("\n").trim() } + + if (!this.api.getModel().id.includes("claude")) { + // it seems not just llama models are doing this, but also gemini and potentially others + newContent = fixModelHtmlEscaping(newContent) + newContent = removeInvalidChars(newContent) + } } else { // can't happen, since we already checked for content/diff above. but need to do this for type error break } - if (!this.api.getModel().id.includes("claude")) { - // it seems not just llama models are doing this, but also gemini and potentially others - if ( - newContent.includes(">") || - newContent.includes("<") || - newContent.includes(""") - ) { - newContent = newContent - .replace(/>/g, ">") - .replace(/</g, "<") - .replace(/"/g, '"') - } - } - newContent = newContent.trimEnd() // remove any trailing newlines, since it's automatically inserted by the editor const sharedMessageProps: ClineSayTool = { @@ -1294,7 +1293,7 @@ export class Cline { `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.` + - `4. If you need to make further changes to this file, use this final_file_content as the new reference for your SEARCH/REPLACE operations, as it is now the current state of the file (including the user's edits and any auto-formatting done by the system).\n` + + `4. IMPORTANT: If you need to make further changes to this file, use this final_file_content as the new reference for your SEARCH/REPLACE operations, as it is now the current state of the file (including the user's edits and any auto-formatting done by the user's editor).\n` + `${newProblemsMessage}`, ) } else { @@ -1302,7 +1301,7 @@ export class Cline { `The content was successfully saved to ${relPath.toPosix()}.\n\n` + `Here is the full, updated content of the file:\n\n` + `\n${finalContent}\n\n\n` + - `Please note: If you need to make further changes to this file, use this final_file_content as the new reference for your SEARCH/REPLACE operations, as it is now the current state of the file (including any auto-formatting done by the system).\n\n` + + `IMPORTANT: If you need to make further changes to this file, use this final_file_content as the new reference for your SEARCH/REPLACE operations, as it is now the current state of the file (including any auto-formatting done by the user's editor).\n\n` + `${newProblemsMessage}`, ) } diff --git a/src/core/assistant-message/diff.ts b/src/core/assistant-message/diff.ts index 238a351e52..ea19518a95 100644 --- a/src/core/assistant-message/diff.ts +++ b/src/core/assistant-message/diff.ts @@ -252,11 +252,11 @@ export async function constructNewFileContent( inReplace = true // Remove trailing linebreak for adding the === marker - if (currentSearchContent.endsWith("\r\n")) { - currentSearchContent = currentSearchContent.slice(0, -2) - } else if (currentSearchContent.endsWith("\n")) { - currentSearchContent = currentSearchContent.slice(0, -1) - } + // if (currentSearchContent.endsWith("\r\n")) { + // currentSearchContent = currentSearchContent.slice(0, -2) + // } else if (currentSearchContent.endsWith("\n")) { + // currentSearchContent = currentSearchContent.slice(0, -1) + // } if (!currentSearchContent) { // Empty search block @@ -319,12 +319,12 @@ export async function constructNewFileContent( if (line === ">>>>>>> REPLACE") { // Finished one replace block - // Remove the artificially added linebreak in the last line of the REPLACE block - if (result.endsWith("\r\n")) { - result = result.slice(0, -2) - } else if (result.endsWith("\n")) { - result = result.slice(0, -1) - } + // // Remove the artificially added linebreak in the last line of the REPLACE block + // if (result.endsWith("\r\n")) { + // result = result.slice(0, -2) + // } else if (result.endsWith("\n")) { + // result = result.slice(0, -1) + // } // Advance lastProcessedIndex to after the matched section lastProcessedIndex = searchEndIndex diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index f66c67bbc7..8bf95cda71 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -793,12 +793,17 @@ You have access to two tools for working with files: **write_to_file** and **rep # Auto-formatting Considerations - After using either write_to_file or replace_in_file, the user's editor may automatically format the file -- This auto-formatting may modify the file structure, for example: - - Breaking single lines into multiple lines - - Adjusting indentation - - Standardizing spacing and line endings -- The tool response will include the final state of the file after any auto-formatting -- Use this final state as your reference point for any subsequent edits. This is particularly important when crafting SEARCH blocks for replace_in_file which require the content to match what's in the file exactly. +- This auto-formatting may modify the file contents, for example: + - Breaking single lines into multiple lines (e.g. long function declarations, object literals, array definitions) + - Adjusting indentation to match project style (e.g. 2 spaces vs 4 spaces vs tabs) + - Standardizing spacing and line endings (e.g. removing extra whitespace, ensuring consistent newlines) + - Converting single quotes to double quotes (or vice versa based on project preferences) + - Organizing imports (e.g. sorting, grouping by type) + - Adding/removing trailing commas in objects and arrays + - Enforcing consistent brace style (e.g. same-line vs new-line) + - Standardizing semicolon usage (adding or removing based on style) +- The write_to_file and replace_in_file tool responses will include the final state of the file after any auto-formatting +- Use this final state as your reference point for any subsequent edits. This is ESPECIALLY important when crafting SEARCH blocks for replace_in_file which require the content to match what's in the file exactly. # Workflow Tips @@ -855,6 +860,7 @@ RULES - At the end of each user message, you will automatically receive environment_details. This information is not written by the user themselves, but is auto-generated to provide potentially relevant context about the project structure and environment. While this information can be valuable for understanding the project context, do not treat it as a direct part of the user's request or response. Use it to inform your actions and decisions, but don't assume the user is explicitly asking about or referring to this information unless they clearly do so in their message. When using environment_details, explain your actions clearly to ensure the user understands, as they may not be aware of these details. - Before executing commands, check the "Actively Running Terminals" section in environment_details. If present, consider how these active processes might impact your task. For example, if a local development server is already running, you wouldn't need to start it again. If no active terminals are listed, proceed with command execution as normal. - MCP operations should be used one at a time, similar to other tool usage. Wait for confirmation of success before proceeding with additional operations. +- When using the replace_in_file tool, you must include complete lines in your SEARCH blocks, not partial lines. The system requires exact line matches and cannot match partial lines. For example, if you want to match a line containing "const x = 5;", your SEARCH block must include the entire line, not just "x = 5" or other fragments. - It is critical you wait for the user's response after each tool use, in order to confirm the success of the tool use. For example, if asked to make a todo app, you would create a file, wait for the user's response it was created successfully, then create another file if needed, wait for the user's response it was created successfully, etc.${ supportsComputerUse ? " Then if you want to test your work, you might use browser_action to launch the site, wait for the user's response confirming the site was launched along with a screenshot, then perhaps e.g., click a button to test functionality if needed, wait for the user's response confirming the button was clicked along with a screenshot of the new state, before finally closing the browser." diff --git a/src/utils/string.ts b/src/utils/string.ts new file mode 100644 index 0000000000..83e364d55e --- /dev/null +++ b/src/utils/string.ts @@ -0,0 +1,22 @@ +/** + * Fixes incorrectly escaped HTML entities in AI model outputs + * @param text String potentially containing incorrectly escaped HTML entities from AI models + * @returns String with HTML entities converted back to normal characters + */ +export function fixModelHtmlEscaping(text: string): string { + return text + .replace(/>/g, ">") + .replace(/</g, "<") + .replace(/"/g, '"') + .replace(/&/g, "&") + .replace(/'/g, "'") +} + +/** + * Removes invalid characters (like the replacement character �) from a string + * @param text String potentially containing invalid characters + * @returns String with invalid characters removed + */ +export function removeInvalidChars(text: string): string { + return text.replace(/\uFFFD/g, "") +}