diff --git a/tests/tools/test_fuzzy_match.py b/tests/tools/test_fuzzy_match.py index 9db45b7a5..3f7d31582 100644 --- a/tests/tools/test_fuzzy_match.py +++ b/tests/tools/test_fuzzy_match.py @@ -262,3 +262,70 @@ class TestFindClosestLines: # Should include line numbers in format "N| content" assert "|" in result + +class TestFormatNoMatchHint: + """Gating tests for format_no_match_hint — the shared helper that decides + whether a 'Did you mean?' snippet should be appended to an error. + """ + + def setup_method(self): + from tools.fuzzy_match import format_no_match_hint + self.fmt = format_no_match_hint + + def test_fires_on_could_not_find_with_match(self): + """Classic no-match: similar content exists → hint fires.""" + content = "def foo():\n pass\ndef bar():\n pass\n" + result = self.fmt( + "Could not find a match for old_string in the file", + 0, "def baz():", content, + ) + assert "Did you mean" in result + assert "foo" in result or "bar" in result + + def test_silent_on_ambiguous_match_error(self): + """'Found N matches' is not a missing-match failure — no hint.""" + content = "aaa bbb aaa\n" + result = self.fmt( + "Found 2 matches for old_string. Provide more context to make it unique, or use replace_all=True.", + 0, "aaa", content, + ) + assert result == "" + + def test_silent_on_escape_drift_error(self): + """Escape-drift errors are intentional blocks — hint would mislead.""" + content = "x = 1\n" + result = self.fmt( + "Escape-drift detected: old_string and new_string contain the literal sequence '\\\\''...", + 0, "x = \\'1\\'", content, + ) + assert result == "" + + def test_silent_on_identical_strings(self): + """old_string == new_string — hint irrelevant.""" + result = self.fmt( + "old_string and new_string are identical", + 0, "foo", "foo bar\n", + ) + assert result == "" + + def test_silent_when_match_count_nonzero(self): + """If match succeeded, we shouldn't be in the error path — defense in depth.""" + result = self.fmt( + "Could not find a match for old_string in the file", + 1, "foo", "foo bar\n", + ) + assert result == "" + + def test_silent_on_none_error(self): + """No error at all — no hint.""" + result = self.fmt(None, 0, "foo", "bar\n") + assert result == "" + + def test_silent_when_no_similar_content(self): + """Even for a valid no-match error, skip hint when nothing similar exists.""" + result = self.fmt( + "Could not find a match for old_string in the file", + 0, "totally_unique_xyzzy_qux", "abc\nxyz\n", + ) + assert result == "" + diff --git a/tools/file_operations.py b/tools/file_operations.py index c9b5d3d64..87ad13968 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -741,10 +741,8 @@ class ShellFileOperations(FileOperations): if error or match_count == 0: err_msg = error or f"Could not find match for old_string in {path}" try: - from tools.fuzzy_match import find_closest_lines - hint = find_closest_lines(old_string, content) - if hint: - err_msg += "\n\nDid you mean one of these sections?\n" + hint + from tools.fuzzy_match import format_no_match_hint + err_msg += format_no_match_hint(err_msg, match_count, old_string, content) except Exception: pass return PatchResult(error=err_msg) diff --git a/tools/file_tools.py b/tools/file_tools.py index af6701f82..5b44ff03d 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -670,8 +670,11 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None, result_json = json.dumps(result_dict, ensure_ascii=False) # Hint when old_string not found — saves iterations where the agent # retries with stale content instead of re-reading the file. + # Suppressed when patch_replace already attached a rich "Did you mean?" + # snippet (which is strictly more useful than the generic hint). if result_dict.get("error") and "Could not find" in str(result_dict["error"]): - result_json += "\n\n[Hint: old_string not found. Use read_file to verify the current content, or search_files to locate the text.]" + if "Did you mean one of these sections?" not in str(result_dict["error"]): + result_json += "\n\n[Hint: old_string not found. Use read_file to verify the current content, or search_files to locate the text.]" return result_json except Exception as e: return tool_error(str(e)) diff --git a/tools/fuzzy_match.py b/tools/fuzzy_match.py index 301794644..9a922cd9b 100644 --- a/tools/fuzzy_match.py +++ b/tools/fuzzy_match.py @@ -681,3 +681,24 @@ def find_closest_lines(old_string: str, content: str, context_lines: int = 2, ma return "" return "\n---\n".join(parts) + + +def format_no_match_hint(error: Optional[str], match_count: int, + old_string: str, content: str) -> str: + """Return a '\\n\\nDid you mean...' snippet for plain no-match errors. + + Gated so the hint only fires for actual "old_string not found" failures. + Ambiguous-match ("Found N matches"), escape-drift, and identical-strings + errors all have ``match_count == 0`` but a "did you mean?" snippet would + be misleading — those failed for unrelated reasons. + + Returns an empty string when there's nothing useful to append. + """ + if match_count != 0: + return "" + if not error or not error.startswith("Could not find"): + return "" + hint = find_closest_lines(old_string, content) + if not hint: + return "" + return "\n\nDid you mean one of these sections?\n" + hint diff --git a/tools/patch_parser.py b/tools/patch_parser.py index 0c961083c..d2a298fc9 100644 --- a/tools/patch_parser.py +++ b/tools/patch_parser.py @@ -290,10 +290,16 @@ def _validate_operations( ) if count == 0: label = f"'{hunk.context_hint}'" if hunk.context_hint else "(no hint)" - errors.append( + msg = ( f"{op.file_path}: hunk {label} not found" + (f" — {match_error}" if match_error else "") ) + try: + from tools.fuzzy_match import format_no_match_hint + msg += format_no_match_hint(match_error, count, search_pattern, simulated) + except Exception: + pass + errors.append(msg) else: # Advance simulation so subsequent hunks validate correctly. # 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]: error = None if error: - return False, f"Could not apply hunk: {error}" + err_msg = f"Could not apply hunk: {error}" + try: + from tools.fuzzy_match import format_no_match_hint + err_msg += format_no_match_hint(error, 0, search_pattern, new_content) + except Exception: + pass + return False, err_msg else: # Addition-only hunk (no context or removed lines). # Insert at the location indicated by the context hint, or at end of file. diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 33d3976ea..493b434c5 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -449,9 +449,15 @@ def _patch_skill( if match_error: # Show a short preview of the file so the model can self-correct preview = content[:500] + ("..." if len(content) > 500 else "") + err_msg = match_error + try: + from tools.fuzzy_match import format_no_match_hint + err_msg += format_no_match_hint(match_error, match_count, old_string, content) + except Exception: + pass return { "success": False, - "error": match_error, + "error": err_msg, "file_preview": preview, }