diff --git a/src/core/diff/strategies/__tests__/grok-malformed-diff.spec.ts b/src/core/diff/strategies/__tests__/grok-malformed-diff.spec.ts new file mode 100644 index 0000000000..774fdf8266 --- /dev/null +++ b/src/core/diff/strategies/__tests__/grok-malformed-diff.spec.ts @@ -0,0 +1,327 @@ +import { MultiSearchReplaceDiffStrategy } from "../multi-search-replace" +import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace" + +describe("Grok Malformed Diff Detection", () => { + describe("MultiSearchReplaceDiffStrategy", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy() + }) + + describe("detectGrokMalformedDiff", () => { + it("detects consecutive separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("detects separator before search marker", () => { + const diff = "=======\n<<<<<<< SEARCH\ncontent\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("detects too many separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("returns false for valid diff", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(false) + }) + + it("returns false for multiple valid diffs", () => { + const diff = + "<<<<<<< SEARCH\ncontent1\n=======\nreplace1\n>>>>>>> REPLACE\n\n" + + "<<<<<<< SEARCH\ncontent2\n=======\nreplace2\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(false) + }) + }) + + describe("analyzeDiffStructure", () => { + it("provides debugging info for malformed diff", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["analyzeDiffStructure"](diff) + + expect(result).toContain("Found 1 SEARCH markers") + expect(result).toContain("Found 2 separator (=======) markers") + expect(result).toContain("Found 1 REPLACE markers") + expect(result).toContain("WARNING: More separators than SEARCH blocks") + expect(result).toContain("ERROR: Consecutive separators") + }) + + it("provides debugging info for unbalanced markers", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplace\n<<<<<<< SEARCH\nanother" + const result = strategy["analyzeDiffStructure"](diff) + + expect(result).toContain("Found 2 SEARCH markers") + expect(result).toContain("Found 1 separator (=======) markers") + expect(result).toContain("Found 0 REPLACE markers") + expect(result).toContain("WARNING: Unbalanced markers") + }) + }) + + describe("validateMarkerSequencing with Grok detection", () => { + it("returns helpful error for Grok-style malformed diff", () => { + const diff = "=======\ncontent\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(false) + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("This often happens when AI models generate incorrect diff syntax") + expect(result.error).toContain("DEBUGGING INFO:") + expect(result.error).toContain("SUGGESTIONS:") + expect(result.debugInfo).toBeDefined() + }) + + it("returns helpful error for consecutive separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(false) + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("Try using the read_file tool first") + expect(result.error).toContain("Use simpler, smaller diff blocks") + }) + + it("still validates normal diffs correctly", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(true) + }) + }) + + describe("applyDiff with malformed content", () => { + it("returns error with debug info for malformed diff", async () => { + const originalContent = "function test() {\n return true;\n}" + const malformedDiff = "=======\nfunction test() {\n return false;\n}\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, malformedDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + } + }) + + it("handles Grok-style issues gracefully", async () => { + const originalContent = "const x = 1;" + const grokDiff = "<<<<<<< SEARCH\nconst x = 1;\n=======\n=======\nconst x = 2;\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, grokDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("ERROR: Consecutive separators") + } + }) + }) + }) + + describe("MultiFileSearchReplaceDiffStrategy", () => { + let strategy: MultiFileSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiFileSearchReplaceDiffStrategy() + }) + + describe("detectGrokMalformedDiff", () => { + it("detects consecutive separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("detects separator before search marker", () => { + const diff = "=======\n<<<<<<< SEARCH\ncontent\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("detects too many separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(true) + }) + + it("returns false for valid diff", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(false) + }) + + it("returns false for multiple valid diffs", () => { + const diff = + "<<<<<<< SEARCH\ncontent1\n=======\nreplace1\n>>>>>>> REPLACE\n\n" + + "<<<<<<< SEARCH\ncontent2\n=======\nreplace2\n>>>>>>> REPLACE" + const result = strategy["detectGrokMalformedDiff"](diff) + expect(result).toBe(false) + }) + }) + + describe("analyzeDiffStructure", () => { + it("provides debugging info for malformed diff", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["analyzeDiffStructure"](diff) + + expect(result).toContain("Found 1 SEARCH markers") + expect(result).toContain("Found 2 separator (=======) markers") + expect(result).toContain("Found 1 REPLACE markers") + expect(result).toContain("WARNING: More separators than SEARCH blocks") + expect(result).toContain("ERROR: Consecutive separators") + }) + + it("provides debugging info for unbalanced markers", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplace\n<<<<<<< SEARCH\nanother" + const result = strategy["analyzeDiffStructure"](diff) + + expect(result).toContain("Found 2 SEARCH markers") + expect(result).toContain("Found 1 separator (=======) markers") + expect(result).toContain("Found 0 REPLACE markers") + expect(result).toContain("WARNING: Unbalanced markers") + }) + }) + + describe("validateMarkerSequencing with Grok detection", () => { + it("returns helpful error for Grok-style malformed diff", () => { + const diff = "=======\ncontent\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(false) + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("This often happens when AI models generate incorrect diff syntax") + expect(result.error).toContain("DEBUGGING INFO:") + expect(result.error).toContain("SUGGESTIONS:") + expect(result.debugInfo).toBeDefined() + }) + + it("returns helpful error for consecutive separators", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\n=======\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(false) + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("Try using the read_file tool first") + expect(result.error).toContain("Use simpler, smaller diff blocks") + }) + + it("still validates normal diffs correctly", () => { + const diff = "<<<<<<< SEARCH\ncontent\n=======\nreplacement\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.success).toBe(true) + }) + }) + + describe("applyDiff with malformed content", () => { + it("returns error with debug info for malformed diff", async () => { + const originalContent = "function test() {\n return true;\n}" + const malformedDiff = "=======\nfunction test() {\n return false;\n}\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, malformedDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + } + }) + + it("handles Grok-style issues gracefully", async () => { + const originalContent = "const x = 1;" + const grokDiff = "<<<<<<< SEARCH\nconst x = 1;\n=======\n=======\nconst x = 2;\n>>>>>>> REPLACE" + + const result = await strategy.applyDiff(originalContent, grokDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("ERROR: Consecutive separators") + } + }) + }) + }) + + describe("Real-world Grok scenarios", () => { + let strategy: MultiSearchReplaceDiffStrategy + + beforeEach(() => { + strategy = new MultiSearchReplaceDiffStrategy() + }) + + it("handles case where Grok adds consecutive separators", async () => { + const originalContent = `function processPayment(amount) { + // Process payment logic + return { success: true }; +}` + + // Simulating a Grok model that incorrectly adds consecutive separators + // This triggers Grok detection + const grokDiff = [ + "<<<<<<< SEARCH", + "function processPayment(amount) {", + " // Process payment logic", + "=======", + "=======", + "function processPayment(amount, currency) {", + " // Enhanced payment logic", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, grokDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + } + }) + + it("handles case where separator appears before search marker", async () => { + // This simulates the exact issue @mrbm reported + const originalContent = `export function calculateTotal(items) { + let total = 0; + for (const item of items) { + total += item.price; + } + return total; +}` + + // Malformed diff that Grok might generate - separator before search + const malformedDiff = [ + "=======", + "<<<<<<< SEARCH", + "export function calculateTotal(items) {", + " let total = 0;", + "=======", + "export function calculateTotal(items, taxRate = 0) {", + " let total = 0;", + ">>>>>>> REPLACE", + ].join("\n") + + const result = await strategy.applyDiff(originalContent, malformedDiff) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).not.toContain("When removing merge conflict markers") + expect(result.error).toContain("AI models generate incorrect diff syntax") + } + }) + + it("provides actionable suggestions for Grok users", () => { + const diff = "=======\ncontent\n>>>>>>> REPLACE" + const result = strategy["validateMarkerSequencing"](diff) + + expect(result.error).toContain("Try using the read_file tool first") + expect(result.error).toContain("Ensure your SEARCH block exactly matches") + expect(result.error).toContain("Use simpler, smaller diff blocks") + expect(result.error).toContain("CORRECT FORMAT:") + }) + }) +}) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts index b25286f5fa..ebe78ade4e 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts @@ -96,8 +96,9 @@ describe("MultiSearchReplaceDiffStrategy", () => { const diff = "=======\n" + "content\n" + ">>>>>>> REPLACE" const result = strategy["validateMarkerSequencing"](diff) expect(result.success).toBe(false) - expect(result.error).toContain("'=======' found in your diff content") - expect(result.error).toContain("Diff block is malformed") + // This now triggers Grok detection instead of the regular error + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("AI models generate incorrect diff syntax") }) it("detects missing separator", () => { @@ -112,8 +113,9 @@ describe("MultiSearchReplaceDiffStrategy", () => { const diff = "<<<<<<< SEARCH\n" + "content\n" + "=======\n" + "=======\n" + ">>>>>>> REPLACE" const result = strategy["validateMarkerSequencing"](diff) expect(result.success).toBe(false) - expect(result.error).toContain("'=======' found in your diff content") - expect(result.error).toContain("When removing merge conflict markers") + // This now triggers Grok detection for consecutive separators + expect(result.error).toContain("The diff content appears to be malformed") + expect(result.error).toContain("AI models generate incorrect diff syntax") }) it("detects replace before separator (merge conflict message)", () => { diff --git a/src/core/diff/strategies/multi-file-search-replace.ts b/src/core/diff/strategies/multi-file-search-replace.ts index a212cf2b8e..417c7a33da 100644 --- a/src/core/diff/strategies/multi-file-search-replace.ts +++ b/src/core/diff/strategies/multi-file-search-replace.ts @@ -242,7 +242,36 @@ Each file requires its own path, start_line, and diff elements. .replace(/^\\:start_line:/gm, ":start_line:") } - private validateMarkerSequencing(diffContent: string): { success: boolean; error?: string } { + private validateMarkerSequencing(diffContent: string): { success: boolean; error?: string; debugInfo?: string } { + // First, check if this looks like a Grok model issue with malformed content + // Grok models sometimes generate extra markers or malformed diffs + const isLikelyGrokIssue = this.detectGrokMalformedDiff(diffContent) + + if (isLikelyGrokIssue) { + // Log for debugging but try to be more helpful + const debugInfo = this.analyzeDiffStructure(diffContent) + return { + success: false, + error: + `ERROR: The diff content appears to be malformed. This often happens when AI models generate incorrect diff syntax.\n\n` + + `DEBUGGING INFO:\n${debugInfo}\n\n` + + `SUGGESTIONS:\n` + + `1. Try using the read_file tool first to see the exact file content\n` + + `2. Ensure your SEARCH block exactly matches the file content (including whitespace)\n` + + `3. Use simpler, smaller diff blocks for more reliable results\n` + + `4. If the file contains special characters or markers, they may need escaping\n\n` + + `CORRECT FORMAT:\n` + + `<<<<<<< SEARCH\n` + + `:start_line: (optional) The line number where search starts\n` + + `-------\n` + + `[exact content to find]\n` + + `=======\n` + + `[new content to replace with]\n` + + `>>>>>>> REPLACE`, + debugInfo: debugInfo, + } + } + enum State { START, AFTER_SEARCH, @@ -291,7 +320,7 @@ Each file requires its own path, start_line, and diff elements. "\n" + "CORRECT FORMAT:\n\n" + "<<<<<<< SEARCH\n" + - ":start_line: (required) The line number of original content where the search block starts.\n" + + ":start_line: (optional) The line number where search starts\n" + "-------\n" + "[exact content to find including whitespace]\n" + "=======\n" + @@ -387,6 +416,83 @@ Each file requires its own path, start_line, and diff elements. } } + /** + * Detects if the diff content appears to be malformed in a way typical of Grok models + */ + private detectGrokMalformedDiff(diffContent: string): boolean { + const lines = diffContent.split("\n") + + // Check for common Grok model issues: + // 1. Multiple consecutive separators + // 2. Separators appearing before any SEARCH marker + // 3. Incomplete diff blocks + // 4. Markers in unexpected positions + + let consecutiveSeparators = 0 + let hasSearchMarker = false + let separatorBeforeSearch = false + + for (let i = 0; i < lines.length; i++) { + const line = lines[i].trim() + + if (line === "=======") { + if (!hasSearchMarker) { + separatorBeforeSearch = true + } + if (i > 0 && lines[i - 1].trim() === "=======") { + consecutiveSeparators++ + } + } + + if (line.startsWith("<<<<<<< SEARCH")) { + hasSearchMarker = true + } + } + + // If we see patterns typical of Grok issues, flag it + return ( + consecutiveSeparators > 0 || + separatorBeforeSearch || + lines.filter((l) => l.trim() === "=======").length > + lines.filter((l) => l.trim().startsWith("<<<<<<< SEARCH")).length * 2 + ) + } + + /** + * Analyzes the diff structure to provide debugging information + */ + private analyzeDiffStructure(diffContent: string): string { + const lines = diffContent.split("\n") + const searchMarkers = lines.filter((l) => l.trim().startsWith("<<<<<<< SEARCH")).length + const separators = lines.filter((l) => l.trim() === "=======").length + const replaceMarkers = lines.filter((l) => l.trim() === ">>>>>>> REPLACE").length + + let debugInfo = `- Found ${searchMarkers} SEARCH markers\n` + debugInfo += `- Found ${separators} separator (=======) markers\n` + debugInfo += `- Found ${replaceMarkers} REPLACE markers\n` + + // Check for balance + if (searchMarkers !== replaceMarkers) { + debugInfo += `- WARNING: Unbalanced markers (SEARCH: ${searchMarkers}, REPLACE: ${replaceMarkers})\n` + } + if (separators > searchMarkers) { + debugInfo += `- WARNING: More separators than SEARCH blocks (may indicate malformed content)\n` + } + + // Find problematic lines + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (line.trim() === "=======" && i > 0) { + const prevLine = lines[i - 1].trim() + if (prevLine === "=======") { + debugInfo += `- ERROR: Consecutive separators at lines ${i} and ${i + 1}\n` + } + } + } + + return debugInfo + } + async applyDiff( originalContent: string, diffContent: string | Array<{ content: string; startLine?: number }>, diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index a6a9913203..c29aaf1e2a 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -190,7 +190,36 @@ Only use a single line of '=======' between search and replacement content, beca .replace(/^\\:start_line:/gm, ":start_line:") } - private validateMarkerSequencing(diffContent: string): { success: boolean; error?: string } { + private validateMarkerSequencing(diffContent: string): { success: boolean; error?: string; debugInfo?: string } { + // First, check if this looks like a Grok model issue with malformed content + // Grok models sometimes generate extra markers or malformed diffs + const isLikelyGrokIssue = this.detectGrokMalformedDiff(diffContent) + + if (isLikelyGrokIssue) { + // Log for debugging but try to be more helpful + const debugInfo = this.analyzeDiffStructure(diffContent) + return { + success: false, + error: + `ERROR: The diff content appears to be malformed. This often happens when AI models generate incorrect diff syntax.\n\n` + + `DEBUGGING INFO:\n${debugInfo}\n\n` + + `SUGGESTIONS:\n` + + `1. Try using the read_file tool first to see the exact file content\n` + + `2. Ensure your SEARCH block exactly matches the file content (including whitespace)\n` + + `3. Use simpler, smaller diff blocks for more reliable results\n` + + `4. If the file contains special characters or markers, they may need escaping\n\n` + + `CORRECT FORMAT:\n` + + `<<<<<<< SEARCH\n` + + `:start_line: (optional) The line number where search starts\n` + + `-------\n` + + `[exact content to find]\n` + + `=======\n` + + `[new content to replace with]\n` + + `>>>>>>> REPLACE`, + debugInfo: debugInfo, + } + } + enum State { START, AFTER_SEARCH, @@ -238,7 +267,7 @@ Only use a single line of '=======' between search and replacement content, beca "\n" + "CORRECT FORMAT:\n\n" + "<<<<<<< SEARCH\n" + - ":start_line: (required) The line number of original content where the search block starts.\n" + + ":start_line: (optional) The line number where search starts\n" + "-------\n" + "[exact content to find including whitespace]\n" + "=======\n" + @@ -334,6 +363,83 @@ Only use a single line of '=======' between search and replacement content, beca } } + /** + * Detects if the diff content appears to be malformed in a way typical of Grok models + */ + private detectGrokMalformedDiff(diffContent: string): boolean { + const lines = diffContent.split("\n") + + // Check for common Grok model issues: + // 1. Multiple consecutive separators + // 2. Separators appearing before any SEARCH marker + // 3. Incomplete diff blocks + // 4. Markers in unexpected positions + + let consecutiveSeparators = 0 + let hasSearchMarker = false + let separatorBeforeSearch = false + + for (let i = 0; i < lines.length; i++) { + const line = lines[i].trim() + + if (line === "=======") { + if (!hasSearchMarker) { + separatorBeforeSearch = true + } + if (i > 0 && lines[i - 1].trim() === "=======") { + consecutiveSeparators++ + } + } + + if (line.startsWith("<<<<<<< SEARCH")) { + hasSearchMarker = true + } + } + + // If we see patterns typical of Grok issues, flag it + return ( + consecutiveSeparators > 0 || + separatorBeforeSearch || + lines.filter((l) => l.trim() === "=======").length > + lines.filter((l) => l.trim().startsWith("<<<<<<< SEARCH")).length * 2 + ) + } + + /** + * Analyzes the diff structure to provide debugging information + */ + private analyzeDiffStructure(diffContent: string): string { + const lines = diffContent.split("\n") + const searchMarkers = lines.filter((l) => l.trim().startsWith("<<<<<<< SEARCH")).length + const separators = lines.filter((l) => l.trim() === "=======").length + const replaceMarkers = lines.filter((l) => l.trim() === ">>>>>>> REPLACE").length + + let debugInfo = `- Found ${searchMarkers} SEARCH markers\n` + debugInfo += `- Found ${separators} separator (=======) markers\n` + debugInfo += `- Found ${replaceMarkers} REPLACE markers\n` + + // Check for balance + if (searchMarkers !== replaceMarkers) { + debugInfo += `- WARNING: Unbalanced markers (SEARCH: ${searchMarkers}, REPLACE: ${replaceMarkers})\n` + } + if (separators > searchMarkers) { + debugInfo += `- WARNING: More separators than SEARCH blocks (may indicate malformed content)\n` + } + + // Find problematic lines + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (line.trim() === "=======" && i > 0) { + const prevLine = lines[i - 1].trim() + if (prevLine === "=======") { + debugInfo += `- ERROR: Consecutive separators at lines ${i} and ${i + 1}\n` + } + } + } + + return debugInfo + } + async applyDiff( originalContent: string, diffContent: string,