Skip to content

fix: 删除 edit tool 中的旧逻辑处理， 现在已经不需要这些处理了， 大模型够屌 #1251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
claude-code-best merged 2 commits into from
May 28, 2026
Merged

fix: 删除 edit tool 中的旧逻辑处理， 现在已经不需要这些处理了， 大模型够屌 #1251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
124e321
refactor: remove tab/quote normalization from FileEditTool
claude-code-best May 28, 2026
0e39283
fix: resolve pre-existing typecheck errors (zod v4 compat + RCS web e…
claude-code-best May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions packages/builtin-tools/src/tools/FileEditTool/FileEditTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
areFileEditsInputsEquivalent,
findActualString,
getPatchForEdit,
preserveQuoteStyle,
} from './utils.js'

// V8/Bun string length limit is ~2^30 characters (~1 billion). For typical
Expand Down Expand Up @@ -297,7 +296,7 @@ export const FileEditTool = buildTool({

const file = fileContent

// Use findActualString to handle quote normalization
// Use findActualString to find exact match
const actualOldString = findActualString(file, old_string)
if (!actualOldString) {
return {
Expand Down Expand Up @@ -452,23 +451,16 @@ export const FileEditTool = buildTool({
}
}

// 3. Use findActualString to handle quote normalization
// 3. Find the exact string in file content
const actualOldString =
findActualString(originalFileContents, old_string) || old_string

// Preserve curly quotes in new_string when the file uses them
const actualNewString = preserveQuoteStyle(
old_string,
actualOldString,
new_string,
)

// 4. Generate patch
const { patch, updatedFile } = getPatchForEdit({
filePath: absoluteFilePath,
fileContents: originalFileContents,
oldString: actualOldString,
newString: actualNewString,
newString: new_string,
replaceAll: replace_all,
})

Expand Down
5 changes: 2 additions & 3 deletions packages/builtin-tools/src/tools/FileEditTool/UI.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { readEditContext } from 'src/utils/readEditContext.js';
import { firstLineOf } from 'src/utils/stringUtils.js';
import type { ThemeName } from 'src/utils/theme.js';
import type { FileEditOutput } from './types.js';
import { findActualString, getPatchForEdit, preserveQuoteStyle } from './utils.js';
import { findActualString, getPatchForEdit } from './utils.js';

export function userFacingName(
input:
Expand Down Expand Up @@ -265,12 +265,11 @@ async function loadRejectionDiff(
return { patch, firstLine: null, fileContent: undefined };
}
const actualOld = findActualString(ctx.content, oldString) || oldString;
const actualNew = preserveQuoteStyle(oldString, actualOld, newString);
const { patch } = getPatchForEdit({
filePath,
fileContents: ctx.content,
oldString: actualOld,
newString: actualNew,
newString: newString,
replaceAll,
});
return {
Expand Down
160 changes: 3 additions & 157 deletions packages/builtin-tools/src/tools/FileEditTool/__tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,45 +4,8 @@ import { logMock } from '../../../../../../tests/mocks/log'
// Mock log.ts to cut the heavy dependency chain
mock.module('src/utils/log.ts', logMock)

const {
normalizeQuotes,
stripTrailingWhitespace,
findActualString,
preserveQuoteStyle,
applyEditToFile,
LEFT_SINGLE_CURLY_QUOTE,
RIGHT_SINGLE_CURLY_QUOTE,
LEFT_DOUBLE_CURLY_QUOTE,
RIGHT_DOUBLE_CURLY_QUOTE,
} = await import('../utils')

// ─── normalizeQuotes ────────────────────────────────────────────────────

describe('normalizeQuotes', () => {
test('converts left single curly to straight', () => {
expect(normalizeQuotes(`${LEFT_SINGLE_CURLY_QUOTE}hello`)).toBe("'hello")
})

test('converts right single curly to straight', () => {
expect(normalizeQuotes(`hello${RIGHT_SINGLE_CURLY_QUOTE}`)).toBe("hello'")
})

test('converts left double curly to straight', () => {
expect(normalizeQuotes(`${LEFT_DOUBLE_CURLY_QUOTE}hello`)).toBe('"hello')
})

test('converts right double curly to straight', () => {
expect(normalizeQuotes(`hello${RIGHT_DOUBLE_CURLY_QUOTE}`)).toBe('hello"')
})

test('leaves straight quotes unchanged', () => {
expect(normalizeQuotes('\'hello\' "world"')).toBe('\'hello\' "world"')
})

test('handles empty string', () => {
expect(normalizeQuotes('')).toBe('')
})
})
const { stripTrailingWhitespace, findActualString, applyEditToFile } =
await import('../utils')

// ─── stripTrailingWhitespace ────────────────────────────────────────────

Expand Down Expand Up @@ -91,12 +54,6 @@ describe('findActualString', () => {
expect(findActualString('hello world', 'hello')).toBe('hello')
})

test('finds match with curly quotes normalized', () => {
const fileContent = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}`
const result = findActualString(fileContent, '"hello"')
expect(result).not.toBeNull()
})

test('returns null when not found', () => {
expect(findActualString('hello world', 'xyz')).toBeNull()
})
Expand All @@ -107,124 +64,13 @@ describe('findActualString', () => {
expect(result).toBe('')
})

// ── Tab/space normalization (Bug #2 reproduction) ──

test('finds match when search uses spaces but file uses tabs', () => {
// File content uses Tab indentation
const fileContent = '\tif (x) {\n\t\treturn 1;\n\t}'
// User copies from Read output which renders tabs as spaces
const searchWithSpaces = ' if (x) {\n return 1;\n }'
const result = findActualString(fileContent, searchWithSpaces)
expect(result).not.toBeNull()
expect(result).toBe(fileContent)
})

test('finds match when search mixes tabs and spaces inconsistently', () => {
const fileContent = '\tconst x = 1; // comment'
const searchMixed = ' const x = 1; // comment'
const result = findActualString(fileContent, searchMixed)
expect(result).not.toBeNull()
})

test('finds match for single-line tab-to-space mismatch', () => {
const fileContent = '\t\torder_price = NormalizeDouble(ask, digits);'
const searchSpaces = ' order_price = NormalizeDouble(ask, digits);'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
})

// ── CJK / UTF-8 characters (Bug #1 reproduction) ──
// ── CJK / UTF-8 characters ──

test('finds match with CJK characters in content', () => {
const fileContent = 'input int x = 620; // 止盈点数(点) — 32个pip=320点'
const result = findActualString(fileContent, fileContent)
expect(result).toBe(fileContent)
})

test('finds match with CJK characters when tab/space differs', () => {
const fileContent = '\t// 向上突破 → Sell Limit (逆方向做空)'
const searchSpaces = ' // 向上突破 → Sell Limit (逆方向做空)'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
expect(result).toBe(fileContent)
})

// ── Multiline with tabs + CJK (combined Bug #1 + #2) ──

test('finds multiline match with tabs and CJK characters', () => {
const fileContent =
'\tif(effective_dir == BREAKOUT_UP)\n\t\t{\n\t\t\t// 向上突破\n\t\t}'
const searchSpaces =
' if(effective_dir == BREAKOUT_UP)\n {\n // 向上突破\n }'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
expect(result).toBe(fileContent)
})

// ── Returned string must be a valid substring of fileContent ──

test('returned string from tab match is a real substring of fileContent', () => {
const fileContent = 'prefix\n\t\tindented code\nsuffix'
const searchSpaces = 'prefix\n indented code\nsuffix'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
expect(fileContent.includes(result!)).toBe(true)
})

test('returned string from partial tab match is a real substring', () => {
const fileContent = 'line1\n\tif (x) {\n\t\tdoStuff();\n\t}\nline5'
const searchSpaces = ' if (x) {\n doStuff();\n }'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
expect(fileContent.includes(result!)).toBe(true)
})

test('tab match with mixed indentation levels', () => {
const fileContent =
'class Foo {\n\t\tmethod1() {\n\t\t\treturn 42;\n\t\t}\n}'
const searchSpaces =
'class Foo {\n method1() {\n return 42;\n }\n}'
const result = findActualString(fileContent, searchSpaces)
expect(result).not.toBeNull()
expect(fileContent.includes(result!)).toBe(true)
})
})

// ─── preserveQuoteStyle ─────────────────────────────────────────────────

describe('preserveQuoteStyle', () => {
test('returns newString unchanged when no normalization happened', () => {
expect(preserveQuoteStyle('hello', 'hello', 'world')).toBe('world')
})

test('converts straight double quotes to curly in replacement', () => {
const oldString = '"hello"'
const actualOldString = `${LEFT_DOUBLE_CURLY_QUOTE}hello${RIGHT_DOUBLE_CURLY_QUOTE}`
const newString = '"world"'
const result = preserveQuoteStyle(oldString, actualOldString, newString)
expect(result).toContain(LEFT_DOUBLE_CURLY_QUOTE)
expect(result).toContain(RIGHT_DOUBLE_CURLY_QUOTE)
})

test('converts straight single quotes to curly in replacement', () => {
const oldString = "'hello'"
const actualOldString = `${LEFT_SINGLE_CURLY_QUOTE}hello${RIGHT_SINGLE_CURLY_QUOTE}`
const newString = "'world'"
const result = preserveQuoteStyle(oldString, actualOldString, newString)
expect(result).toContain(LEFT_SINGLE_CURLY_QUOTE)
expect(result).toContain(RIGHT_SINGLE_CURLY_QUOTE)
})

test('treats apostrophe in contraction as right curly quote', () => {
const oldString = "'it's a test'"
const actualOldString = `${LEFT_SINGLE_CURLY_QUOTE}it${RIGHT_SINGLE_CURLY_QUOTE}s a test${RIGHT_SINGLE_CURLY_QUOTE}`
const newString = "'don't worry'"
const result = preserveQuoteStyle(oldString, actualOldString, newString)
// The leading ' at position 0 should be LEFT_SINGLE_CURLY_QUOTE
expect(result[0]).toBe(LEFT_SINGLE_CURLY_QUOTE)
// The apostrophe in "don't" (between n and t) should be RIGHT_SINGLE_CURLY_QUOTE
expect(result).toContain(RIGHT_SINGLE_CURLY_QUOTE)
})
})

// ─── applyEditToFile ────────────────────────────────────────────────────
Expand Down
Loading
Loading