Skip to content

Commit 124788b

Browse files
authored
Feat/fuzzy-file-edit (Gitlawb#1561)
* feat(FileEditTool): add whitespace-agnostic fallback matching * test(FileEditTool): add unit tests for whitespace-agnostic matcher * fix(FileEditTool): preserve boundary whitespace in fuzzy match as requested by CodeRabbit * fix: address PR feedback on token boundaries and indentation recovery * fix: recover deep indentation for nested blocks * fix: isolate trailing newline boundary from next line indentation * fix: abort fuzzy match if requested indentation map conflicts * fix: resolve typecheck error by checking adjustNewStringIndentation return value * fix: preserve exact vertical newline count and horizontal boundary spacing * fix: enforce strict inline whitespace and preserve Markdown hard breaks * fix: add missing boolean argument to normalizeIndentation in adjustNewStringIndentation
1 parent 9a72ecd commit 124788b

2 files changed

Lines changed: 447 additions & 0 deletions

File tree

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
import { describe, expect, test } from 'bun:test'
2+
import { findWhitespaceAgnosticMatch, adjustNewStringIndentation } from './utils.js'
3+
4+
describe('findWhitespaceAgnosticMatch', () => {
5+
test('returns exact match for simple string', () => {
6+
const fileContent = 'const x = 1;\nconst y = 2;'
7+
const searchString = 'const x = 1;'
8+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('const x = 1;')
9+
})
10+
11+
test('handles missing trailing newlines', () => {
12+
const fileContent = 'function hello() {\n console.log("world");\n}\n'
13+
const searchString = 'function hello() {\n console.log("world");\n}'
14+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('function hello() {\n console.log("world");\n}')
15+
})
16+
17+
test('handles indentation changes', () => {
18+
const fileContent = 'function hello() {\n console.log("world");\n}'
19+
const searchString = 'function hello() {\n console.log("world");\n}'
20+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('function hello() {\n console.log("world");\n}')
21+
})
22+
23+
test('rejects inline space changes to protect tokenization and operators', () => {
24+
const fileContent = 'if ( a === b ) { return c; }'
25+
const searchString = 'if(a===b){return c;}'
26+
// Inline space differences are now strictly rejected to prevent merging/splitting tokens
27+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
28+
})
29+
30+
test('prevents operator token collapsing across fuzzy matches', () => {
31+
const fileContent = 'const z = i++ + j;'
32+
const searchString = 'const z = i + ++j;'
33+
// If inline spaces are ignored, both become i+++j, which would be a dangerous match.
34+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
35+
})
36+
37+
test('recovers leading boundary horizontal whitespace without consuming line breaks', () => {
38+
const fileContent = 'function hello() {\n foo();\n}'
39+
const searchString = ' foo();' // Agent provided leading spaces
40+
// Leading spaces are ignored in the match, and boundary expansion
41+
// recovers the exact file indentation. The `\n` is safely preserved!
42+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe(' foo();')
43+
})
44+
45+
test('prevents trailing-newline searches from consuming next line indentation', () => {
46+
const fileContent = 'if ok:\n foo()\n bar()\n'
47+
const searchString = ' foo()\n'
48+
const actualOldString = findWhitespaceAgnosticMatch(fileContent, searchString)
49+
expect(actualOldString).toBe(' foo()\n')
50+
})
51+
52+
test('rejects fuzzy match when LLM collapses blank lines (CodeRabbit P2 fix)', () => {
53+
const fileContent = 'A paragraph.\n\nNext paragraph.'
54+
const searchString = 'A paragraph.\nNext paragraph.'
55+
// The exact newline count mismatch forces it to reject the fuzzy match.
56+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
57+
})
58+
59+
test('rejects fuzzy match when LLM hallucinates blank lines (CodeRabbit P2 fix)', () => {
60+
const fileContent = 'A paragraph.\nNext paragraph.'
61+
const searchString = 'A paragraph.\n\nNext paragraph.'
62+
// The exact newline count mismatch forces it to reject the fuzzy match.
63+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
64+
})
65+
66+
test('preserves Markdown hard breaks in fuzzy match (CodeRabbit P2 fix)', () => {
67+
const fileContent = 'foo \nbar'
68+
const searchString = 'foo\nbar'
69+
// isMarkdown = true protects trailing spaces before a newline
70+
expect(findWhitespaceAgnosticMatch(fileContent, searchString, true)).toBeNull()
71+
})
72+
73+
test('ignores trailing garbage spaces for non-Markdown files', () => {
74+
const fileContent = 'foo \nbar'
75+
const searchString = 'foo\nbar'
76+
// isMarkdown = false drops trailing spaces to be agnostic
77+
expect(findWhitespaceAgnosticMatch(fileContent, searchString, false)).toBe('foo \nbar')
78+
})
79+
80+
test('keeps inline whitespace exact to protect semantics (CodeRabbit P2 fix)', () => {
81+
const fileContent = 'const msg = "hello world";'
82+
const searchString = 'const msg = "hello world";'
83+
// The inline spaces do not match, so it rejects it!
84+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
85+
})
86+
87+
test('prevents matching across token boundaries', () => {
88+
// LLM forgot the space between two tokens
89+
const fileContent = 'const foobar = 1;'
90+
const searchString = 'const foo bar = 1;'
91+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
92+
93+
// LLM inserted a space inside a token
94+
const fileContent2 = 'const foo bar = 1;'
95+
const searchString2 = 'const foobar = 1;'
96+
expect(findWhitespaceAgnosticMatch(fileContent2, searchString2)).toBeNull()
97+
})
98+
99+
test('returns null if no match found', () => {
100+
const fileContent = 'const a = 1;'
101+
const searchString = 'const b = 2;'
102+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
103+
})
104+
105+
test('returns null if multiple matches found to prevent accidental replacement', () => {
106+
const fileContent = 'const a = 1;\nconst a = 1;'
107+
const searchString = 'const a = 1;'
108+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
109+
})
110+
111+
test('prevents multiline strings from matching single-line strings with same tokens', () => {
112+
// P1: A newline in the search string should not match an inline space in the file
113+
const fileContent = 'const x = a + b;'
114+
const searchString = 'const x = a\n + b;'
115+
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
116+
117+
const fileContent2 = '.foo .bar { color: red; }'
118+
const searchString2 = '.foo\n .bar { color: red; }'
119+
expect(findWhitespaceAgnosticMatch(fileContent2, searchString2)).toBeNull()
120+
})
121+
})
122+
123+
describe('adjustNewStringIndentation', () => {
124+
test('returns newString unmodified if oldString and fileMatch have same indentation', () => {
125+
const oldString = ' foo();\n bar();'
126+
const fileMatch = ' foo();\n bar();'
127+
const newString = ' foo();\n baz();'
128+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(newString)
129+
})
130+
131+
test('recovers nested structure when root has no indentation (CodeRabbit P2 fix)', () => {
132+
const oldString = 'if ok:\n foo()'
133+
const fileMatch = 'if ok:\n foo()' // file uses 4 spaces instead of 2 for nested line
134+
const newString = 'if ok:\n bar()'
135+
// It should preserve the nested 4 spaces for bar() even though the root `if ok:` is 0 spaces
136+
const expected = 'if ok:\n bar()'
137+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
138+
})
139+
140+
test('handles deeper unseen relative indentation intelligently', () => {
141+
const oldString = 'if ok:\n foo()'
142+
const fileMatch = 'if ok:\n foo()'
143+
const newString = 'if ok:\n for x in y:\n bar()' // LLM added a deeper block at 4 spaces
144+
// It should map 0 -> 0, 2 -> 4, and 4 -> 4 + 2 remaining = 6
145+
const expected = 'if ok:\n for x in y:\n bar()'
146+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
147+
})
148+
149+
test('adds indentation when file has more overall indentation', () => {
150+
const oldString = ' foo();\n bar();'
151+
const fileMatch = ' foo();\n bar();' // file has +2 spaces
152+
const newString = ' foo();\n baz();\n qux();' // newString has base 2 spaces
153+
const expected = ' foo();\n baz();\n qux();'
154+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
155+
})
156+
157+
test('removes indentation when file has less overall indentation', () => {
158+
const oldString = ' if ok:\n foo();'
159+
const fileMatch = ' if ok:\n foo();' // file has 2 spaces instead of 4
160+
const newString = ' if ok:\n bar();\n baz();' // newString has deeper nest
161+
const expected = ' if ok:\n bar();\n baz();'
162+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
163+
})
164+
165+
test('handles completely different indentation styles (spaces vs tabs)', () => {
166+
const oldString = ' if ok:\n foo();'
167+
const fileMatch = '\tif ok:\n\t\tfoo();'
168+
const newString = ' if ok:\n baz();' // added deeper space indent
169+
const expected = '\tif ok:\n\t\t baz();' // prepends tab prefix and keeps remaining spaces
170+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
171+
})
172+
173+
test('rejects conflicting indentation maps (CodeRabbit P2 fix)', () => {
174+
const oldString = 'if ok:\n foo()\n bar()'
175+
// File actually has bar() outside the block
176+
const fileMatch = 'if ok:\n foo()\nbar()'
177+
const newString = 'if ok:\n baz()\n qux()'
178+
// oldIndent " " maps to " " for foo(), but maps to "" for bar()
179+
// It should detect the conflict and return null
180+
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBeNull()
181+
})
182+
})

0 commit comments

Comments
 (0)