fix(agent): address code review findings for tool hint enhancement

- C1: Fix IndexError on empty list arguments via _get_args() helper
- I1: Remove redundant branch in _fmt_known
- I2: Export abbreviate_path from nanobot.utils.__init__
- I3: Fix _abbreviate_url negative-budget format consistency
- S1: Move FORMATS to class-level _TOOL_HINT_FORMATS constant
- S2: Add list_dir to FORMATS registry (ls path)
- G1-G5: Add tests for empty list args, None args, URL edge cases,
  mixed folding groups, and list_dir format
This commit is contained in:
chengyongru
2026-04-07 15:15:07 +08:00
committed by Xubin Ren
parent b1d3c00deb
commit 3e3a7654f8
5 changed files with 107 additions and 23 deletions
+49 -1
View File
@@ -4,7 +4,7 @@ from nanobot.agent.loop import AgentLoop
from nanobot.providers.base import ToolCallRequest
def _tc(name: str, args: dict) -> ToolCallRequest:
def _tc(name: str, args) -> ToolCallRequest:
return ToolCallRequest(id="c1", name=name, arguments=args)
@@ -147,3 +147,51 @@ class TestToolHintMultipleCalls:
assert 'grep "TODO"' in result
assert "read main.py" in result
assert ", " in result
class TestToolHintEdgeCases:
"""Test edge cases and defensive handling (G1, G2)."""
def test_known_tool_empty_list_args(self):
"""C1/G1: Empty list arguments should not crash."""
result = AgentLoop._tool_hint([_tc("read_file", [])])
assert result == "read_file"
def test_known_tool_none_args(self):
"""G2: None arguments should not crash."""
result = AgentLoop._tool_hint([_tc("read_file", None)])
assert result == "read_file"
def test_fallback_empty_list_args(self):
"""C1: Empty list args in fallback should not crash."""
result = AgentLoop._tool_hint([_tc("custom_tool", [])])
assert result == "custom_tool"
def test_fallback_none_args(self):
"""G2: None args in fallback should not crash."""
result = AgentLoop._tool_hint([_tc("custom_tool", None)])
assert result == "custom_tool"
def test_list_dir_registered(self):
"""S2: list_dir should use 'ls' format."""
result = AgentLoop._tool_hint([_tc("list_dir", {"path": "/tmp"})])
assert result == "ls /tmp"
class TestToolHintMixedFolding:
"""G4: Mixed folding groups with interleaved same-tool segments."""
def test_read_read_grep_grep_read(self):
"""read×2, grep×2, read — should produce two separate groups."""
calls = [
_tc("read_file", {"path": "a.py"}),
_tc("read_file", {"path": "b.py"}),
_tc("grep", {"pattern": "x"}),
_tc("grep", {"pattern": "y"}),
_tc("read_file", {"path": "c.py"}),
]
result = AgentLoop._tool_hint(calls)
assert "\u00d7 2" in result
# Should have 3 groups: read×2, grep×2, read
parts = result.split(", ")
assert len(parts) == 3
+26
View File
@@ -77,3 +77,29 @@ class TestAbbreviatePathURLs:
def test_short_url_unchanged(self):
url = "https://example.com/api"
assert abbreviate_path(url) == url
def test_url_no_path_just_domain(self):
"""G3: URL with no path should return as-is if short enough."""
url = "https://example.com"
assert abbreviate_path(url) == url
def test_url_with_query_string(self):
"""G3: URL with query params should abbreviate path part."""
url = "https://example.com/api/v2/endpoint?key=value&other=123"
result = abbreviate_path(url, max_len=40)
assert "example.com" in result
assert "\u2026" in result
def test_url_very_long_basename(self):
"""G3: URL with very long basename should truncate basename."""
url = "https://example.com/path/very_long_resource_name_file.json"
result = abbreviate_path(url, max_len=35)
assert "example.com" in result
assert "\u2026" in result
def test_url_negative_budget_consistent_format(self):
"""I3: Negative budget should still produce domain/…/basename format."""
url = "https://a.co/very/deep/path/with/lots/of/segments/and/a/long/basename.txt"
result = abbreviate_path(url, max_len=20)
assert "a.co" in result
assert "/\u2026/" in result