feat: add high-value tool result hints for patch and search_files (#722)
Add contextual [Hint: ...] suffixes to tool results where they save real iterations: - patch (no match): suggests read_file/search_files to verify content before retrying — addresses the common pattern where the agent retries with stale old_string instead of re-reading the file. - search_files (truncated): provides explicit next offset and suggests narrowing the search — clearer than relying on total_count inference. Other hints proposed in #722 (terminal, web_search, web_extract, browser_snapshot, search zero-results, search content-matches) were evaluated and found to be low-value: either already covered by existing mechanisms (read_file pagination, similar-files, schema descriptions) or guidance the agent already follows from its own reasoning. 5 new tests covering hint presence/absence for both tools.
This commit is contained in:
parent
97b1c76b14
commit
491605cfea
2 changed files with 103 additions and 2 deletions
|
|
@ -200,3 +200,91 @@ class TestSearchHandler:
|
||||||
from tools.file_tools import search_tool
|
from tools.file_tools import search_tool
|
||||||
result = json.loads(search_tool(pattern="x"))
|
result = json.loads(search_tool(pattern="x"))
|
||||||
assert "error" in result
|
assert "error" in result
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Tool result hint tests (#722)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
class TestPatchHints:
|
||||||
|
"""Patch tool should hint when old_string is not found."""
|
||||||
|
|
||||||
|
@patch("tools.file_tools._get_file_ops")
|
||||||
|
def test_no_match_includes_hint(self, mock_get):
|
||||||
|
mock_ops = MagicMock()
|
||||||
|
result_obj = MagicMock()
|
||||||
|
result_obj.to_dict.return_value = {
|
||||||
|
"error": "Could not find match for old_string in foo.py"
|
||||||
|
}
|
||||||
|
mock_ops.patch_replace.return_value = result_obj
|
||||||
|
mock_get.return_value = mock_ops
|
||||||
|
|
||||||
|
from tools.file_tools import patch_tool
|
||||||
|
raw = patch_tool(mode="replace", path="foo.py", old_string="x", new_string="y")
|
||||||
|
assert "[Hint:" in raw
|
||||||
|
assert "read_file" in raw
|
||||||
|
|
||||||
|
@patch("tools.file_tools._get_file_ops")
|
||||||
|
def test_success_no_hint(self, mock_get):
|
||||||
|
mock_ops = MagicMock()
|
||||||
|
result_obj = MagicMock()
|
||||||
|
result_obj.to_dict.return_value = {"success": True, "diff": "--- a\n+++ b"}
|
||||||
|
mock_ops.patch_replace.return_value = result_obj
|
||||||
|
mock_get.return_value = mock_ops
|
||||||
|
|
||||||
|
from tools.file_tools import patch_tool
|
||||||
|
raw = patch_tool(mode="replace", path="foo.py", old_string="x", new_string="y")
|
||||||
|
assert "[Hint:" not in raw
|
||||||
|
|
||||||
|
|
||||||
|
class TestSearchHints:
|
||||||
|
"""Search tool should hint when results are truncated."""
|
||||||
|
|
||||||
|
@patch("tools.file_tools._get_file_ops")
|
||||||
|
def test_truncated_results_hint(self, mock_get):
|
||||||
|
mock_ops = MagicMock()
|
||||||
|
result_obj = MagicMock()
|
||||||
|
result_obj.to_dict.return_value = {
|
||||||
|
"total_count": 100,
|
||||||
|
"matches": [{"path": "a.py", "line": 1, "content": "x"}] * 50,
|
||||||
|
"truncated": True,
|
||||||
|
}
|
||||||
|
mock_ops.search.return_value = result_obj
|
||||||
|
mock_get.return_value = mock_ops
|
||||||
|
|
||||||
|
from tools.file_tools import search_tool
|
||||||
|
raw = search_tool(pattern="foo", offset=0, limit=50)
|
||||||
|
assert "[Hint:" in raw
|
||||||
|
assert "offset=50" in raw
|
||||||
|
|
||||||
|
@patch("tools.file_tools._get_file_ops")
|
||||||
|
def test_non_truncated_no_hint(self, mock_get):
|
||||||
|
mock_ops = MagicMock()
|
||||||
|
result_obj = MagicMock()
|
||||||
|
result_obj.to_dict.return_value = {
|
||||||
|
"total_count": 3,
|
||||||
|
"matches": [{"path": "a.py", "line": 1, "content": "x"}] * 3,
|
||||||
|
}
|
||||||
|
mock_ops.search.return_value = result_obj
|
||||||
|
mock_get.return_value = mock_ops
|
||||||
|
|
||||||
|
from tools.file_tools import search_tool
|
||||||
|
raw = search_tool(pattern="foo")
|
||||||
|
assert "[Hint:" not in raw
|
||||||
|
|
||||||
|
@patch("tools.file_tools._get_file_ops")
|
||||||
|
def test_truncated_hint_with_nonzero_offset(self, mock_get):
|
||||||
|
mock_ops = MagicMock()
|
||||||
|
result_obj = MagicMock()
|
||||||
|
result_obj.to_dict.return_value = {
|
||||||
|
"total_count": 150,
|
||||||
|
"matches": [{"path": "a.py", "line": 1, "content": "x"}] * 50,
|
||||||
|
"truncated": True,
|
||||||
|
}
|
||||||
|
mock_ops.search.return_value = result_obj
|
||||||
|
mock_get.return_value = mock_ops
|
||||||
|
|
||||||
|
from tools.file_tools import search_tool
|
||||||
|
raw = search_tool(pattern="foo", offset=50, limit=50)
|
||||||
|
assert "[Hint:" in raw
|
||||||
|
assert "offset=100" in raw
|
||||||
|
|
|
||||||
|
|
@ -164,7 +164,13 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
|
||||||
else:
|
else:
|
||||||
return json.dumps({"error": f"Unknown mode: {mode}"})
|
return json.dumps({"error": f"Unknown mode: {mode}"})
|
||||||
|
|
||||||
return json.dumps(result.to_dict(), ensure_ascii=False)
|
result_dict = result.to_dict()
|
||||||
|
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.
|
||||||
|
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.]"
|
||||||
|
return result_json
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return json.dumps({"error": str(e)}, ensure_ascii=False)
|
return json.dumps({"error": str(e)}, ensure_ascii=False)
|
||||||
|
|
||||||
|
|
@ -180,7 +186,14 @@ def search_tool(pattern: str, target: str = "content", path: str = ".",
|
||||||
pattern=pattern, path=path, target=target, file_glob=file_glob,
|
pattern=pattern, path=path, target=target, file_glob=file_glob,
|
||||||
limit=limit, offset=offset, output_mode=output_mode, context=context
|
limit=limit, offset=offset, output_mode=output_mode, context=context
|
||||||
)
|
)
|
||||||
return json.dumps(result.to_dict(), ensure_ascii=False)
|
result_dict = result.to_dict()
|
||||||
|
result_json = json.dumps(result_dict, ensure_ascii=False)
|
||||||
|
# Hint when results were truncated — explicit next offset is clearer
|
||||||
|
# than relying on the model to infer it from total_count vs match count.
|
||||||
|
if result_dict.get("truncated"):
|
||||||
|
next_offset = offset + limit
|
||||||
|
result_json += f"\n\n[Hint: Results truncated. Use offset={next_offset} to see more, or narrow with a more specific pattern or file_glob.]"
|
||||||
|
return result_json
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return json.dumps({"error": str(e)}, ensure_ascii=False)
|
return json.dumps({"error": str(e)}, ensure_ascii=False)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue