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

* refactor: remove tab/quote normalization from FileEditTool

* fix: resolve pre-existing typecheck errors (zod v4 compat + RCS web exclude)
This commit is contained in:
claude-code-best
2026-05-28 21:52:31 +08:00
committed by GitHub
parent c982104476
commit a91653a0dd
14 changed files with 23 additions and 447 deletions

View File

@@ -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
@@ -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 {
@@ -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,
})

View File

@@ -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:
@@ -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 {

View File

@@ -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 ────────────────────────────────────────────
@@ -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()
})
@@ -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 ────────────────────────────────────────────────────

View File

@@ -15,27 +15,6 @@ import {
} from 'src/utils/file.js'
import type { EditInput, FileEdit } from './types.js'
// Claude can't output curly quotes, so we define them as constants here for Claude to use
// in the code. We do this because we normalize curly quotes to straight quotes
// when applying edits.
export const LEFT_SINGLE_CURLY_QUOTE = ''
export const RIGHT_SINGLE_CURLY_QUOTE = ''
export const LEFT_DOUBLE_CURLY_QUOTE = '“'
export const RIGHT_DOUBLE_CURLY_QUOTE = '”'
/**
* Normalizes quotes in a string by converting curly quotes to straight quotes
* @param str The string to normalize
* @returns The string with all curly quotes replaced by straight quotes
*/
export function normalizeQuotes(str: string): string {
return str
.replaceAll(LEFT_SINGLE_CURLY_QUOTE, "'")
.replaceAll(RIGHT_SINGLE_CURLY_QUOTE, "'")
.replaceAll(LEFT_DOUBLE_CURLY_QUOTE, '"')
.replaceAll(RIGHT_DOUBLE_CURLY_QUOTE, '"')
}
/**
* Strips trailing whitespace from each line in a string while preserving line endings
* @param str The string to process
@@ -64,261 +43,22 @@ export function stripTrailingWhitespace(str: string): string {
}
/**
* Normalizes whitespace for fuzzy matching by converting tabs to spaces
* and collapsing leading whitespace on each line to a canonical form.
* This handles the case where Read tool output renders tabs as spaces,
* so users copy spaces from the output but the file actually has tabs.
*/
function normalizeWhitespace(str: string): string {
return str.replace(/\t/g, ' ')
}
/**
* Finds the actual string in the file content that matches the search string,
* accounting for quote normalization and tab/space differences.
*
* Matching cascade:
* 1. Exact match
* 2. Quote normalization (curly → straight quotes)
* 3. Tab/space normalization (tabs ↔ spaces in leading whitespace)
* 4. Quote + tab/space normalization combined
* Finds the exact string in the file content.
*
* @param fileContent The file content to search in
* @param searchString The string to search for
* @returns The actual string found in the file, or null if not found
* @returns The search string if found, or null if not found
*/
export function findActualString(
fileContent: string,
searchString: string,
): string | null {
// First try exact match
if (fileContent.includes(searchString)) {
return searchString
}
// Try with normalized quotes
const normalizedSearch = normalizeQuotes(searchString)
const normalizedFile = normalizeQuotes(fileContent)
const searchIndex = normalizedFile.indexOf(normalizedSearch)
if (searchIndex !== -1) {
// Find the actual string in the file that matches
return fileContent.substring(searchIndex, searchIndex + searchString.length)
}
// Try with tab/space normalization — handles the case where Read output
// renders tabs as spaces and the user copies the rendered version
const wsNormalizedFile = normalizeWhitespace(fileContent)
const wsNormalizedSearch = normalizeWhitespace(searchString)
const wsSearchIndex = wsNormalizedFile.indexOf(wsNormalizedSearch)
if (wsSearchIndex !== -1) {
// Map the match position back to the original file content.
// We need to find the corresponding range in the original string.
return mapNormalizedMatchBackToFile(
fileContent,
wsNormalizedFile,
wsSearchIndex,
wsNormalizedSearch.length,
)
}
// Try combined: quote normalization + tab/space normalization
const combinedFile = normalizeWhitespace(normalizedFile)
const combinedSearch = normalizeWhitespace(normalizedSearch)
const combinedIndex = combinedFile.indexOf(combinedSearch)
if (combinedIndex !== -1) {
return mapNormalizedMatchBackToFile(
fileContent,
combinedFile,
combinedIndex,
combinedSearch.length,
)
}
return null
}
/**
* Given a match found in a normalized version of fileContent, map the match
* position back to the original fileContent and extract the corresponding
* substring.
*
* Strategy: walk through both strings character by character, building a
* mapping from normalized offset to original offset. When a tab is expanded
* to 4 spaces in the normalized version, the normalized offset advances by 4
* while the original offset advances by 1.
*/
function mapNormalizedMatchBackToFile(
fileContent: string,
normalizedFile: string,
normalizedStart: number,
normalizedLength: number,
): string {
// Build a sparse mapping from normalized position → original position.
// We only need to map the range [normalizedStart, normalizedStart + normalizedLength].
let normPos = 0
let origPos = 0
let origStart = -1
let origEnd = -1
while (
origPos < fileContent.length &&
normPos <= normalizedStart + normalizedLength
) {
if (normPos === normalizedStart) {
origStart = origPos
}
if (normPos === normalizedStart + normalizedLength) {
origEnd = origPos
break
}
const origChar = fileContent[origPos]!
if (origChar === '\t') {
// Tab expands to 4 spaces in normalized version
const nextNormPos = normPos + 4
// If normalizedStart falls within this expanded tab, snap to origPos
if (
normPos < normalizedStart &&
nextNormPos > normalizedStart &&
origStart === -1
) {
origStart = origPos
}
if (
normPos < normalizedStart + normalizedLength &&
nextNormPos > normalizedStart + normalizedLength &&
origEnd === -1
) {
origEnd = origPos + 1
}
normPos = nextNormPos
origPos++
} else {
normPos++
origPos++
}
}
// Fallback: if we couldn't map precisely, use character-count heuristic
if (origStart === -1) origStart = 0
if (origEnd === -1) {
// Approximate: use the ratio of original to normalized length
const ratio = fileContent.length / normalizedFile.length
origEnd = Math.round(origStart + normalizedLength * ratio)
}
return fileContent.substring(origStart, origEnd)
}
/**
* When old_string matched via quote normalization (curly quotes in file,
* straight quotes from model), apply the same curly quote style to new_string
* so the edit preserves the file's typography.
*
* Uses a simple open/close heuristic: a quote character preceded by whitespace,
* start of string, or opening punctuation is treated as an opening quote;
* otherwise it's a closing quote.
*/
export function preserveQuoteStyle(
oldString: string,
actualOldString: string,
newString: string,
): string {
// If they're the same, no normalization happened
if (oldString === actualOldString) {
return newString
}
// Detect which curly quote types were in the file
const hasDoubleQuotes =
actualOldString.includes(LEFT_DOUBLE_CURLY_QUOTE) ||
actualOldString.includes(RIGHT_DOUBLE_CURLY_QUOTE)
const hasSingleQuotes =
actualOldString.includes(LEFT_SINGLE_CURLY_QUOTE) ||
actualOldString.includes(RIGHT_SINGLE_CURLY_QUOTE)
if (!hasDoubleQuotes && !hasSingleQuotes) {
return newString
}
let result = newString
if (hasDoubleQuotes) {
result = applyCurlyDoubleQuotes(result)
}
if (hasSingleQuotes) {
result = applyCurlySingleQuotes(result)
}
return result
}
function isOpeningContext(chars: string[], index: number): boolean {
if (index === 0) {
return true
}
const prev = chars[index - 1]
return (
prev === ' ' ||
prev === '\t' ||
prev === '\n' ||
prev === '\r' ||
prev === '(' ||
prev === '[' ||
prev === '{' ||
prev === '\u2014' || // em dash
prev === '\u2013' // en dash
)
}
function applyCurlyDoubleQuotes(str: string): string {
const chars = [...str]
const result: string[] = []
for (let i = 0; i < chars.length; i++) {
if (chars[i] === '"') {
result.push(
isOpeningContext(chars, i)
? LEFT_DOUBLE_CURLY_QUOTE
: RIGHT_DOUBLE_CURLY_QUOTE,
)
} else {
result.push(chars[i]!)
}
}
return result.join('')
}
function applyCurlySingleQuotes(str: string): string {
const chars = [...str]
const result: string[] = []
for (let i = 0; i < chars.length; i++) {
if (chars[i] === "'") {
// Don't convert apostrophes in contractions (e.g., "don't", "it's")
// An apostrophe between two letters is a contraction, not a quote
const prev = i > 0 ? chars[i - 1] : undefined
const next = i < chars.length - 1 ? chars[i + 1] : undefined
const prevIsLetter = prev !== undefined && /\p{L}/u.test(prev)
const nextIsLetter = next !== undefined && /\p{L}/u.test(next)
if (prevIsLetter && nextIsLetter) {
// Apostrophe in a contraction — use right single curly quote
result.push(RIGHT_SINGLE_CURLY_QUOTE)
} else {
result.push(
isOpeningContext(chars, i)
? LEFT_SINGLE_CURLY_QUOTE
: RIGHT_SINGLE_CURLY_QUOTE,
)
}
} else {
result.push(chars[i]!)
}
}
return result.join('')
}
/**
* Transform edits to ensure replace_all always has a boolean value
* @param edits Array of edits with optional replace_all

View File

@@ -4966,7 +4966,7 @@ function handleChannelEnable(
// channel messages queue at priority 'next' and are seen by the model on
// the turn after they arrive.
connection.client.setNotificationHandler(
ChannelMessageNotificationSchema(),
ChannelMessageNotificationSchema() as any,
async notification => {
const { content, meta } = notification.params
logMCPDebug(
@@ -5042,7 +5042,7 @@ function reregisterChannelHandlerAfterReconnect(
'Channel notifications re-registered after reconnect',
)
connection.client.setNotificationHandler(
ChannelMessageNotificationSchema(),
ChannelMessageNotificationSchema() as any,
async notification => {
const { content, meta } = notification.params
logMCPDebug(

View File

@@ -4,7 +4,7 @@ import { Suspense, use, useState } from 'react';
import { useTerminalSize } from '../hooks/useTerminalSize.js';
import { Box, Text } from '@anthropic/ink';
import type { FileEdit } from '@claude-code-best/builtin-tools/tools/FileEditTool/types.js';
import { findActualString, preserveQuoteStyle } from '@claude-code-best/builtin-tools/tools/FileEditTool/utils.js';
import { findActualString } from '@claude-code-best/builtin-tools/tools/FileEditTool/utils.js';
import { adjustHunkLineNumbers, CONTEXT_LINES, getPatchForDisplay } from '../utils/diff.js';
import { logError } from '../utils/log.js';
import { CHUNK_SIZE, openForScan, readCapped, scanForContext } from '../utils/readEditContext.js';
@@ -135,6 +135,5 @@ function diffToolInputsOnly(filePath: string, edits: FileEdit[]): DiffData {
function normalizeEdit(fileContent: string, edit: FileEdit): FileEdit {
const actualOld = findActualString(fileContent, edit.old_string) || edit.old_string;
const actualNew = preserveQuoteStyle(edit.old_string, actualOld, edit.new_string);
return { ...edit, old_string: actualOld, new_string: actualNew };
return { ...edit, old_string: actualOld };
}

View File

@@ -146,7 +146,7 @@ async function main(): Promise<void> {
shutdown1PEventLogging,
logForDebugging,
registerPermissionHandler(server, handler) {
server.setNotificationHandler(ChannelPermissionRequestNotificationSchema(), async notification =>
server.setNotificationHandler(ChannelPermissionRequestNotificationSchema() as any, async notification =>
handler(notification.params),
);
},

View File

@@ -47,7 +47,7 @@ export function useIdeAtMentioned(
// If we found a connected IDE client, register our handler
if (ideClient) {
ideClient.client.setNotificationHandler(
AtMentionedSchema(),
AtMentionedSchema() as any,
notification => {
if (ideClientRef.current !== ideClient) {
return

View File

@@ -27,7 +27,7 @@ export function useIdeLogging(mcpClients: MCPServerConnection[]): void {
if (ideClient) {
// Register the log event handler
ideClient.client.setNotificationHandler(
LogEventSchema(),
LogEventSchema() as any,
notification => {
const { eventName, eventData } = notification.params
logEvent(

View File

@@ -110,7 +110,7 @@ export function useIdeSelection(
// Register notification handler for selection_changed events
ideClient.client.setNotificationHandler(
SelectionChangedSchema(),
SelectionChangedSchema() as any,
notification => {
if (currentIDERef.current !== ideClient) {
return

View File

@@ -48,7 +48,7 @@ export function usePromptsFromClaudeInChrome(
}
if (mcpClient) {
mcpClient.client.setNotificationHandler(ClaudeInChromePromptNotificationSchema(), notification => {
mcpClient.client.setNotificationHandler(ClaudeInChromePromptNotificationSchema() as any, notification => {
if (mcpClientRef.current !== mcpClient) {
return;
}

View File

@@ -504,7 +504,7 @@ export function useManageMCPConnections(
case 'register':
logMCPDebug(client.name, 'Channel notifications registered')
client.client.setNotificationHandler(
ChannelMessageNotificationSchema(),
ChannelMessageNotificationSchema() as any,
async notification => {
const { content, meta } = notification.params
logMCPDebug(
@@ -539,7 +539,7 @@ export function useManageMCPConnections(
client.capabilities?.experimental?.['claude/channel/permission']
) {
client.client.setNotificationHandler(
ChannelPermissionNotificationSchema(),
ChannelPermissionNotificationSchema() as any,
async notification => {
const { request_id, behavior } = notification.params
const resolved =

View File

@@ -69,7 +69,7 @@ export function setupVscodeSdkMcp(sdkClients: MCPServerConnection[]): void {
vscodeMcpClient = client
client.client.setNotificationHandler(
LogEventNotificationSchema(),
LogEventNotificationSchema() as any,
async notification => {
const { eventName, eventData } = notification.params
logEvent(

View File

@@ -32,5 +32,5 @@
"packages/**/*.ts",
"packages/**/*.tsx"
],
"exclude": ["node_modules"]
"exclude": ["node_modules", "packages/remote-control-server/web"]
}