-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: improve diff editing error messages for Qwen3 model compatibility #7248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,6 +99,12 @@ If you're not confident in the exact content to search for, use the read_file to | |
| When applying the diffs, be extra careful to remember to change any closing brackets or other syntax that may be affected by the diff farther down in the file. | ||
| ALWAYS make as many changes in a single 'apply_diff' request as possible using multiple SEARCH/REPLACE blocks | ||
|
|
||
| **IMPORTANT ESCAPING RULES:** | ||
| - The diff structure markers (<<<<<<< SEARCH, =======, >>>>>>> REPLACE) should NEVER be escaped - they define the diff structure | ||
| - ONLY escape these patterns when they appear in the actual file content you're searching for or replacing | ||
| - Example: If you're removing merge conflict markers from a file, escape them in the SEARCH content: \\======= | ||
| - Do NOT escape the ======= that separates your SEARCH and REPLACE sections | ||
|
|
||
| Parameters: | ||
| - path: (required) The path of the file to modify (relative to the current workspace directory ${args.cwd}) | ||
| - diff: (required) The search/replace block defining the changes. | ||
|
|
@@ -213,37 +219,42 @@ Only use a single line of '=======' between search and replacement content, beca | |
| `ERROR: Special marker '${found}' found in your diff content at line ${state.line}:\n` + | ||
| "\n" + | ||
| `When removing merge conflict markers like '${found}' from files, you MUST escape them\n` + | ||
| "in your SEARCH section by prepending a backslash (\\) at the beginning of the line:\n" + | ||
| "in your SEARCH section by prepending a backslash (\\) at the beginning of the line.\n" + | ||
| "\n" + | ||
| "IMPORTANT CLARIFICATION:\n" + | ||
| "- ONLY escape these patterns when they appear in the actual file content you're modifying\n" + | ||
| "- The diff structure markers themselves (<<<<<<< SEARCH, =======, >>>>>>> REPLACE) should NEVER be escaped\n" + | ||
| "\n" + | ||
| "CORRECT FORMAT:\n\n" + | ||
| "<<<<<<< SEARCH\n" + | ||
| "content before\n" + | ||
| `\\${found} <-- Note the backslash here in this example\n` + | ||
| `\\${found} <-- Escape ONLY when this is part of the file content\n` + | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor inconsistency: This line has 4 spaces after the escape example while the same line in multi-file-search-replace.ts has only 1 space. Consider making the formatting consistent across both files. |
||
| "content after\n" + | ||
| "=======\n" + | ||
| "replacement content\n" + | ||
| ">>>>>>> REPLACE\n" + | ||
| "\n" + | ||
| "Without escaping, the system confuses your content with diff syntax markers.\n" + | ||
| "You may use multiple diff blocks in a single diff request, but ANY of ONLY the following separators that occur within SEARCH or REPLACE content must be escaped, as follows:\n" + | ||
| `\\${SEARCH}\n` + | ||
| `\\${SEP}\n` + | ||
| `\\${REPLACE}\n`, | ||
| "Without escaping, the system confuses your content with diff syntax markers.", | ||
| }) | ||
|
|
||
| const reportInvalidDiffError = (found: string, expected: string) => ({ | ||
| success: false, | ||
| error: | ||
| `ERROR: Diff block is malformed: marker '${found}' found in your diff content at line ${state.line}. Expected: ${expected}\n` + | ||
| "\n" + | ||
| "CORRECT FORMAT:\n\n" + | ||
| "CORRECT DIFF STRUCTURE:\n" + | ||
| "<<<<<<< SEARCH\n" + | ||
| ":start_line: (required) The line number of original content where the search block starts.\n" + | ||
| "-------\n" + | ||
| "[exact content to find including whitespace]\n" + | ||
| "=======\n" + | ||
| ":start_line:NUMBER (optional - specify line number)\n" + | ||
| "------- (optional separator)\n" + | ||
| "[exact content to find]\n" + | ||
| "======= (required - separates search from replace)\n" + | ||
| "[new content to replace with]\n" + | ||
| ">>>>>>> REPLACE\n", | ||
| ">>>>>>> REPLACE (required - ends the diff block)\n" + | ||
| "\n" + | ||
| "The markers above (<<<<<<< SEARCH, =======, >>>>>>> REPLACE) are part of the diff syntax.\n" + | ||
| "They should appear exactly as shown, without any escaping.\n" + | ||
| "\n" + | ||
| "Make sure you're following this exact structure for your diff blocks.", | ||
| }) | ||
|
|
||
| const reportLineMarkerInReplaceError = (marker: string) => ({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,7 +120,7 @@ export class ClineProvider | |
|
|
||
| public isViewLaunched = false | ||
| public settingsImportedAt?: number | ||
| public readonly latestAnnouncementId = "jul-29-2025-3-25-0" // Update for v3.25.0 announcement | ||
| public readonly latestAnnouncementId = "aug-20-2025-stealth-model" // Update for stealth model announcement | ||
| public readonly providerSettingsManager: ProviderSettingsManager | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional? This file contains changes for a 'stealth model announcement' feature that seems completely unrelated to the diff editing error message improvements described in the PR. Should this be in a separate PR to keep changes focused? |
||
| public readonly customModesManager: CustomModesManager | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,11 @@ import { Trans } from "react-i18next" | |
| import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" | ||
|
|
||
| import { Package } from "@roo/package" | ||
|
|
||
| import { useAppTranslation } from "@src/i18n/TranslationContext" | ||
| import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from "@src/components/ui" | ||
| import { useExtensionState } from "@src/context/ExtensionStateContext" | ||
| import { vscode } from "@src/utils/vscode" | ||
| import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@src/components/ui" | ||
| import { Button } from "@src/components/ui" | ||
|
|
||
| interface AnnouncementProps { | ||
| hideAnnouncement: () => void | ||
|
|
@@ -23,6 +25,7 @@ interface AnnouncementProps { | |
| const Announcement = ({ hideAnnouncement }: AnnouncementProps) => { | ||
| const { t } = useAppTranslation() | ||
| const [open, setOpen] = useState(true) | ||
| const { cloudIsAuthenticated } = useExtensionState() | ||
|
|
||
| return ( | ||
| <Dialog | ||
|
|
@@ -37,98 +40,64 @@ const Announcement = ({ hideAnnouncement }: AnnouncementProps) => { | |
| <DialogContent className="max-w-96"> | ||
| <DialogHeader> | ||
| <DialogTitle>{t("chat:announcement.title", { version: Package.version })}</DialogTitle> | ||
| <DialogDescription> | ||
| {t("chat:announcement.description", { version: Package.version })} | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <div> | ||
| <h3>{t("chat:announcement.whatsNew")}</h3> | ||
| <ul className="space-y-2"> | ||
| <li> | ||
| •{" "} | ||
| <Trans | ||
| i18nKey="chat:announcement.feature1" | ||
| components={{ | ||
| bold: <b />, | ||
| code: <code />, | ||
| settingsLink: ( | ||
| <VSCodeLink | ||
| href="#" | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| setOpen(false) | ||
| hideAnnouncement() | ||
| window.postMessage( | ||
| { | ||
| type: "action", | ||
| action: "settingsButtonClicked", | ||
| values: { section: "codebaseIndexing" }, | ||
| }, | ||
| "*", | ||
| ) | ||
| }} | ||
| /> | ||
| ), | ||
| }} | ||
| /> | ||
| </li> | ||
| <li> | ||
| •{" "} | ||
| <Trans | ||
| i18nKey="chat:announcement.feature2" | ||
| components={{ | ||
| bold: <b />, | ||
| code: <code />, | ||
| }} | ||
| /> | ||
| </li> | ||
| <li> | ||
| •{" "} | ||
| <Trans | ||
| i18nKey="chat:announcement.feature3" | ||
| i18nKey="chat:announcement.stealthModel.feature" | ||
| components={{ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These announcement UI changes appear unrelated to fixing Qwen3 diff editing issues. Consider splitting these changes into a separate PR for better change tracking and review clarity. |
||
| bold: <b />, | ||
| code: <code />, | ||
| }} | ||
| /> | ||
| </li> | ||
| </ul> | ||
| <Trans | ||
| i18nKey="chat:announcement.detailsDiscussLinks" | ||
| components={{ discordLink: <DiscordLink />, redditLink: <RedditLink /> }} | ||
| /> | ||
|
|
||
| <p className="text-xs text-muted-foreground mt-2">{t("chat:announcement.stealthModel.note")}</p> | ||
|
|
||
| <div className="mt-4"> | ||
| {!cloudIsAuthenticated ? ( | ||
| <Button | ||
| onClick={() => { | ||
| vscode.postMessage({ type: "rooCloudSignIn" }) | ||
| }} | ||
| className="w-full"> | ||
| {t("chat:announcement.stealthModel.connectButton")} | ||
| </Button> | ||
| ) : ( | ||
| <div className="text-sm w-full"> | ||
| <Trans | ||
| i18nKey="chat:announcement.stealthModel.selectModel" | ||
| components={{ | ||
| code: <code className="px-1 py-0.5 bg-gray-100 dark:bg-gray-800 rounded" />, | ||
| settingsLink: ( | ||
| <VSCodeLink | ||
| href="#" | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| setOpen(false) | ||
| hideAnnouncement() | ||
| window.postMessage( | ||
| { | ||
| type: "action", | ||
| action: "settingsButtonClicked", | ||
| values: { section: "provider" }, | ||
| }, | ||
| "*", | ||
| ) | ||
| }} | ||
| /> | ||
| ), | ||
| }} | ||
| /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ) | ||
| } | ||
|
|
||
| const DiscordLink = () => ( | ||
| <VSCodeLink | ||
| href="https://discord.gg/roocode" | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| window.postMessage( | ||
| { type: "action", action: "openExternal", data: { url: "https://discord.gg/roocode" } }, | ||
| "*", | ||
| ) | ||
| }}> | ||
| Discord | ||
| </VSCodeLink> | ||
| ) | ||
|
|
||
| const RedditLink = () => ( | ||
| <VSCodeLink | ||
| href="https://reddit.com/r/RooCode" | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| window.postMessage( | ||
| { type: "action", action: "openExternal", data: { url: "https://reddit.com/r/RooCode" } }, | ||
| "*", | ||
| ) | ||
| }}> | ||
| </VSCodeLink> | ||
| ) | ||
|
|
||
| export default memo(Announcement) | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new escaping rules section is helpful! Could we add one more example showing what happens when these patterns appear in actual file content vs. diff markers? This would make it even clearer for AI models.