mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 10:14:19 -05:00
Feat/fuzzy-file-edit (#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
This commit is contained in:
3kin0x
@@ -0,0 +1,182 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { findWhitespaceAgnosticMatch, adjustNewStringIndentation } from './utils.js'
|
||||
|
||||
describe('findWhitespaceAgnosticMatch', () => {
|
||||
test('returns exact match for simple string', () => {
|
||||
const fileContent = 'const x = 1;\nconst y = 2;'
|
||||
const searchString = 'const x = 1;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('const x = 1;')
|
||||
})
|
||||
|
||||
test('handles missing trailing newlines', () => {
|
||||
const fileContent = 'function hello() {\n console.log("world");\n}\n'
|
||||
const searchString = 'function hello() {\n console.log("world");\n}'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('function hello() {\n console.log("world");\n}')
|
||||
})
|
||||
|
||||
test('handles indentation changes', () => {
|
||||
const fileContent = 'function hello() {\n console.log("world");\n}'
|
||||
const searchString = 'function hello() {\n console.log("world");\n}'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe('function hello() {\n console.log("world");\n}')
|
||||
})
|
||||
|
||||
test('rejects inline space changes to protect tokenization and operators', () => {
|
||||
const fileContent = 'if ( a === b ) { return c; }'
|
||||
const searchString = 'if(a===b){return c;}'
|
||||
// Inline space differences are now strictly rejected to prevent merging/splitting tokens
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('prevents operator token collapsing across fuzzy matches', () => {
|
||||
const fileContent = 'const z = i++ + j;'
|
||||
const searchString = 'const z = i + ++j;'
|
||||
// If inline spaces are ignored, both become i+++j, which would be a dangerous match.
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('recovers leading boundary horizontal whitespace without consuming line breaks', () => {
|
||||
const fileContent = 'function hello() {\n foo();\n}'
|
||||
const searchString = ' foo();' // Agent provided leading spaces
|
||||
// Leading spaces are ignored in the match, and boundary expansion
|
||||
// recovers the exact file indentation. The `\n` is safely preserved!
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBe(' foo();')
|
||||
})
|
||||
|
||||
test('prevents trailing-newline searches from consuming next line indentation', () => {
|
||||
const fileContent = 'if ok:\n foo()\n bar()\n'
|
||||
const searchString = ' foo()\n'
|
||||
const actualOldString = findWhitespaceAgnosticMatch(fileContent, searchString)
|
||||
expect(actualOldString).toBe(' foo()\n')
|
||||
})
|
||||
|
||||
test('rejects fuzzy match when LLM collapses blank lines (CodeRabbit P2 fix)', () => {
|
||||
const fileContent = 'A paragraph.\n\nNext paragraph.'
|
||||
const searchString = 'A paragraph.\nNext paragraph.'
|
||||
// The exact newline count mismatch forces it to reject the fuzzy match.
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('rejects fuzzy match when LLM hallucinates blank lines (CodeRabbit P2 fix)', () => {
|
||||
const fileContent = 'A paragraph.\nNext paragraph.'
|
||||
const searchString = 'A paragraph.\n\nNext paragraph.'
|
||||
// The exact newline count mismatch forces it to reject the fuzzy match.
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('preserves Markdown hard breaks in fuzzy match (CodeRabbit P2 fix)', () => {
|
||||
const fileContent = 'foo \nbar'
|
||||
const searchString = 'foo\nbar'
|
||||
// isMarkdown = true protects trailing spaces before a newline
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString, true)).toBeNull()
|
||||
})
|
||||
|
||||
test('ignores trailing garbage spaces for non-Markdown files', () => {
|
||||
const fileContent = 'foo \nbar'
|
||||
const searchString = 'foo\nbar'
|
||||
// isMarkdown = false drops trailing spaces to be agnostic
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString, false)).toBe('foo \nbar')
|
||||
})
|
||||
|
||||
test('keeps inline whitespace exact to protect semantics (CodeRabbit P2 fix)', () => {
|
||||
const fileContent = 'const msg = "hello world";'
|
||||
const searchString = 'const msg = "hello world";'
|
||||
// The inline spaces do not match, so it rejects it!
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('prevents matching across token boundaries', () => {
|
||||
// LLM forgot the space between two tokens
|
||||
const fileContent = 'const foobar = 1;'
|
||||
const searchString = 'const foo bar = 1;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
|
||||
// LLM inserted a space inside a token
|
||||
const fileContent2 = 'const foo bar = 1;'
|
||||
const searchString2 = 'const foobar = 1;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent2, searchString2)).toBeNull()
|
||||
})
|
||||
|
||||
test('returns null if no match found', () => {
|
||||
const fileContent = 'const a = 1;'
|
||||
const searchString = 'const b = 2;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('returns null if multiple matches found to prevent accidental replacement', () => {
|
||||
const fileContent = 'const a = 1;\nconst a = 1;'
|
||||
const searchString = 'const a = 1;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
})
|
||||
|
||||
test('prevents multiline strings from matching single-line strings with same tokens', () => {
|
||||
// P1: A newline in the search string should not match an inline space in the file
|
||||
const fileContent = 'const x = a + b;'
|
||||
const searchString = 'const x = a\n + b;'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent, searchString)).toBeNull()
|
||||
|
||||
const fileContent2 = '.foo .bar { color: red; }'
|
||||
const searchString2 = '.foo\n .bar { color: red; }'
|
||||
expect(findWhitespaceAgnosticMatch(fileContent2, searchString2)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('adjustNewStringIndentation', () => {
|
||||
test('returns newString unmodified if oldString and fileMatch have same indentation', () => {
|
||||
const oldString = ' foo();\n bar();'
|
||||
const fileMatch = ' foo();\n bar();'
|
||||
const newString = ' foo();\n baz();'
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(newString)
|
||||
})
|
||||
|
||||
test('recovers nested structure when root has no indentation (CodeRabbit P2 fix)', () => {
|
||||
const oldString = 'if ok:\n foo()'
|
||||
const fileMatch = 'if ok:\n foo()' // file uses 4 spaces instead of 2 for nested line
|
||||
const newString = 'if ok:\n bar()'
|
||||
// It should preserve the nested 4 spaces for bar() even though the root `if ok:` is 0 spaces
|
||||
const expected = 'if ok:\n bar()'
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
|
||||
})
|
||||
|
||||
test('handles deeper unseen relative indentation intelligently', () => {
|
||||
const oldString = 'if ok:\n foo()'
|
||||
const fileMatch = 'if ok:\n foo()'
|
||||
const newString = 'if ok:\n for x in y:\n bar()' // LLM added a deeper block at 4 spaces
|
||||
// It should map 0 -> 0, 2 -> 4, and 4 -> 4 + 2 remaining = 6
|
||||
const expected = 'if ok:\n for x in y:\n bar()'
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
|
||||
})
|
||||
|
||||
test('adds indentation when file has more overall indentation', () => {
|
||||
const oldString = ' foo();\n bar();'
|
||||
const fileMatch = ' foo();\n bar();' // file has +2 spaces
|
||||
const newString = ' foo();\n baz();\n qux();' // newString has base 2 spaces
|
||||
const expected = ' foo();\n baz();\n qux();'
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
|
||||
})
|
||||
|
||||
test('removes indentation when file has less overall indentation', () => {
|
||||
const oldString = ' if ok:\n foo();'
|
||||
const fileMatch = ' if ok:\n foo();' // file has 2 spaces instead of 4
|
||||
const newString = ' if ok:\n bar();\n baz();' // newString has deeper nest
|
||||
const expected = ' if ok:\n bar();\n baz();'
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
|
||||
})
|
||||
|
||||
test('handles completely different indentation styles (spaces vs tabs)', () => {
|
||||
const oldString = ' if ok:\n foo();'
|
||||
const fileMatch = '\tif ok:\n\t\tfoo();'
|
||||
const newString = ' if ok:\n baz();' // added deeper space indent
|
||||
const expected = '\tif ok:\n\t\t baz();' // prepends tab prefix and keeps remaining spaces
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBe(expected)
|
||||
})
|
||||
|
||||
test('rejects conflicting indentation maps (CodeRabbit P2 fix)', () => {
|
||||
const oldString = 'if ok:\n foo()\n bar()'
|
||||
// File actually has bar() outside the block
|
||||
const fileMatch = 'if ok:\n foo()\nbar()'
|
||||
const newString = 'if ok:\n baz()\n qux()'
|
||||
// oldIndent " " maps to " " for foo(), but maps to "" for bar()
|
||||
// It should detect the conflict and return null
|
||||
expect(adjustNewStringIndentation(oldString, fileMatch, newString)).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -638,6 +638,35 @@ export function normalizeFileEditInput({
|
||||
}
|
||||
}
|
||||
|
||||
// Fallback to whitespace-agnostic match
|
||||
const fuzzyMatch = findWhitespaceAgnosticMatch(
|
||||
fileContent,
|
||||
desanitizedOldString,
|
||||
isMarkdown,
|
||||
)
|
||||
|
||||
if (fuzzyMatch) {
|
||||
// Fix P2: Apply the recovered indentation from the file to the new_string
|
||||
let adjustedNewString = adjustNewStringIndentation(
|
||||
desanitizedOldString,
|
||||
fuzzyMatch,
|
||||
normalizedNewString,
|
||||
)
|
||||
|
||||
if (adjustedNewString !== null) {
|
||||
// Apply the same exact replacements to new_string
|
||||
for (const { from, to } of appliedReplacements) {
|
||||
adjustedNewString = adjustedNewString.replaceAll(from, to)
|
||||
}
|
||||
|
||||
return {
|
||||
old_string: fuzzyMatch,
|
||||
new_string: adjustedNewString,
|
||||
replace_all,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
old_string,
|
||||
new_string: normalizedNewString,
|
||||
@@ -773,3 +802,239 @@ export function areFileEditsInputsEquivalent(
|
||||
|
||||
return areFileEditsEquivalent(input1.edits, input2.edits, fileContent)
|
||||
}
|
||||
|
||||
/**
|
||||
* Adjusts the absolute indentation of `newString` based on the difference
|
||||
* between the base indentation of `oldString` and the actual `fileMatch`.
|
||||
* Returns null if the indentation mapping is conflicting (e.g. LLM merged blocks).
|
||||
*/
|
||||
export function adjustNewStringIndentation(
|
||||
oldString: string,
|
||||
fileMatch: string,
|
||||
newString: string,
|
||||
): string | null {
|
||||
// If no formatting difference, no adjustment needed
|
||||
if (oldString === fileMatch) return newString
|
||||
|
||||
// Tokenize both strings to build a mapping from oldString characters to fileMatch characters.
|
||||
const oldNorm = normalizeIndentation(oldString, false)
|
||||
const actualNorm = normalizeIndentation(fileMatch, false)
|
||||
|
||||
// Find where the normalized forms align
|
||||
const matchIndex = actualNorm.normalized.indexOf(oldNorm.normalized)
|
||||
if (matchIndex === -1) {
|
||||
// Should not happen since fileMatch was derived from oldString, but fallback to safety
|
||||
return newString
|
||||
}
|
||||
|
||||
// Build the indent map mapping from hallucinated indent (oldIndent) to true indent (actualIndent)
|
||||
const indentMap = new Map<string, string>()
|
||||
const oldLines = oldString.split('\n')
|
||||
let oldCharIndex = 0
|
||||
|
||||
for (let i = 0; i < oldLines.length; i++) {
|
||||
const line = oldLines[i]!
|
||||
const match = line.match(/^[ \t]*/)
|
||||
const oldIndent = match ? match[0] : ''
|
||||
|
||||
// Find the first non-whitespace character in this line
|
||||
const nonWsMatch = line.match(/\S/)
|
||||
if (nonWsMatch) {
|
||||
const nonWsIndexInLine = nonWsMatch.index!
|
||||
const nonWsIndexInOldString = oldCharIndex + nonWsIndexInLine
|
||||
|
||||
// Map this character to actualNorm index
|
||||
let normIndex = -1
|
||||
for (let k = 0; k < oldNorm.mapping.length; k++) {
|
||||
if (oldNorm.mapping[k] === nonWsIndexInOldString) {
|
||||
normIndex = k
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if (normIndex !== -1) {
|
||||
const actualNormIndex = matchIndex + normIndex
|
||||
if (actualNormIndex < actualNorm.mapping.length) {
|
||||
const actualCharIndex = actualNorm.mapping[actualNormIndex]!
|
||||
|
||||
// Find the leading whitespace of the line containing `actualCharIndex` in `fileMatch`
|
||||
let startOfLine = actualCharIndex
|
||||
while (startOfLine > 0 && fileMatch[startOfLine - 1] !== '\n') {
|
||||
startOfLine--
|
||||
}
|
||||
|
||||
let actualIndent = ''
|
||||
for (let k = startOfLine; k < actualCharIndex; k++) {
|
||||
if (fileMatch[k] === ' ' || fileMatch[k] === '\t') {
|
||||
actualIndent += fileMatch[k]
|
||||
} else {
|
||||
break // Should not happen if it's truly the first non-ws char
|
||||
}
|
||||
}
|
||||
|
||||
const existingIndent = indentMap.get(oldIndent)
|
||||
if (existingIndent !== undefined && existingIndent !== actualIndent) {
|
||||
// CodeRabbit P2 fix: Conflicting indentation map.
|
||||
// The same hallucinated indentation corresponds to different actual indentations in the file.
|
||||
// This means the LLM merged lines from different structural blocks.
|
||||
// We must reject the match to prevent unsafe re-indentation.
|
||||
return null
|
||||
}
|
||||
|
||||
indentMap.set(oldIndent, actualIndent)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
oldCharIndex += line.length + 1 // +1 for the '\n'
|
||||
}
|
||||
|
||||
// If there's no mapping (e.g. empty strings), return newString
|
||||
if (indentMap.size === 0) return newString
|
||||
|
||||
// Apply the indent map to newString
|
||||
const newLines = newString.split('\n')
|
||||
const adjustedLines = newLines.map(line => {
|
||||
// Ignore completely empty lines
|
||||
if (line.trim() === '') return line
|
||||
|
||||
const match = line.match(/^[ \t]*/)
|
||||
const newIndent = match ? match[0] : ''
|
||||
|
||||
if (indentMap.has(newIndent)) {
|
||||
return indentMap.get(newIndent) + line.slice(newIndent.length)
|
||||
}
|
||||
|
||||
// If not found (e.g. LLM introduced a new deeper nesting level),
|
||||
// find the longest known prefix and append the remaining relative whitespace.
|
||||
let longestPrefix = ''
|
||||
let mappedPrefix = ''
|
||||
for (const [oldInd, actualInd] of indentMap.entries()) {
|
||||
if (
|
||||
newIndent.startsWith(oldInd) &&
|
||||
oldInd.length > longestPrefix.length
|
||||
) {
|
||||
longestPrefix = oldInd
|
||||
mappedPrefix = actualInd
|
||||
}
|
||||
}
|
||||
|
||||
if (longestPrefix !== '') {
|
||||
const remainingIndent = newIndent.slice(longestPrefix.length)
|
||||
return mappedPrefix + remainingIndent + line.slice(newIndent.length)
|
||||
}
|
||||
|
||||
return line // Fallback
|
||||
})
|
||||
|
||||
return adjustedLines.join('\n')
|
||||
}
|
||||
|
||||
function normalizeIndentation(str: string, isMarkdown: boolean) {
|
||||
let normalized = ''
|
||||
const mapping: number[] = []
|
||||
|
||||
let i = 0
|
||||
while (i < str.length) {
|
||||
if (str[i] === '\n' || str[i] === '\r') {
|
||||
normalized += str[i]
|
||||
mapping.push(i)
|
||||
i++
|
||||
} else if (/[ \t]/.test(str[i]!)) {
|
||||
const startWs = i
|
||||
while (i < str.length && /[ \t]/.test(str[i]!)) {
|
||||
i++
|
||||
}
|
||||
|
||||
const isLeading = startWs === 0 || str[startWs - 1] === '\n' || str[startWs - 1] === '\r'
|
||||
const isTrailing = i === str.length || str[i] === '\n' || str[i] === '\r'
|
||||
|
||||
if (isLeading) {
|
||||
// Drop leading indentation entirely. The boundary logic will recover the exact original indentation.
|
||||
} else if (isTrailing && !isMarkdown) {
|
||||
// Drop trailing whitespace entirely for non-markdown files to stay agnostic to garbage spaces.
|
||||
} else {
|
||||
// P2 Fix: Keep inline whitespace (and Markdown trailing hard breaks) exactly as is
|
||||
// to protect string literals, regexes, and semantic Markdown breaks.
|
||||
for (let k = startWs; k < i; k++) {
|
||||
normalized += str[k]
|
||||
mapping.push(k)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
normalized += str[i]
|
||||
mapping.push(i)
|
||||
i++
|
||||
}
|
||||
}
|
||||
|
||||
return { normalized, mapping }
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds a substring within fileContent that matches searchString, ignoring formatting differences
|
||||
* by ignoring leading and trailing spaces, while strictly preserving
|
||||
* inline spaces to prevent token boundary corruption (like merging operators or words).
|
||||
* If exactly one match is found, returns the exact substring from fileContent.
|
||||
*/
|
||||
export function findWhitespaceAgnosticMatch(
|
||||
fileContent: string,
|
||||
searchString: string,
|
||||
isMarkdown: boolean = false,
|
||||
): string | null {
|
||||
const search = normalizeIndentation(searchString, isMarkdown)
|
||||
if (search.normalized.trim().length === 0) return null
|
||||
|
||||
const file = normalizeIndentation(fileContent, isMarkdown)
|
||||
|
||||
const matchIndex = file.normalized.indexOf(search.normalized)
|
||||
if (matchIndex === -1) return null
|
||||
|
||||
// Ensure the match is unique to avoid replacing the wrong block
|
||||
const nextMatchIndex = file.normalized.indexOf(
|
||||
search.normalized,
|
||||
matchIndex + 1,
|
||||
)
|
||||
if (nextMatchIndex !== -1) {
|
||||
return null
|
||||
}
|
||||
|
||||
const originalStart = file.mapping[matchIndex]
|
||||
const originalEnd = file.mapping[matchIndex + search.normalized.length - 1]
|
||||
|
||||
if (originalStart === undefined || originalEnd === undefined) return null
|
||||
|
||||
let start = originalStart
|
||||
let end = originalEnd
|
||||
|
||||
// If caller included boundary whitespace, keep equivalent boundary whitespace
|
||||
// from the file so replacement does not duplicate/misplace indentation.
|
||||
if (/^[ \t]/.test(searchString)) {
|
||||
while (start > 0 && /[ \t]/.test(fileContent[start - 1]!)) start--
|
||||
} else if (/^\s/.test(searchString)) {
|
||||
while (start > 0 && /\s/.test(fileContent[start - 1]!)) start--
|
||||
}
|
||||
|
||||
if (/(?:\r?\n)$/.test(searchString)) {
|
||||
// P1 fix: If the search string ends perfectly with a newline,
|
||||
// do NOT consume the indentation of the NEXT line.
|
||||
// The mapped originalEnd might point to the first space of the next line.
|
||||
// Pull it back to the newline character.
|
||||
while (end > start && /[ \t]/.test(fileContent[end]!)) {
|
||||
end--
|
||||
}
|
||||
} else if (/[ \t]$/.test(searchString)) {
|
||||
while (
|
||||
end + 1 < fileContent.length &&
|
||||
/[ \t]/.test(fileContent[end + 1]!)
|
||||
) {
|
||||
end++
|
||||
}
|
||||
} else if (/\s$/.test(searchString)) {
|
||||
while (end + 1 < fileContent.length && /\s/.test(fileContent[end + 1]!)) {
|
||||
end++
|
||||
}
|
||||
}
|
||||
|
||||
return fileContent.substring(start, end + 1)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user