diff --git a/agent/tools/read/read.py b/agent/tools/read/read.py index b9738b4b..9e5a0fec 100644 --- a/agent/tools/read/read.py +++ b/agent/tools/read/read.py @@ -297,8 +297,15 @@ class Read(BaseTool): if offset is not None: if offset < 0: # Negative offset: read from end - # -20 means "last 20 lines" → start from (total - 20) - start_line = max(0, total_file_lines + offset) + # -20 means "last 20 lines" → start from (total - 20). + # A file ending in "\n" produces a trailing empty element + # from split('\n'); exclude it so offset=-1 returns the + # real last line instead of the empty string after the + # final newline (and -N returns N real lines). + effective_lines = total_file_lines + if all_lines and all_lines[-1] == '': + effective_lines -= 1 + start_line = max(0, effective_lines + offset) else: # Positive offset: read from start (1-indexed) start_line = max(0, offset - 1) # Convert to 0-indexed diff --git a/tests/test_read_offset.py b/tests/test_read_offset.py new file mode 100644 index 00000000..97b321bc --- /dev/null +++ b/tests/test_read_offset.py @@ -0,0 +1,58 @@ +# encoding:utf-8 +""" +Regression tests for the Read tool's negative offset ("read from end"). + +The tool documents that a negative offset reads from the end (e.g. -20 for the +last 20 lines). Because content is split on "\n", a file ending in a newline +yields a trailing empty element, so the line count was one too high and every +negative offset was off by one: offset=-1 returned the empty string after the +final newline instead of the last line, and -N returned N-1 real lines. +""" +import os +import sys +import tempfile +import unittest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from agent.tools.read.read import Read + + +class TestReadNegativeOffset(unittest.TestCase): + def setUp(self): + self.work = tempfile.mkdtemp() + self.tool = Read({"cwd": self.work}) + + def _write(self, name, text): + path = os.path.join(self.work, name) + with open(path, "w", encoding="utf-8") as f: + f.write(text) + return path + + def test_offset_minus_one_returns_last_line(self): + path = self._write("f.txt", "line1\nline2\nline3\n") # trailing newline + result = self.tool.execute({"path": path, "offset": -1}) + self.assertEqual(result.status, "success", result.result) + content = result.result["content"] + self.assertIn("line3", content) # was "" before the fix + self.assertNotIn("line2", content) + self.assertNotIn("line1", content) + + def test_offset_minus_two_returns_last_two_lines(self): + path = self._write("f.txt", "line1\nline2\nline3\n") + result = self.tool.execute({"path": path, "offset": -2}) + self.assertEqual(result.status, "success", result.result) + content = result.result["content"] + self.assertIn("line2", content) + self.assertIn("line3", content) + self.assertNotIn("line1", content) + + def test_file_without_trailing_newline_still_works(self): + path = self._write("f.txt", "a\nb\nc") # no trailing newline + result = self.tool.execute({"path": path, "offset": -1}) + self.assertEqual(result.status, "success", result.result) + self.assertEqual(result.result["content"].strip(), "c") + + +if __name__ == "__main__": + unittest.main()