fix(tools): reject quote escapes that only the replacement carries

Quote-drift correction stripped every escaped quote kind in new_string
once old_string verified against the file. Only the kinds old_string
escapes are proven drift; a kind escaped only in new_string may be an
escape the model meant. For a file holding message = "it's fine", the
call old_string=it\'s fine, new_string=it\'s \"great\" wrote
message = "it's "great"" and reported success.

Raise AmbiguousEscapeDriftError for that case, the same rejection the
backslash path uses for unverifiable replacement escapes, so the file
stays unchanged and the model resends with its intended escaping.

Reported by Greptile on #137288.
This commit is contained in:
emozilla authored and Jeffrey Quesnelle committed 2026-10-12 13:41:29 -04:00
1 parent 6701840772
commit 3a3d1281eb
2 files changed
+41 -3

No files matched your search

+27 -1
View File
@@ -9,7 +9,7 @@ import json
import pytest
from tools.escape_drift_autocorrect import maybe_correct_escape_drift
from tools.escape_drift_autocorrect import AmbiguousEscapeDriftError, maybe_correct_escape_drift
@pytest.fixture
@@ -38,6 +38,24 @@ def test_mixed_quote_escaping_is_not_corrected():
assert maybe_correct_escape_drift(old, new, content) == (old, new, None)
# The anchor proves only apostrophe drift; the replacement's \" has no evidence either way.
_UNPROVEN_QUOTE_KIND = (
'message = "it' + "'" + 's fine"\n',
r"it\'s fine",
r"it\'s \"great\"",
)
def test_quote_kind_escaped_only_in_replacement_is_rejected():
content, old, new = _UNPROVEN_QUOTE_KIND
with pytest.raises(AmbiguousEscapeDriftError):
maybe_correct_escape_drift(old, new, content)
from tools.fuzzy_match import fuzzy_find_and_replace
out, count, _strategy, err = fuzzy_find_and_replace(content, old, new)
assert count == 0 and out == content
assert "intended escaping" in err
_SHARED_CASES = [
('pat = r"\\d+"\n', r'pat = r"\\d+"', 'pat = None', 'pat = None\n'),
("x = 'it'\n", r"x = \'it\'", r"x = \'its\'", "x = 'its'\n"),
@@ -131,6 +149,14 @@ class TestPatchEndToEnd:
assert r.get("success") is not True
assert f.read_text() == original
def test_quote_kind_escaped_only_in_replacement_is_rejected(self, workdir):
content, old, new = _UNPROVEN_QUOTE_KIND
f = workdir / "e.py"
f.write_text(content)
r = _patch_tool(path=str(f), old_string=old, new_string=new, task_id="t-drift")
assert r.get("success") is not True
assert f.read_text() == content
def test_quote_mixed_call_rejected_not_mangled(self, workdir):
content, old, new = _MIXED_CALLS["quote_new_line_really_escaped"]
f = workdir / "b.py"
+14 -2
View File
@@ -94,7 +94,8 @@ def maybe_correct_quote_escaping(old_string: str, new_string: str, content: str)
appear verbatim in content, which verifies old_string (and rules out a file that really
contains ``\\'``/``\\"``). new_string's added code is not in the file, so it is only
stripped when ``_quote_escapes_all_spurious`` holds for the whole call; otherwise the call
is left unchanged for the existing reject-and-ask guard.
is left unchanged for the existing reject-and-ask guard. Raises ``AmbiguousEscapeDriftError``
when new_string escapes a quote kind that old_string does not.
"""
has_quote_suspect = (
("\\'" in old_string and "\\'" in new_string)
@@ -108,6 +109,16 @@ def maybe_correct_quote_escaping(old_string: str, new_string: str, content: str)
stripped_old = _strip_spurious_quote_escapes(old_string)
if stripped_old not in content:
return old_string, new_string
# Only old_string is checked against the file, so only the quote kinds it escapes are
# proven drift. A kind escaped only in new_string may be an escape the model meant.
unproven = [q for q in ("'", '"') if "\\" + q in new_string and "\\" + q not in old_string]
if unproven:
shown = " and ".join("\\" + q for q in unproven)
raise AmbiguousEscapeDriftError(
"Escape-drift detected: removing the spurious quote escapes in old_string matches "
f"the file, but new_string also contains {shown}, which old_string gives no evidence "
"for. Re-read the file and resend old_string/new_string with their intended escaping."
)
return stripped_old, _strip_spurious_quote_escapes(new_string)
@@ -118,7 +129,8 @@ def maybe_correct_escape_drift(old_string: str, new_string: str, content: str) -
Tries quote escaping, then backslash correction of the anchor only. Raises
``AmbiguousEscapeDriftError`` for a verified doubled anchor whose replacement
contains backslashes; the caller must return an error without attempting a match.
contains backslashes, or a verified quote correction whose replacement escapes a
quote kind the anchor does not; the caller must return an error without matching.
Returns ``(corrected_old, corrected_new, note)``: ``note`` is a human-readable description
of the correction that was applied, or ``None`` if neither fired. The caller attaches