From fcc520df477ae7f1876d24b3071207312c2d10f5 Mon Sep 17 00:00:00 2001
From: xiaweiwei67-stack <293320877+xiaweiwei67-stack@users.noreply.github.com>
Date: Tue, 7 Jul 2026 16:53:35 +0800
Subject: [PATCH] fix(read): correct off-by-one for negative offset with
 trailing newline

The Read tool documents that a negative offset reads from the end (-N = last N lines). Content is split on newline, so a file ending in a newline produces a trailing empty element and total_file_lines is one too high. Every negative offset was therefore 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.

Exclude the trailing empty element when computing the start line for negative offsets. Adds regression tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 agent/tools/read/read.py  | 11 ++++++--
 tests/test_read_offset.py | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 tests/test_read_offset.py

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()