Skip to content

Commit 5e6427a

Browse files
committed
fix(patch): gate 'did you mean?' to no-match + extend to v4a/skill_manage
Follow-ups on top of @teyrebaz33's cherry-picked commit: 1. New shared helper format_no_match_hint() in fuzzy_match.py with a startswith('Could not find') gate so the snippet only appends to genuine no-match errors — not to 'Found N matches' (ambiguous), 'Escape-drift detected', or 'identical strings' errors, which would all mislead the model. 2. file_tools.patch_tool suppresses the legacy generic '[Hint: old_string not found...]' string when the rich 'Did you mean?' snippet is already attached — no more double-hint. 3. Wire the same helper into patch_parser.py (V4A patch mode, both _validate_operations and _apply_update) and skill_manager_tool.py so all three fuzzy callers surface the hint consistently. Tests: 7 new gating tests in TestFormatNoMatchHint cover every error class (ambiguous, drift, identical, non-zero match count, None error, no similar content, happy path). 34/34 test_fuzzy_match, 96/96 test_file_tools + test_patch_parser + test_skill_manager_tool pass. E2E verified across all four scenarios: no-match-with-similar, no-match-no-similar, ambiguous, success. V4A mode confirmed end-to-end with a non-matching hunk.
1 parent 15abf4e commit 5e6427a

6 files changed

Lines changed: 115 additions & 8 deletions

File tree

tests/tools/test_fuzzy_match.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,70 @@ def test_includes_line_numbers(self):
262262
# Should include line numbers in format "N| content"
263263
assert "|" in result
264264

265+
266+
class TestFormatNoMatchHint:
267+
"""Gating tests for format_no_match_hint — the shared helper that decides
268+
whether a 'Did you mean?' snippet should be appended to an error.
269+
"""
270+
271+
def setup_method(self):
272+
from tools.fuzzy_match import format_no_match_hint
273+
self.fmt = format_no_match_hint
274+
275+
def test_fires_on_could_not_find_with_match(self):
276+
"""Classic no-match: similar content exists → hint fires."""
277+
content = "def foo():\n pass\ndef bar():\n pass\n"
278+
result = self.fmt(
279+
"Could not find a match for old_string in the file",
280+
0, "def baz():", content,
281+
)
282+
assert "Did you mean" in result
283+
assert "foo" in result or "bar" in result
284+
285+
def test_silent_on_ambiguous_match_error(self):
286+
"""'Found N matches' is not a missing-match failure — no hint."""
287+
content = "aaa bbb aaa\n"
288+
result = self.fmt(
289+
"Found 2 matches for old_string. Provide more context to make it unique, or use replace_all=True.",
290+
0, "aaa", content,
291+
)
292+
assert result == ""
293+
294+
def test_silent_on_escape_drift_error(self):
295+
"""Escape-drift errors are intentional blocks — hint would mislead."""
296+
content = "x = 1\n"
297+
result = self.fmt(
298+
"Escape-drift detected: old_string and new_string contain the literal sequence '\\\\''...",
299+
0, "x = \\'1\\'", content,
300+
)
301+
assert result == ""
302+
303+
def test_silent_on_identical_strings(self):
304+
"""old_string == new_string — hint irrelevant."""
305+
result = self.fmt(
306+
"old_string and new_string are identical",
307+
0, "foo", "foo bar\n",
308+
)
309+
assert result == ""
310+
311+
def test_silent_when_match_count_nonzero(self):
312+
"""If match succeeded, we shouldn't be in the error path — defense in depth."""
313+
result = self.fmt(
314+
"Could not find a match for old_string in the file",
315+
1, "foo", "foo bar\n",
316+
)
317+
assert result == ""
318+
319+
def test_silent_on_none_error(self):
320+
"""No error at all — no hint."""
321+
result = self.fmt(None, 0, "foo", "bar\n")
322+
assert result == ""
323+
324+
def test_silent_when_no_similar_content(self):
325+
"""Even for a valid no-match error, skip hint when nothing similar exists."""
326+
result = self.fmt(
327+
"Could not find a match for old_string in the file",
328+
0, "totally_unique_xyzzy_qux", "abc\nxyz\n",
329+
)
330+
assert result == ""
331+

tools/file_operations.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -741,10 +741,8 @@ def patch_replace(self, path: str, old_string: str, new_string: str,
741741
if error or match_count == 0:
742742
err_msg = error or f"Could not find match for old_string in {path}"
743743
try:
744-
from tools.fuzzy_match import find_closest_lines
745-
hint = find_closest_lines(old_string, content)
746-
if hint:
747-
err_msg += "\n\nDid you mean one of these sections?\n" + hint
744+
from tools.fuzzy_match import format_no_match_hint
745+
err_msg += format_no_match_hint(err_msg, match_count, old_string, content)
748746
except Exception:
749747
pass
750748
return PatchResult(error=err_msg)

tools/file_tools.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,8 +670,11 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
670670
result_json = json.dumps(result_dict, ensure_ascii=False)
671671
# Hint when old_string not found — saves iterations where the agent
672672
# retries with stale content instead of re-reading the file.
673+
# Suppressed when patch_replace already attached a rich "Did you mean?"
674+
# snippet (which is strictly more useful than the generic hint).
673675
if result_dict.get("error") and "Could not find" in str(result_dict["error"]):
674-
result_json += "\n\n[Hint: old_string not found. Use read_file to verify the current content, or search_files to locate the text.]"
676+
if "Did you mean one of these sections?" not in str(result_dict["error"]):
677+
result_json += "\n\n[Hint: old_string not found. Use read_file to verify the current content, or search_files to locate the text.]"
675678
return result_json
676679
except Exception as e:
677680
return tool_error(str(e))

tools/fuzzy_match.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,3 +681,24 @@ def find_closest_lines(old_string: str, content: str, context_lines: int = 2, ma
681681
return ""
682682

683683
return "\n---\n".join(parts)
684+
685+
686+
def format_no_match_hint(error: Optional[str], match_count: int,
687+
old_string: str, content: str) -> str:
688+
"""Return a '\\n\\nDid you mean...' snippet for plain no-match errors.
689+
690+
Gated so the hint only fires for actual "old_string not found" failures.
691+
Ambiguous-match ("Found N matches"), escape-drift, and identical-strings
692+
errors all have ``match_count == 0`` but a "did you mean?" snippet would
693+
be misleading — those failed for unrelated reasons.
694+
695+
Returns an empty string when there's nothing useful to append.
696+
"""
697+
if match_count != 0:
698+
return ""
699+
if not error or not error.startswith("Could not find"):
700+
return ""
701+
hint = find_closest_lines(old_string, content)
702+
if not hint:
703+
return ""
704+
return "\n\nDid you mean one of these sections?\n" + hint

tools/patch_parser.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,16 @@ def _validate_operations(
290290
)
291291
if count == 0:
292292
label = f"'{hunk.context_hint}'" if hunk.context_hint else "(no hint)"
293-
errors.append(
293+
msg = (
294294
f"{op.file_path}: hunk {label} not found"
295295
+ (f" — {match_error}" if match_error else "")
296296
)
297+
try:
298+
from tools.fuzzy_match import format_no_match_hint
299+
msg += format_no_match_hint(match_error, count, search_pattern, simulated)
300+
except Exception:
301+
pass
302+
errors.append(msg)
297303
else:
298304
# Advance simulation so subsequent hunks validate correctly.
299305
# Reuse the result from the call above — no second fuzzy run.
@@ -537,7 +543,13 @@ def _apply_update(op: PatchOperation, file_ops: Any) -> Tuple[bool, str]:
537543
error = None
538544

539545
if error:
540-
return False, f"Could not apply hunk: {error}"
546+
err_msg = f"Could not apply hunk: {error}"
547+
try:
548+
from tools.fuzzy_match import format_no_match_hint
549+
err_msg += format_no_match_hint(error, 0, search_pattern, new_content)
550+
except Exception:
551+
pass
552+
return False, err_msg
541553
else:
542554
# Addition-only hunk (no context or removed lines).
543555
# Insert at the location indicated by the context hint, or at end of file.

tools/skill_manager_tool.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,9 +449,15 @@ def _patch_skill(
449449
if match_error:
450450
# Show a short preview of the file so the model can self-correct
451451
preview = content[:500] + ("..." if len(content) > 500 else "")
452+
err_msg = match_error
453+
try:
454+
from tools.fuzzy_match import format_no_match_hint
455+
err_msg += format_no_match_hint(match_error, match_count, old_string, content)
456+
except Exception:
457+
pass
452458
return {
453459
"success": False,
454-
"error": match_error,
460+
"error": err_msg,
455461
"file_preview": preview,
456462
}
457463

0 commit comments

Comments
 (0)