mirror of
https://github.com/zhayujie/chatgpt-on-wechat.git
synced 2026-07-17 11:07:11 +08:00
refactor(edit): unify fuzzy uniqueness check with the fuzzy matcher
The uniqueness guard counted occurrences via normalize_for_fuzzy_match, while fuzzy_find_text located matches with a whitespace-flexible regex, so the two could disagree. Extract the pattern builder as a single source of truth (_build_fuzzy_pattern) and add count_matches, which counts with the same exact-then-fuzzy strategy used to locate and replace. This is the optional follow-up suggested in the review of #2942. Adds regression tests for exact and fuzzy multi-match rejection.
This commit is contained in:
@@ -13,7 +13,7 @@ from agent.tools.utils.diff import (
|
|||||||
detect_line_ending,
|
detect_line_ending,
|
||||||
normalize_to_lf,
|
normalize_to_lf,
|
||||||
restore_line_endings,
|
restore_line_endings,
|
||||||
normalize_for_fuzzy_match,
|
count_matches,
|
||||||
fuzzy_find_text,
|
fuzzy_find_text,
|
||||||
generate_diff_string
|
generate_diff_string
|
||||||
)
|
)
|
||||||
@@ -110,10 +110,10 @@ class Edit(BaseTool):
|
|||||||
"The old text must match exactly including all whitespace and newlines."
|
"The old text must match exactly including all whitespace and newlines."
|
||||||
)
|
)
|
||||||
|
|
||||||
# Calculate occurrence count (use fuzzy normalized content for consistency)
|
# Count occurrences with the same matcher used to locate and
|
||||||
fuzzy_content = normalize_for_fuzzy_match(normalized_content)
|
# replace (fuzzy_find_text), so the uniqueness guard cannot
|
||||||
fuzzy_old_text = normalize_for_fuzzy_match(normalized_old_text)
|
# disagree with what actually gets replaced.
|
||||||
occurrences = fuzzy_content.count(fuzzy_old_text)
|
occurrences = count_matches(normalized_content, normalized_old_text)
|
||||||
|
|
||||||
if occurrences > 1:
|
if occurrences > 1:
|
||||||
return ToolResult.fail(
|
return ToolResult.fail(
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ from .diff import (
|
|||||||
normalize_to_lf,
|
normalize_to_lf,
|
||||||
restore_line_endings,
|
restore_line_endings,
|
||||||
normalize_for_fuzzy_match,
|
normalize_for_fuzzy_match,
|
||||||
|
count_matches,
|
||||||
fuzzy_find_text,
|
fuzzy_find_text,
|
||||||
generate_diff_string,
|
generate_diff_string,
|
||||||
FuzzyMatchResult
|
FuzzyMatchResult
|
||||||
@@ -39,6 +40,7 @@ __all__ = [
|
|||||||
'normalize_to_lf',
|
'normalize_to_lf',
|
||||||
'restore_line_endings',
|
'restore_line_endings',
|
||||||
'normalize_for_fuzzy_match',
|
'normalize_for_fuzzy_match',
|
||||||
|
'count_matches',
|
||||||
'fuzzy_find_text',
|
'fuzzy_find_text',
|
||||||
'generate_diff_string',
|
'generate_diff_string',
|
||||||
'FuzzyMatchResult',
|
'FuzzyMatchResult',
|
||||||
|
|||||||
@@ -93,6 +93,40 @@ class FuzzyMatchResult:
|
|||||||
self.content_for_replacement = content_for_replacement
|
self.content_for_replacement = content_for_replacement
|
||||||
|
|
||||||
|
|
||||||
|
def _build_fuzzy_pattern(old_text: str) -> Optional[str]:
|
||||||
|
"""
|
||||||
|
Build the whitespace-flexible regex used to locate ``old_text`` fuzzily.
|
||||||
|
|
||||||
|
Returns ``None`` when ``old_text`` has no non-whitespace content to match.
|
||||||
|
This is the single source of truth for fuzzy matching, so that *finding* a
|
||||||
|
match (:func:`fuzzy_find_text`) and *counting* occurrences
|
||||||
|
(:func:`count_matches`) always use the exact same rules.
|
||||||
|
"""
|
||||||
|
stripped = old_text.strip('\n')
|
||||||
|
if not stripped.strip():
|
||||||
|
return None
|
||||||
|
|
||||||
|
source_lines = stripped.split('\n')
|
||||||
|
line_patterns = []
|
||||||
|
for i, line in enumerate(source_lines):
|
||||||
|
tokens = line.split()
|
||||||
|
if not tokens:
|
||||||
|
line_patterns.append(r'[ \t]*')
|
||||||
|
continue
|
||||||
|
# Tolerate any run of blanks between tokens.
|
||||||
|
core = r'[ \t]+'.join(re.escape(tok) for tok in tokens)
|
||||||
|
# First-line leading whitespace is folded into the match only when
|
||||||
|
# old_text itself was indented here; otherwise it stays OUTSIDE the
|
||||||
|
# match so a no-indent old_text preserves (does not swallow and drop)
|
||||||
|
# the file's existing indentation -- mirroring an exact substring
|
||||||
|
# match. Inner lines always tolerate indentation: it sits inside the
|
||||||
|
# matched region and is re-supplied by new_text.
|
||||||
|
if i > 0 or line[:1] in (' ', '\t'):
|
||||||
|
core = r'[ \t]*' + core
|
||||||
|
line_patterns.append(core + r'[ \t]*')
|
||||||
|
return '\n'.join(line_patterns)
|
||||||
|
|
||||||
|
|
||||||
def fuzzy_find_text(content: str, old_text: str) -> FuzzyMatchResult:
|
def fuzzy_find_text(content: str, old_text: str) -> FuzzyMatchResult:
|
||||||
"""
|
"""
|
||||||
Find text in content, try exact match first, then fuzzy match
|
Find text in content, try exact match first, then fuzzy match
|
||||||
@@ -121,27 +155,8 @@ def fuzzy_find_text(content: str, old_text: str) -> FuzzyMatchResult:
|
|||||||
# doing so previously returned the normalized copy as
|
# doing so previously returned the normalized copy as
|
||||||
# content_for_replacement, which caused the whole file to be rewritten
|
# content_for_replacement, which caused the whole file to be rewritten
|
||||||
# with collapsed indentation (every untouched line got reformatted).
|
# with collapsed indentation (every untouched line got reformatted).
|
||||||
stripped = old_text.strip('\n')
|
pattern = _build_fuzzy_pattern(old_text)
|
||||||
if stripped.strip():
|
if pattern is not None:
|
||||||
source_lines = stripped.split('\n')
|
|
||||||
line_patterns = []
|
|
||||||
for i, line in enumerate(source_lines):
|
|
||||||
tokens = line.split()
|
|
||||||
if not tokens:
|
|
||||||
line_patterns.append(r'[ \t]*')
|
|
||||||
continue
|
|
||||||
# Tolerate any run of blanks between tokens.
|
|
||||||
core = r'[ \t]+'.join(re.escape(tok) for tok in tokens)
|
|
||||||
# First-line leading whitespace is folded into the match only when
|
|
||||||
# old_text itself was indented here; otherwise it stays OUTSIDE the
|
|
||||||
# match so a no-indent old_text preserves (does not swallow and drop)
|
|
||||||
# the file's existing indentation -- mirroring an exact substring
|
|
||||||
# match. Inner lines always tolerate indentation: it sits inside the
|
|
||||||
# matched region and is re-supplied by new_text.
|
|
||||||
if i > 0 or line[:1] in (' ', '\t'):
|
|
||||||
core = r'[ \t]*' + core
|
|
||||||
line_patterns.append(core + r'[ \t]*')
|
|
||||||
pattern = '\n'.join(line_patterns)
|
|
||||||
match = re.search(pattern, content)
|
match = re.search(pattern, content)
|
||||||
if match:
|
if match:
|
||||||
return FuzzyMatchResult(
|
return FuzzyMatchResult(
|
||||||
@@ -155,6 +170,28 @@ def fuzzy_find_text(content: str, old_text: str) -> FuzzyMatchResult:
|
|||||||
return FuzzyMatchResult(found=False)
|
return FuzzyMatchResult(found=False)
|
||||||
|
|
||||||
|
|
||||||
|
def count_matches(content: str, old_text: str) -> int:
|
||||||
|
"""
|
||||||
|
Count occurrences of ``old_text`` using the SAME strategy as
|
||||||
|
:func:`fuzzy_find_text`: an exact substring when one is present, otherwise
|
||||||
|
the whitespace-flexible fuzzy regex.
|
||||||
|
|
||||||
|
The edit tool's uniqueness guard must agree with the matcher that actually
|
||||||
|
performs the replacement. Counting through a separate normalization pass
|
||||||
|
(the previous approach) could disagree with the regex used to locate and
|
||||||
|
replace, so both paths now share :func:`_build_fuzzy_pattern`.
|
||||||
|
"""
|
||||||
|
if not old_text:
|
||||||
|
return 0
|
||||||
|
# Mirror fuzzy_find_text: prefer exact matching when it applies.
|
||||||
|
if content.find(old_text) != -1:
|
||||||
|
return content.count(old_text)
|
||||||
|
pattern = _build_fuzzy_pattern(old_text)
|
||||||
|
if pattern is None:
|
||||||
|
return 0
|
||||||
|
return len(re.findall(pattern, content))
|
||||||
|
|
||||||
|
|
||||||
def generate_diff_string(old_content: str, new_content: str) -> dict:
|
def generate_diff_string(old_content: str, new_content: str) -> dict:
|
||||||
"""
|
"""
|
||||||
Generate unified diff string
|
Generate unified diff string
|
||||||
|
|||||||
@@ -98,6 +98,41 @@ class TestEditFuzzyPreservesWhitespace(unittest.TestCase):
|
|||||||
"def foo():\n x = 100\n y = 2\n return x + y\n",
|
"def foo():\n x = 100\n y = 2\n return x + y\n",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_exact_match_rejects_multiple_occurrences(self):
|
||||||
|
# Two byte-identical statements; the exact-match path applies and the
|
||||||
|
# uniqueness guard counts exact occurrences, so the ambiguous edit is
|
||||||
|
# rejected instead of silently editing only the first.
|
||||||
|
with open(self.path, "w", encoding="utf-8") as f:
|
||||||
|
f.write("a = 1\nb = 2\na = 1\n")
|
||||||
|
result = self.tool.execute({
|
||||||
|
"path": self.path,
|
||||||
|
"oldText": "a = 1",
|
||||||
|
"newText": "a = 9",
|
||||||
|
})
|
||||||
|
self.assertEqual(result.status, "error", result.result)
|
||||||
|
self.assertIn("occurrences", result.result)
|
||||||
|
# An ambiguous match must leave the file untouched.
|
||||||
|
self.assertEqual(self._read(), "a = 1\nb = 2\na = 1\n")
|
||||||
|
|
||||||
|
def test_fuzzy_match_rejects_multiple_occurrences(self):
|
||||||
|
# oldText uses loose spacing, so the exact match fails and the fuzzy
|
||||||
|
# path runs. The uniqueness guard now counts with the SAME regex used
|
||||||
|
# to match/replace, so an ambiguous fuzzy match (two hits) is rejected
|
||||||
|
# rather than silently editing the first one.
|
||||||
|
with open(self.path, "w", encoding="utf-8") as f:
|
||||||
|
f.write("def foo():\n x = 1\n y = 2\n x = 1\n")
|
||||||
|
result = self.tool.execute({
|
||||||
|
"path": self.path,
|
||||||
|
"oldText": "x = 1",
|
||||||
|
"newText": "x = 99",
|
||||||
|
})
|
||||||
|
self.assertEqual(result.status, "error", result.result)
|
||||||
|
self.assertIn("occurrences", result.result)
|
||||||
|
self.assertEqual(
|
||||||
|
self._read(),
|
||||||
|
"def foo():\n x = 1\n y = 2\n x = 1\n",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user