From 547b502718c52af1a3721800ce248bdf9c7a8933 Mon Sep 17 00:00:00 2001 From: Kimi Agent Date: Sat, 14 Mar 2026 20:40:19 -0400 Subject: [PATCH] fix: smart_read_file accepts path= kwarg from LLMs (#113) LLMs naturally call read_file(path=...) but the wrapper only accepted file_name=. Pydantic strict validation rejected the mismatch. Now accepts both file_name and path kwargs, with clear error on missing both. Added 6 tests covering: positional args, path kwarg, no-args error, directory listing, empty dir, hidden file filtering. --- src/timmy/tools.py | 7 +- tests/test_smart_read_file.py | 123 ++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 tests/test_smart_read_file.py diff --git a/src/timmy/tools.py b/src/timmy/tools.py index e212438e..ec8db02b 100644 --- a/src/timmy/tools.py +++ b/src/timmy/tools.py @@ -196,8 +196,13 @@ def _make_smart_read_file(file_tools: FileTools) -> Callable: """ original_read = file_tools.read_file - def smart_read_file(file_name: str, encoding: str = "utf-8") -> str: + def smart_read_file(file_name: str = "", encoding: str = "utf-8", **kwargs) -> str: """Reads the contents of the file `file_name` and returns the contents if successful.""" + # LLMs often call read_file(path=...) instead of read_file(file_name=...) + if not file_name: + file_name = kwargs.get("path", "") + if not file_name: + return "Error: no file_name or path provided." # Resolve the path the same way FileTools does _safe, resolved = file_tools.check_escape(file_name) if _safe and resolved.is_dir(): diff --git a/tests/test_smart_read_file.py b/tests/test_smart_read_file.py new file mode 100644 index 00000000..8c3830c0 --- /dev/null +++ b/tests/test_smart_read_file.py @@ -0,0 +1,123 @@ +"""Tests for the smart_read_file wrapper in timmy.tools. + +Covers the wrapper's behavior for: +- Handling directories (return listing instead of error) +- Accepting `path=` keyword as alias for `file_name=` (the LLM pattern bug fix) +""" + +from unittest.mock import MagicMock + +import pytest + +from timmy.tools import _make_smart_read_file + + +class TestSmartReadFile: + """Test suite for the smart_read_file wrapper function.""" + + @pytest.fixture + def mock_file_tools(self, tmp_path): + """Create a mock FileTools instance for testing. + + - check_escape returns (True, Path) for safe paths + - read_file returns file contents + """ + file_tools = MagicMock() + + def mock_check_escape(file_name): + """Mock implementation that returns safe paths.""" + resolved = tmp_path / file_name + return (True, resolved) + + file_tools.check_escape = mock_check_escape + file_tools.read_file = MagicMock(return_value="file contents") + + return file_tools + + def test_read_file_with_file_name(self, mock_file_tools): + """Test normal call with file_name positional arg.""" + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file("test.txt") + + assert result == "file contents" + mock_file_tools.read_file.assert_called_once_with("test.txt", encoding="utf-8") + + def test_read_file_with_path_kwarg(self, mock_file_tools): + """Test call with path='./config/agents.yaml' (the LLM pattern). + + LLMs often call read_file(path=...) instead of read_file(file_name=...). + The wrapper should accept path= as an alias for file_name=. + """ + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file(path="./config/agents.yaml") + + assert result == "file contents" + mock_file_tools.read_file.assert_called_once_with("./config/agents.yaml", encoding="utf-8") + + def test_read_file_no_args(self, mock_file_tools): + """Test call with no args returns error message.""" + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file() + + assert result == "Error: no file_name or path provided." + mock_file_tools.read_file.assert_not_called() + + def test_read_file_directory(self, mock_file_tools, tmp_path): + """Test call with directory path returns listing. + + When the user (or the LLM) passes a directory path to read_file, + the wrapper detects that case, lists the directory entries, and returns + a helpful message so the model can pick the right file on its own. + """ + # Create a test directory with some files + test_dir = tmp_path / "config" + test_dir.mkdir() + (test_dir / "agents.yaml").write_text("") + (test_dir / "settings.json").write_text("") + (test_dir / "README.md").write_text("") + + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file("config") + + assert "'config' is a directory, not a file." in result + assert "Files inside:" in result + assert " - agents.yaml" in result + assert " - README.md" in result + assert " - settings.json" in result + assert "Please call read_file with one of the files listed above." in result + mock_file_tools.read_file.assert_not_called() + + def test_read_file_empty_directory(self, mock_file_tools, tmp_path): + """Test call with empty directory returns empty directory message.""" + # Create an empty test directory + test_dir = tmp_path / "empty_dir" + test_dir.mkdir() + + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file("empty_dir") + + assert "'empty_dir' is a directory, not a file." in result + assert " (empty directory)" in result + mock_file_tools.read_file.assert_not_called() + + def test_read_file_directory_hides_hidden_files(self, mock_file_tools, tmp_path): + """Test that hidden files (starting with .) are not listed.""" + # Create a test directory with hidden files + test_dir = tmp_path / "config" + test_dir.mkdir() + (test_dir / "visible.txt").write_text("") + (test_dir / ".hidden").write_text("") + (test_dir / ".gitignore").write_text("") + + smart_read_file = _make_smart_read_file(mock_file_tools) + + result = smart_read_file("config") + + assert "visible.txt" in result + assert ".hidden" not in result + assert ".gitignore" not in result