Skip to content

Commit 518bae4

Browse files
nigeldelvierocoderabbitai[bot]
nigeldelviero
and
coderabbitai[bot]
authored
fix(edit-unsuccessfull): introduce configurable relaxed diff thresholds and diagnostics (#470)
* feat(issue-457-edit-unsuccessfull): introduce configurable relaxed diff thresholds and diagnostics * Update packages/types/src/global-settings.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(issue-457-edit-unsuccessfull): fix js doc and remove hard-coded english * fix(issue-457-edit-unsuccessfull): limit original content preview to a bounded window on math failure * fix(issue-457-edit-unsuccessfull): clamp the threshold * fix(issue-457-edit-unsuccessfull): remove access private property * fix(issue-457-edit-unsuccessfull): remove from Pick and import types * fix(issue-457-edit-unsuccessfull): Move diffFuzzyThreshold to new 'File Edits' section * Revert "fix(issue-457-edit-unsuccessfull): limit original content preview to a bounded window on math failure" This reverts commit 4b2af0d. * fix(issue-457-edit-unsuccessfull): clarify diffuzyThreshold JSDoc comment * fix(issue-457-edit-unsuccessfull): Default diffFuzzyThreshold to 1.0 for safety * fix(issue-457-edit-unsuccessfull): Correct the documented default for diffFuzzyThreshold and fix failure-diagnostics assertions unconditional * fix(issue-457-edit-unsuccessfull): added missing translations * feat(issue-457-edit-unsuccessfull): introduce configurable relaxed diff thresholds and diagnostics * Update packages/types/src/global-settings.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(issue-457-edit-unsuccessfull): fix js doc and remove hard-coded english * fix(issue-457-edit-unsuccessfull): limit original content preview to a bounded window on math failure * fix(issue-457-edit-unsuccessfull): remove from Pick and import types * fix(issue-457-edit-unsuccessfull): Move diffFuzzyThreshold to new 'File Edits' section * Revert "fix(issue-457-edit-unsuccessfull): limit original content preview to a bounded window on math failure" This reverts commit 4b2af0d. * fix(issue-457-edit-unsuccessfull): clarify diffuzyThreshold JSDoc comment * fix(issue-457-edit-unsuccessfull): Correct the documented default for diffFuzzyThreshold and fix failure-diagnostics assertions unconditional * fix(multi-search-replace): cap original content preview to bounded window when startLine is absent Prevents full file content (potentially including secrets) from being sent to the LLM API via pushToolResult on failed match. When startLine is not provided, the error message now slices resultLines around matchIndex using bufferLines, mirroring the existing behavior of the startLine-present branch. * fix(multi-search-replace): address review feedback - Cap original content preview to bounded window when startLine is absent, preventing full file content (potentially including secrets) from being sent to LLM via pushToolError - Move test assertions outside if (!result.success) guards so tests fail explicitly instead of silently passing when failParts is absent * fix(multi-search-replace): fix coverage holes - Fix ternary condition (startLine && endLine instead of startLine !== undefined && endLine !== undefined) so bounded window else arm is reachable when startLine is 0/NaN - Add test for fuzzy match failure without :start_line: to exercise bounded window code path - Add UI test for diffFuzzyThreshold slider interaction * fix(test): align no-match diagnostics expectations with current error output * test(settings): remove invalid checkpoints default assertion * test(settings): cover settings fallback defaults * test: stabilize settings fallback defaults * test: cover ClineProvider partial branches * test: cover multi-search-replace failure branches * test: cover remaining codecov patch branches on ClineProvider and multi-search-replace * docs: correct constructor comment to reflect actual default threshold (1.0) * fix: align schema min(0.5) with JSDoc range and constructor clamp * fix: remove duplicate fileEdits.diffFuzzyThreshold entry in en locale * test: cover diffFuzzyThreshold passthrough in getStateToPostToWebview --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 0f1054e commit 518bae4

32 files changed

Lines changed: 588 additions & 21 deletions

‎packages/types/src/global-settings.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
import { z } from "zod"
22

3-
import { type Keys } from "./type-fu.js"
3+
import { codebaseIndexConfigSchema, codebaseIndexModelsSchema } from "./codebase-index.js"
4+
import { experimentsSchema } from "./experiment.js"
5+
import { historyItemSchema } from "./history.js"
6+
import { customModePromptsSchema, customSupportPromptsSchema, modeConfigSchema } from "./mode.js"
47
import {
58
type ProviderSettings,
69
PROVIDER_SETTINGS_KEYS,
710
providerSettingsEntrySchema,
811
providerSettingsSchema,
912
} from "./provider-settings.js"
10-
import { historyItemSchema } from "./history.js"
11-
import { codebaseIndexModelsSchema, codebaseIndexConfigSchema } from "./codebase-index.js"
12-
import { experimentsSchema } from "./experiment.js"
1313
import { telemetrySettingsSchema } from "./telemetry.js"
14-
import { modeConfigSchema } from "./mode.js"
15-
import { customModePromptsSchema, customSupportPromptsSchema } from "./mode.js"
1614
import { toolNamesSchema } from "./tool.js"
15+
import { type Keys } from "./type-fu.js"
1716
import { languagesSchema } from "./vscode.js"
1817

1918
/**
@@ -23,6 +22,16 @@ import { languagesSchema } from "./vscode.js"
2322
*/
2423
export const DEFAULT_WRITE_DELAY_MS = 1000
2524

25+
/**
26+
* Default fuzzy matching threshold for the multi-search-replace diff strategy.
27+
* A value of 1.0 (exact match) is used by default for safety, especially when
28+
* auto-approval for writes is enabled. This prevents unintended changes from
29+
* being applied due to minor mismatches. Users can lower this threshold manually
30+
* in settings to reduce "Edit Unsuccessful" errors caused by minor whitespace
31+
* or formatting differences, accepting a higher risk of unintended edits.
32+
*/
33+
export const DEFAULT_DIFF_FUZZY_THRESHOLD = 1.0
34+
2635
/**
2736
* Terminal output preview size options for persisted command output.
2837
*
@@ -102,6 +111,12 @@ export const globalSettingsSchema = z.object({
102111
alwaysAllowWriteOutsideWorkspace: z.boolean().optional(),
103112
alwaysAllowWriteProtected: z.boolean().optional(),
104113
writeDelayMs: z.number().min(0).optional(),
114+
/**
115+
* Fuzzy matching threshold for the multi-search-replace diff strategy.
116+
* Range: 0.5 (50% minimum similarity) to 1.0 (exact match only).
117+
* `@default` 1.0
118+
*/
119+
diffFuzzyThreshold: z.number().min(0.5).max(1).optional(),
105120
requestDelaySeconds: z.number().optional(),
106121
alwaysAllowMcp: z.boolean().optional(),
107122
alwaysAllowModeSwitch: z.boolean().optional(),

‎packages/types/src/vscode-extension-host.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ export type ExtensionState = Pick<
332332
taskHistory: HistoryItem[]
333333

334334
writeDelayMs: number
335+
diffFuzzyThreshold: number
335336

336337
enableCheckpoints: boolean
337338
checkpointTimeout: number // Timeout for checkpoint initialization in seconds (default: 15)

‎src/__tests__/single-open-invariant.spec.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,46 @@ describe("Single-open-task invariant", () => {
7373
expect(addClineToStack).toHaveBeenCalledTimes(1)
7474
})
7575

76+
it("Subtask create: keeps existing task open when parentTask is provided", async () => {
77+
vi.spyOn(ProfileValidatorMod.ProfileValidator, "isProfileAllowed").mockReturnValue(true)
78+
79+
const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
80+
const addClineToStack = vi.fn().mockResolvedValue(undefined)
81+
const parentTask = { taskId: "parent-1" }
82+
83+
const provider = {
84+
clineStack: [parentTask],
85+
setValues: vi.fn(),
86+
getState: vi.fn().mockResolvedValue({
87+
apiConfiguration: { apiProvider: "anthropic", consecutiveMistakeLimit: 0 },
88+
organizationAllowList: "*",
89+
enableCheckpoints: true,
90+
checkpointTimeout: 60,
91+
cloudUserInfo: null,
92+
}),
93+
removeClineFromStack,
94+
addClineToStack,
95+
setProviderProfile: vi.fn(),
96+
log: vi.fn(),
97+
getStateToPostToWebview: vi.fn(),
98+
providerSettingsManager: { getModeConfigId: vi.fn(), listConfig: vi.fn() },
99+
customModesManager: { getCustomModes: vi.fn().mockResolvedValue([]) },
100+
taskCreationCallback: vi.fn(),
101+
contextProxy: {
102+
extensionUri: {},
103+
setValue: vi.fn(),
104+
getValue: vi.fn(),
105+
setProviderSettings: vi.fn(),
106+
getProviderSettings: vi.fn(() => ({})),
107+
},
108+
} as unknown as ClineProvider
109+
110+
await (ClineProvider.prototype as any).createTask.call(provider, "Subtask", undefined, parentTask as any)
111+
112+
expect(removeClineFromStack).not.toHaveBeenCalled()
113+
expect(addClineToStack).toHaveBeenCalledTimes(1)
114+
})
115+
76116
it("History resume path always closes current before rehydration (non-rehydrating case)", async () => {
77117
const removeClineFromStack = vi.fn().mockResolvedValue(undefined)
78118
const addClineToStack = vi.fn().mockResolvedValue(undefined)

‎src/core/diff/strategies/__tests__/multi-search-replace.spec.ts‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,119 @@ function processData(data) {
878878
expect(result.success).toBe(false)
879879
})
880880

881+
it("should include line range debug info when search fails with start_line marker", async () => {
882+
const originalContent = "line one\nline two"
883+
const diffContent = `test.ts
884+
<<<<<<< SEARCH
885+
:start_line:999
886+
-------
887+
non-existent content that cannot be found in the file
888+
=======
889+
replacement content here
890+
>>>>>>> REPLACE`
891+
892+
const result = await strategy.applyDiff(originalContent, diffContent)
893+
expect(result.success).toBe(false)
894+
const error =
895+
!result.success && result.failParts?.[0]
896+
? "error" in result.failParts[0]
897+
? result.failParts[0].error
898+
: ""
899+
: ""
900+
expect(error).toContain("No sufficiently similar match found")
901+
expect(error).toContain("at line: 999")
902+
expect(error).toContain("Search Range: starting at line 999")
903+
expect(error).toContain("Best Match Found:\n(no match)")
904+
expect(error).toContain("Levenshtein Distance: N/A")
905+
expect(error).toContain("Best Match Length: 0 characters")
906+
})
907+
908+
it("should include scoped original content when search fails with start_line that has a low-score match", async () => {
909+
const originalContent = "function existing() {\n return 42;\n}\n"
910+
const diffContent = `test.ts
911+
<<<<<<< SEARCH
912+
:start_line:1
913+
-------
914+
function different() {
915+
return 99;
916+
}
917+
=======
918+
function newVersion() {
919+
return 99;
920+
}
921+
>>>>>>> REPLACE`
922+
923+
const result = await strategy.applyDiff(originalContent, diffContent)
924+
expect(result.success).toBe(false)
925+
const error =
926+
!result.success && result.failParts?.[0]
927+
? "error" in result.failParts[0]
928+
? result.failParts[0].error
929+
: ""
930+
: ""
931+
expect(error).toContain("No sufficiently similar match found")
932+
expect(error).toContain("at line: 1")
933+
expect(error).toContain("Search Range: starting at line 1")
934+
expect(error).toContain("Best Match Found:")
935+
expect(error).toContain("Original Content:\n1 | function existing()")
936+
})
937+
938+
it("should include best-match debug info when unscoped search is below threshold", async () => {
939+
const strictStrategy = new MultiSearchReplaceDiffStrategy(1, 5)
940+
const originalContent = "function processUsers(data) {\n return data.map(user => user.name);\n}\n"
941+
const diffContent = `test.ts
942+
<<<<<<< SEARCH
943+
function processUsers(data) {
944+
return data.map(user => user.username);
945+
}
946+
=======
947+
function processUsers(data) {
948+
return data.map(user => user.displayName);
949+
}
950+
>>>>>>> REPLACE`
951+
952+
const result = await strictStrategy.applyDiff(originalContent, diffContent)
953+
expect(result.success).toBe(false)
954+
const error =
955+
!result.success && result.failParts?.[0]
956+
? "error" in result.failParts[0]
957+
? result.failParts[0].error
958+
: ""
959+
: ""
960+
expect(error).toContain("Search Range: start to end")
961+
expect(error).toContain("Levenshtein Distance:")
962+
expect(error).toContain("characters")
963+
expect(error).toContain("Best Match Length:")
964+
expect(error).toContain("Best Match Found:\n1 | function processUsers(data)")
965+
})
966+
967+
it("should include zero-match info when unscoped search finds no similarity at all", async () => {
968+
const strictStrategy = new MultiSearchReplaceDiffStrategy(1, 0)
969+
const originalContent = "xxxxxx\nyyyyyy\nzzzzzz"
970+
const diffContent = `test.ts
971+
<<<<<<< SEARCH
972+
!!!!!!
973+
=======
974+
aaaaaa
975+
>>>>>>> REPLACE`
976+
977+
const result = await strictStrategy.applyDiff(originalContent, diffContent)
978+
expect(result.success).toBe(false)
979+
const error =
980+
!result.success && result.failParts?.[0]
981+
? "error" in result.failParts[0]
982+
? result.failParts[0].error
983+
: ""
984+
: ""
985+
expect(error).toContain("No sufficiently similar match found")
986+
expect(error).toContain("Search Range: start to end")
987+
expect(error).toContain("Best Match Found:\n(no match)")
988+
expect(error).toContain("Levenshtein Distance: N/A")
989+
expect(error).toContain("Best Match Length: 0 characters")
990+
expect(error).toContain("Original Content:")
991+
expect(error).toContain("1 | xxxxxx")
992+
})
993+
881994
it("should match content with extra whitespace", async () => {
882995
const originalContent = "function sum(a, b) {\n return a + b;\n}"
883996
const diffContent = `test.ts
@@ -1374,4 +1487,109 @@ function sum(a, b) {
13741487
}
13751488
})
13761489
})
1490+
1491+
describe("fuzzyThreshold and diagnostics", () => {
1492+
const originalContent =
1493+
"function calculateTotal(price: number, tax: number) {\n\tconst subtotal = price;\n\treturn subtotal + tax;\n}\n"
1494+
1495+
it("should succeed with near-miss match (e.g. minor whitespace diff) when threshold is 0.90", async () => {
1496+
const strategy = new MultiSearchReplaceDiffStrategy(0.9)
1497+
// Near-miss search block with slightly different formatting/whitespace (e.g., spaces instead of tab, missing semicolon)
1498+
const diff =
1499+
"<<<<<<< SEARCH\n" +
1500+
"function calculateTotal(price: number, tax: number) {\n" +
1501+
" const subtotal = price\n" +
1502+
" return subtotal + tax;\n" +
1503+
"}\n" +
1504+
"=======\n" +
1505+
"function calculateTotal(price: number, tax: number) {\n" +
1506+
" const subtotal = price;\n" +
1507+
" return (subtotal + tax) * 1.1;\n" +
1508+
"}\n" +
1509+
">>>>>>> REPLACE"
1510+
1511+
const result = await strategy.applyDiff(originalContent, diff)
1512+
expect(result.success).toBe(true)
1513+
if (result.success) {
1514+
expect(result.content).toContain("(subtotal + tax) * 1.1")
1515+
}
1516+
})
1517+
1518+
it("should fail with near-miss match when threshold is set to 1.0", async () => {
1519+
const strategy = new MultiSearchReplaceDiffStrategy(1.0)
1520+
const diff =
1521+
"<<<<<<< SEARCH\n" +
1522+
"function calculateTotal(price: number, tax: number) {\n" +
1523+
" const subtotal = price\n" +
1524+
" return subtotal + tax;\n" +
1525+
"}\n" +
1526+
"=======\n" +
1527+
"function calculateTotal(price: number, tax: number) {\n" +
1528+
" const subtotal = price;\n" +
1529+
" return (subtotal + tax) * 1.1;\n" +
1530+
"}\n" +
1531+
">>>>>>> REPLACE"
1532+
1533+
const result = await strategy.applyDiff(originalContent, diff)
1534+
expect(result.success).toBe(false)
1535+
expect(result.failParts).toBeDefined()
1536+
expect(result.failParts!.length).toBeGreaterThan(0)
1537+
const failedPart = result.failParts![0]
1538+
expect(failedPart).toHaveProperty("error")
1539+
expect((failedPart as { error: string }).error).toContain("No sufficiently similar match found")
1540+
})
1541+
1542+
it("should output enhanced error diagnostics (Levenshtein distance, character counts) when a match fails", async () => {
1543+
const strategy = new MultiSearchReplaceDiffStrategy(0.95)
1544+
const diff =
1545+
"<<<<<<< SEARCH\n" +
1546+
"function calculateGrandTotal(initialPrice: number, standardTax: number) {\n" +
1547+
" const totalVal = initialPrice\n" +
1548+
" return totalVal + standardTax;\n" +
1549+
"}\n" +
1550+
"=======\n" +
1551+
"function calculateTotal(price: number, tax: number) {\n" +
1552+
" const subtotal = price;\n" +
1553+
" return (subtotal + tax) * 1.1;\n" +
1554+
"}\n" +
1555+
">>>>>>> REPLACE"
1556+
1557+
const result = await strategy.applyDiff(originalContent, diff)
1558+
expect(result.success).toBe(false)
1559+
expect(result.failParts).toBeDefined()
1560+
expect(result.failParts!.length).toBeGreaterThan(0)
1561+
const failedPart = result.failParts![0]
1562+
expect(failedPart).toHaveProperty("error")
1563+
const errorMsg = (failedPart as { error: string }).error
1564+
expect(errorMsg).toContain("Debug Info:")
1565+
expect(errorMsg).toContain("Similarity Score:")
1566+
expect(errorMsg).toContain("Required Threshold: 95%")
1567+
expect(errorMsg).toContain("Levenshtein Distance:")
1568+
expect(errorMsg).toContain("Search Length:")
1569+
expect(errorMsg).toContain("Best Match Length:")
1570+
})
1571+
it("should report no-match diagnostics when search content is completely different and no :start_line: is given", async () => {
1572+
const strategy = new MultiSearchReplaceDiffStrategy(0.9)
1573+
const diff =
1574+
"<<<<<<< SEARCH\n" +
1575+
"§§§§§§§§§§§§\n" +
1576+
"§§§§§§§§§§§§\n" +
1577+
"=======\n" +
1578+
"¤¤¤¤¤¤¤¤¤¤¤¤\n" +
1579+
"¤¤¤¤¤¤¤¤¤¤¤¤\n" +
1580+
">>>>>>> REPLACE"
1581+
1582+
const result = await strategy.applyDiff(originalContent, diff)
1583+
expect(result.success).toBe(false)
1584+
expect(result.failParts).toBeDefined()
1585+
expect(result.failParts!.length).toBeGreaterThan(0)
1586+
const failedPart = result.failParts![0]
1587+
expect(failedPart).toHaveProperty("error")
1588+
const errorMsg = (failedPart as { error: string }).error
1589+
expect(errorMsg).toContain("No sufficiently similar match found")
1590+
expect(errorMsg).toContain("Best Match Found:")
1591+
expect(errorMsg).toContain("Levenshtein Distance:")
1592+
expect(errorMsg).toContain("Search Range: start to end")
1593+
})
1594+
})
13771595
})

0 commit comments

Comments
 (0)