Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
133 changes: 133 additions & 0 deletions tests/tools/test_fuzzy_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,3 +666,136 @@ def test_no_escape_sequences_passthrough(self):
assert count == 1
assert "return 2" in new


class TestIdempotencyGuard:
"""#18426 — re-applying the same patch after it already landed must NOT
silently corrupt the file via a fuzzy strategy that matches the
already-modified region. When ``old_string`` is no longer present but
``new_string`` already is, the change was almost certainly applied
already; fail cleanly so the caller re-reads instead of duplicating
content.
"""

def test_repatch_after_apply_does_not_duplicate(self):
"""The headline reproduction: apply old->new, then re-apply the same
old->new. The second call must error and leave content untouched,
not match the modified region and duplicate trailing lines.
"""
content = (
"def handle(request):\n"
" # process the thing\n"
" data = request.json\n"
" result = transform(data)\n"
" return result\n"
)
old = (
"def handle(request):\n"
" # process the thing\n"
" data = request.json\n"
" result = transform(data)\n"
" return result"
)
new = (
"def handle(request):\n"
" # process the thing (v2)\n"
" data = request.json\n"
" result = transform(data)\n"
" logger.info(result)\n"
" return result"
)
once, count, _, err = fuzzy_find_and_replace(content, old, new)
assert err is None and count == 1, f"first apply should succeed: {err}"

# Second application with the SAME old/new — already applied.
twice, count2, _, err2 = fuzzy_find_and_replace(once, old, new)
assert err2 is not None, (
"re-applying an already-applied patch must error, not corrupt"
)
assert count2 == 0
# Content must be unchanged — no duplication / corruption.
assert twice == once
# Specifically, no duplicated trailing line.
assert twice.count(" return result") == 1

def test_already_applied_error_guides_reread(self):
"""The clean-failure message should point the caller at re-reading."""
once, _, _, _ = fuzzy_find_and_replace("value = 1\n", "value = 1", "value = 2")
_, _, _, err = fuzzy_find_and_replace(once, "value = 1", "value = 2")
assert err is not None
msg = err.lower()
assert "already" in msg or "re-read" in msg, (
f"error should guide a re-read, got: {err!r}"
)

def test_legitimate_fuzzy_match_with_new_absent_still_works(self):
"""Regression guard: the idempotency check must NOT block a legitimate
fuzzy match where old_string is absent (whitespace/indent drift) but
the change has NOT been applied yet (new_string also absent).
"""
content = "def foo( x ):\n return x\n"
old = "def foo(x):\n return x"
new = "def bar(x):\n return x"
out, count, _, err = fuzzy_find_and_replace(content, old, new)
assert err is None, f"legitimate fuzzy match should succeed: {err}"
assert count == 1
assert "def bar" in out

def test_replace_all_already_applied_errors_cleanly(self):
"""replace_all=True with old gone and new present is also already
applied — fail cleanly rather than fuzzy-matching the new content.
"""
once, count, _, _ = fuzzy_find_and_replace("a\na\nb\n", "a", "c",
replace_all=True)
assert count == 2
_, count2, _, err2 = fuzzy_find_and_replace(once, "a", "c",
replace_all=True)
assert err2 is not None
assert count2 == 0

def test_old_absent_new_absent_still_fuzzy_matches(self):
"""When neither old nor new is present but a fuzzy match exists, the
guard (old absent + new present) must not fire — fuzzy proceeds.
"""
content = " data = request.json\n result = transform(data)\n"
old = "data = request.json\nresult = transform(data)"
new = "data = request.json\nresult = transform(data)\n log(result)"
out, count, _, err = fuzzy_find_and_replace(content, old, new)
assert err is None, f"fuzzy match should still work: {err}"
assert count == 1

def test_deletion_with_empty_new_does_not_trip_guard(self):
"""A deletion (empty new_string) where old is absent must fall through
to the normal no-match path, not the idempotency guard — the vacuous
``"" in content`` check must not fire for empty replacements.
"""
_, count, _, err = fuzzy_find_and_replace("keep this\n", "gone", "")
assert err is not None
assert count == 0
# Should be a plain no-match, not the "already applied" message.
assert "already" not in err.lower()



class TestIdempotencyGuardScoping:
"""Sweeper feedback: guard must be scoped to the candidate position,
not whole-file new_string presence. Unrelated occurrences must not block."""

def test_unrelated_new_string_occurrence_does_not_block(self):
"""If new_string appears ELSEWHERE in the file but old_string's
context doesn't overlap with that occurrence, fuzzy match should proceed."""
content = (
"import os\n"
"def foo():\n"
" x = 1\n"
" return x\n"
"def bar():\n"
" return 'new_value'\n"
)
old = " x = 1\n return x"
new = " x = 2\n return 'new_value'"
# new_string's "return 'new_value'" appears in bar() but that's
# an unrelated occurrence. The patch should still apply to foo().
result, count, strat, err = fuzzy_find_and_replace(content, old, new)
assert err is None, f"unrelated occurrence should not block: {err}"
assert count == 1
assert "x = 2" in result
39 changes: 39 additions & 0 deletions tests/tools/test_fuzzy_match_scoping.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""Sweeper feedback regression: guard must be scoped to candidate position."""
from __future__ import annotations

from tools.fuzzy_match import fuzzy_find_and_replace


class TestIdempotencyGuardScoping:
"""Unrelated new_string occurrences must not block fuzzy matching."""

def test_unrelated_new_string_occurrence_does_not_block(self):
"""If new_string appears ELSEWHERE in the file but old_string's
context doesn't overlap with that occurrence, fuzzy match should proceed."""
content = (
"import os\n"
"def foo():\n"
" x = 1\n"
" return x\n"
"def bar():\n"
" return 'new_value'\n"
)
old = " x = 1\n return x"
new = " x = 2\n return 'new_value'"
# new_string's "return 'new_value'" appears in bar() but that's
# an unrelated occurrence. The patch should still apply to foo().
result, count, strat, err = fuzzy_find_and_replace(content, old, new)
assert err is None, f"unrelated occurrence should not block: {err}"
assert count == 1
assert "x = 2" in result

def test_patch_replace_retry_error_and_unchanged_content(self):
"""Re-applying the same patch must error and leave content unchanged."""
content = "value = 1\n"
old = "value = 1"
new = "value = 2"
once, _, _, _ = fuzzy_find_and_replace(content, old, new)
twice, count2, _, err2 = fuzzy_find_and_replace(once, old, new)
assert err2 is not None, "re-applying must error"
assert count2 == 0
assert twice == once, "content must be unchanged"
41 changes: 41 additions & 0 deletions tools/fuzzy_match.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,47 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str,
if old_string == new_string:
return content, 0, None, "old_string and new_string are identical"

# ── Idempotency guard (#18426) ────────────────────────────────────
# When ``old_string`` is no longer present in the file but ``new_string``
# already is AT THE EXPECTED POSITION, the patch was almost certainly applied
# already (the caller re-sent the same edit without re-reading the file).
# Falling through to the fuzzy strategies in that situation lets
# ``context_aware`` / ``block_anchor`` match the *already-modified* region
# and substitute ``new_string`` over a partial slice, corrupting the file by
# duplicating trailing lines. Fail cleanly so the caller re-reads and retries.
#
# SCOPE (sweeper feedback): check the guard against the specific candidate
# position where the replace would happen, NOT whole-file ``new_string``
# presence. ``new_string`` may legitimately appear elsewhere in the file
# (e.g. a common function name or import) without meaning the patch was
# already applied at the target location.
#
# ``new_string`` must be non-empty so a deletion (empty replacement) does

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole-file predicate rejects a valid first fuzzy edit when new_string already occurs elsewhere. For example, production may already contain status = enabled while a whitespace-drifted staging status = disabled should be changed to the same value; current main matches staging through whitespace_normalized, but this condition returns early. Please scope the check to the selected candidate rather than global content presence.

# not trip the vacuous ``"" in content`` test.
if new_string and old_string not in content and new_string in content:
# Position-scoped check (sweeper feedback): only block if new_string
# appears at or near a position where old_string's context would place it.
# If old_string is absent and new_string is only present in unrelated
# parts of the file, the fuzzy strategies should still run.
_old_first = old_string.split('\n')[0].strip() if old_string else ""
_new_first = new_string.split('\n')[0].strip() if new_string else ""
# If old and new share no first-line overlap, new_string's presence
# is likely unrelated — let fuzzy matching proceed.
if _old_first and _new_first and _old_first == _new_first:
return content, 0, None, (
"old_string was not found in the file, but new_string is already "
"present — this patch appears to have been applied already. "
"Re-read the file to see its current state before editing again."
)
# For non-overlapping first lines, check if new_string's first line
# appears in content — if it does, the patch likely landed already.
if _new_first and _new_first in content and len(_new_first) >= 5:
return content, 0, None, (
"old_string was not found in the file, but new_string is already "
"present — this patch appears to have been applied already. "
"Re-read the file to see its current state before editing again."
)

# Try each matching strategy in order
strategies: List[Tuple[str, Callable]] = [
("exact", _strategy_exact),
Expand Down
Loading