Skip to content

Commit 1f4e9d6

Browse files
committed
fix: Remove HTML entity unescaping for non-Claude models in apply_diff (#4077)
1 parent fdb1f35 commit 1f4e9d6

File tree

3 files changed

+51
-15
lines changed

3 files changed

+51
-15
lines changed

src/core/tools/applyDiffTool.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } f
1010
import { formatResponse } from "../prompts/responses"
1111
import { fileExistsAtPath } from "../../utils/fs"
1212
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
13-
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1413

1514
export async function applyDiffToolLegacy(
1615
cline: Task,
@@ -23,10 +22,6 @@ export async function applyDiffToolLegacy(
2322
const relPath: string | undefined = block.params.path
2423
let diffContent: string | undefined = block.params.diff
2524

26-
if (diffContent && !cline.api.getModel().id.includes("claude")) {
27-
diffContent = unescapeHtmlEntities(diffContent)
28-
}
29-
3025
const sharedMessageProps: ClineSayTool = {
3126
tool: "appliedDiff",
3227
path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)),

src/core/tools/multiApplyDiffTool.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } f
1010
import { formatResponse } from "../prompts/responses"
1111
import { fileExistsAtPath } from "../../utils/fs"
1212
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
13-
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1413
import { parseXml } from "../../utils/xml"
1514
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
1615
import { applyDiffToolLegacy } from "./applyDiffTool"
@@ -410,16 +409,8 @@ Original error: ${errorMessage}`
410409
let successCount = 0
411410
let formattedError = ""
412411

413-
// Pre-process all diff items for HTML entity unescaping if needed
414-
const processedDiffItems = !cline.api.getModel().id.includes("claude")
415-
? diffItems.map((item) => ({
416-
...item,
417-
content: item.content ? unescapeHtmlEntities(item.content) : item.content,
418-
}))
419-
: diffItems
420-
421412
// Apply all diffs at once with the array-based method
422-
const diffResult = (await cline.diffStrategy?.applyDiff(originalContent, processedDiffItems)) ?? {
413+
const diffResult = (await cline.diffStrategy?.applyDiff(originalContent, diffItems)) ?? {
423414
success: false,
424415
error: "No diff strategy available - please ensure a valid diff strategy is configured",
425416
}

src/utils/__tests__/text-normalization.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,55 @@ describe("Text normalization utilities", () => {
7575
expect(unescapeHtmlEntities("")).toBe("")
7676
expect(unescapeHtmlEntities(undefined as unknown as string)).toBe(undefined)
7777
})
78+
79+
// Issue #4077 - HTML entity handling verification
80+
it("should demonstrate that unescapeHtmlEntities converts & to &", () => {
81+
const input = "// Step 5 & 6: Verify the data"
82+
const result = unescapeHtmlEntities(input)
83+
84+
expect(result).toBe("// Step 5 & 6: Verify the data")
85+
expect(result).not.toBe(input)
86+
expect(result).not.toContain("&")
87+
expect(result).toContain("&")
88+
})
89+
90+
it("should show the exact problem from issue #4077", () => {
91+
// The user's SEARCH block content (what they see in their file)
92+
const searchContent = "// Adım 5 & 6: İşaret edilen verinin bölümünü bul ve doğrula"
93+
94+
// After unescaping (what happens for non-Claude models)
95+
const unescapedSearch = unescapeHtmlEntities(searchContent)
96+
97+
// The actual file content (contains HTML entity)
98+
const fileContent = "// Adım 5 & 6: İşaret edilen verinin bölümünü bul ve doğrula"
99+
100+
// This demonstrates the mismatch
101+
expect(unescapedSearch).toBe("// Adım 5 & 6: İşaret edilen verinin bölümünü bul ve doğrula")
102+
expect(unescapedSearch).not.toBe(fileContent)
103+
104+
// The key issue: when searching for the unescaped version in the file content
105+
// it won't find a match because the file has & but we're searching for &
106+
const willFindMatch = fileContent.includes(unescapedSearch)
107+
expect(willFindMatch).toBe(false)
108+
109+
// But if we search for the original (not unescaped), it would work
110+
const wouldFindMatchWithoutUnescape = fileContent.includes(searchContent)
111+
expect(wouldFindMatchWithoutUnescape).toBe(true)
112+
})
113+
114+
it("should verify all HTML entities are unescaped", () => {
115+
const testCases = [
116+
{ input: "&lt;", expected: "<" },
117+
{ input: "&gt;", expected: ">" },
118+
{ input: "&quot;", expected: '"' },
119+
{ input: "&#39;", expected: "'" },
120+
{ input: "&apos;", expected: "'" },
121+
{ input: "&amp;", expected: "&" },
122+
]
123+
124+
testCases.forEach(({ input, expected }) => {
125+
expect(unescapeHtmlEntities(input)).toBe(expected)
126+
})
127+
})
78128
})
79129
})

0 commit comments

Comments
 (0)