emrg: the read tool's directory branch is a read, and is held to the same limits

emrg/tools/read_tool.py

@@ -50,7 +50,10 @@ def definition(self) -> ToolDefinition:
                 "Read a file from the filesystem. Returns content with "
                 "line numbers prefixing each line (format: '  LINE_NUMBER\\tCONTENT'). "
                 "Supports start_line and line_limit for reading large files in chunks. "
-                "Can read text files. For images (.png/.jpg/.jpeg/.gif/.webp), "
+                "Can read text files. A directory is listed rather than read, and the "
+                "listing is held to the same limits — the default cap is "
+                f"{DEFAULT_MAX_LINES} entries and a cut listing says so. "
+                "For images (.png/.jpg/.jpeg/.gif/.webp), "
                 "returns a vision-format image block so the model can see the picture "
                 "(requires a vision-capable model; otherwise a text placeholder is "
                 "returned). For PDFs and notebooks, use the bash tool with appropriate "
@@ -153,13 +156,77 @@ async def execute(self, arguments: dict) -> ToolResult:
                 error=True,
             )
 
+        # Compute effective limit — two tiers:
+        #   Default (no limit specified): capped at DEFAULT_MAX_LINES to
+        #     prevent excessive token consumption from unknown file sizes.
+        #   Explicit limit: honored up to MAX_LINES (LLM knows what it asked for).
+        # Resolved here rather than beside the file branch it used to sit next to,
+        # because the directory branch below is a read too and is held to the same
+        # two tiers: a limit that binds a file and is dropped for a listing is the
+        # "identical output whether or not you bounded it" defect this branch fixed
+        # (issue #2059, measured 2026-10-10 in `cyc20261010-211559`: a 6,000-entry
+        # directory answered `line_limit=1` with the same 108,112 characters and
+        # 6,001 lines as a read with no arguments at all).
+        if line_limit is not None:
+            effective_limit = min(line_limit, MAX_LINES)
+        else:
+            effective_limit = DEFAULT_MAX_LINES
+
         if path.is_dir():
-            # Directory listing
+            # ── A listing is a read, and is read under the same limits (#2059) ──
+            #
+            # It used to return every entry whatever the caller passed, so the three
+            # numeric parameters were accepted and silently ignored and the output was
+            # unbounded: `DEFAULT_MAX_LINES` stopped a 6,000-line *file* at 1,000 lines
+            # with a continuation note, and the same call on a *directory* returned all
+            # 6,000 entries with nothing said. The module's own promise — "Default
+            # limits prevent oversized tool results" — held for one and not the other.
             entries = sorted(path.iterdir(), key=lambda p: (p.is_file(), p.name))
+            total_entries = len(entries)
+
+            if total_entries == 0:
+                # An empty subject is not a clean one (issue #1872), and the header
+                # alone read as a broken listing rather than an empty directory. This
+                # is the file branch's `(empty file: {path})`, at the branch that was
+                # missing it.
+                return ToolResult(name="read", content=f"(empty directory: {path})")
+
+            start = start_line - 1
+            end = min(start + effective_limit, total_entries)
+            selected = entries[start:end]
+
+            if not selected:
+                return ToolResult(
+                    name="read",
+                    content=(
+                        f"(no entries: start_line={start_line} is past the end of the "
+                        f"listing, which has {total_entries} "
+                        f"entr{'y' if total_entries == 1 else 'ies'})"
+                    ),
+                )
+
             lines: list[str] = [f"Directory listing for {path}/:", ""]
-            for e in entries:
+            for e in selected:
                 suffix = "/" if e.is_dir() else ""
                 lines.append(f"  {e.name}{suffix}")
+
+            if start_line_byte_offset > 0:
+                # Reported rather than accepted in silence. The offset names a position
+                # *inside a line* and exists for resuming one very long line; a listing's
+                # lines hold one entry name each, so there is nothing here to resume —
+                # and dropping it without a word is the defect this branch is fixing.
+                lines.append(
+                    f"\nnote: start_line_byte_offset={start_line_byte_offset} is not "
+                    "applied to a directory listing — it names a position inside a line, "
+                    "and each line of a listing holds one entry name"
+                )
+
+            if end < total_entries:
+                lines.append(
+                    f"\ntruncated at start_line={end + 1}, "
+                    f"start_line_byte_offset=0 — "
+                    f"total {total_entries} entries"
+                )
             return ToolResult(name="read", content="\n".join(lines))
 
         file_size = path.stat().st_size
@@ -238,15 +305,6 @@ async def execute(self, arguments: dict) -> ToolResult:
         if total_lines == 0:
             return ToolResult(name="read", content=f"(empty file: {path})")
 
-        # Compute effective limit — two tiers:
-        #   Default (no limit specified): capped at DEFAULT_MAX_LINES to
-        #     prevent excessive token consumption from unknown file sizes.
-        #   Explicit limit: honored up to MAX_LINES (LLM knows what it asked for).
-        if line_limit is not None:
-            effective_limit = min(line_limit, MAX_LINES)
-        else:
-            effective_limit = DEFAULT_MAX_LINES
-
         start = start_line - 1
         end = min(start + effective_limit, total_lines)
         selected = all_lines[start:end]

tests/test_read_tool.py

@@ -6,7 +6,7 @@
 
 import pytest
 
-from emrg.tools.read_tool import ReadTool
+from emrg.tools.read_tool import DEFAULT_MAX_LINES, ReadTool
 
 
 @pytest.fixture
@@ -94,6 +94,146 @@ def test_read_directory(temp_file):
     assert "test.txt" in result.content
 
 
+class TestAReadIsBoundedWhicheverSubjectItHas:
+    """A listing is a read, so the limits bind it too (issue #2059).
+
+    Measured 2026-10-10 (`cyc20261010-211559`): the directory branch returned every
+    entry whatever the caller passed — `line_limit=1` on a 6,000-entry directory
+    produced a byte-identical 108,112-character result to a read with no arguments,
+    while `DEFAULT_MAX_LINES` stopped a 6,000-line *file* at 1,000 lines and named the
+    continuation. A limit that binds one subject and is dropped for the other is the
+    "identical output whether or not you bounded it" defect, and the caller could not
+    tell that its bound had been ignored.
+    """
+
+    @staticmethod
+    def _dir(tmp_path, count):
+        d = tmp_path / "listing"
+        d.mkdir()
+        for i in range(count):
+            (d / f"entry-{i:05d}.txt").write_text("")
+        return d
+
+    @staticmethod
+    def _entries(content):
+        return [line for line in content.splitlines() if line.startswith("  ")]
+
+    def test_a_listing_honours_an_explicit_limit(self, tmp_path):
+        d = self._dir(tmp_path, 40)
+        tool = ReadTool()
+        whole = _run(tool.execute({"file_path": str(d)}))
+        limited = _run(tool.execute({"file_path": str(d), "line_limit": 2}))
+        assert len(limited.content) < len(whole.content), (
+            "the limit did not shrink the listing, so a caller who bounded the read "
+            "cannot tell whether its bound was applied"
+        )
+        assert len(self._entries(limited.content)) == 2, limited.content
+
+    def test_a_listing_is_capped_by_default_and_names_its_size(self, tmp_path):
+        d = self._dir(tmp_path, DEFAULT_MAX_LINES + 10)
+        tool = ReadTool()
+        result = _run(tool.execute({"file_path": str(d)}))
+        assert len(self._entries(result.content)) == DEFAULT_MAX_LINES, (
+            "the default cap must bind a listing exactly as it binds a file"
+        )
+        assert f"total {DEFAULT_MAX_LINES + 10} entries" in result.content, result.content
+
+    def test_a_listing_says_where_to_continue(self, tmp_path):
+        d = self._dir(tmp_path, 30)
+        tool = ReadTool()
+        result = _run(tool.execute({"file_path": str(d), "line_limit": 10}))
+        assert "truncated at start_line=11, start_line_byte_offset=0" in result.content, (
+            result.content
+        )
+        assert "total 30 entries" in result.content, result.content
+
+    def test_a_start_line_selects_the_entry_to_start_from(self, tmp_path):
+        d = self._dir(tmp_path, 30)
+        tool = ReadTool()
+        result = _run(tool.execute({"file_path": str(d), "start_line": 29, "line_limit": 5}))
+        entries = self._entries(result.content)
+        assert entries == ["  entry-00028.txt", "  entry-00029.txt"], entries
+
+    def test_the_offset_is_reported_as_not_applied_to_a_listing(self, tmp_path):
+        """Reported, not accepted in silence — the defect this branch is fixing.
+
+        `start_line_byte_offset` names a position inside a *line* and exists for resuming
+        one very long line. A listing's lines hold one entry name each, so there is
+        nothing here to resume; dropping it without a word is what the branch used to do
+        to all three parameters.
+        """
+        d = self._dir(tmp_path, 3)
+        tool = ReadTool()
+        result = _run(tool.execute({
+            "file_path": str(d), "line_limit": 1, "start_line_byte_offset": 2,
+        }))
+        assert "start_line_byte_offset=2 is not applied" in result.content, result.content
+
+    def test_a_start_line_past_the_end_is_reported(self, tmp_path):
+        d = self._dir(tmp_path, 3)
+        tool = ReadTool()
+        result = _run(tool.execute({"file_path": str(d), "start_line": 9}))
+        assert "no entries" in result.content, result.content
+        assert "which has 3 entries" in result.content, result.content
+
+    def test_an_empty_directory_is_not_a_clean_reading(self, tmp_path):
+        """The header alone read as a broken listing rather than an empty one."""
+        d = tmp_path / "nothing"
+        d.mkdir()
+        tool = ReadTool()
+        result = _run(tool.execute({"file_path": str(d)}))
+        assert result.content == f"(empty directory: {d})", result.content
+
+    def test_a_one_entry_listing_past_the_end_is_counted_in_the_singular(self, tmp_path):
+        """The singular half of the message above, which no test reached.
+
+        `f"entr{'y' if total_entries == 1 else 'ies'}"` is a branch, and a branch needs a
+        reading: the plural side is covered by `test_a_start_line_past_the_end_is_reported`,
+        and replacing the whole spelling with the bare plural left every test in this file
+        green (measured 2026-10-10) — so the sentence a one-entry listing really produces
+        was pinned by nothing, and a later edit could have made it say "1 entries" without
+        a red anywhere.
+        """
+        d = self._dir(tmp_path, 1)
+        result = _run(ReadTool().execute({"file_path": str(d), "start_line": 2}))
+        assert "which has 1 entry" in result.content, result.content
+        assert "1 entries" not in result.content, result.content
+
+    def test_the_sentence_about_a_listing_is_the_one_the_tool_keeps(self, tmp_path):
+        """The description sentence this class arrived with, read against its behaviour.
+
+        A sentence in a tool description is a claim about behaviour, so it is pinned
+        against the behaviour it claims rather than against its own words —
+        `tests/test_glob_tool.py` states the lesson (*"a promise in a tool description
+        that no test holds is how the universal claim above survived review"*) and
+        `tests/test_read_tool.py::test_read_never_cuts_a_line` is this file's precedent
+        for the same shape, added because a cap introduced later would quietly re-define
+        what its sentence said.
+
+        Measured 2026-10-10: the sentence was **new here and read by nothing** — dropping
+        its promise ("and a cut listing says so") left all 42 tests green, and so did
+        naming a number other than the constant, because the number is interpolated and
+        nothing asked whether the interpolation was still there. The two description
+        assertions below are the ones no other test makes; the two behaviour assertions
+        are the sentence's own claim, in the same reading, so neither half can drift
+        alone.
+        """
+        description = ReadTool().definition().description
+        assert f"{DEFAULT_MAX_LINES} entries" in description, description
+        assert "a cut listing says so" in description, description
+
+        d = self._dir(tmp_path, DEFAULT_MAX_LINES + 5)
+        content = _run(ReadTool().execute({"file_path": str(d)})).content
+        assert len(self._entries(content)) == DEFAULT_MAX_LINES, (
+            "the number the description names is not the number the tool caps at: "
+            f"{content[-200:]!r}"
+        )
+        assert "truncated at" in content, (
+            "'a cut listing says so' must be a sentence the renderer keeps: "
+            f"{content[-200:]!r}"
+        )
+
+
 def test_read_binary_fails(temp_file):
     tool = ReadTool()
     _, d = temp_file