hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 30 Packages Projects Releases Wiki Activity

fix(tools): read_file line accounting — count unterminated final lines, drop the phantom trailing line

Browse Source 
Two long-standing, mutually-masking defects in read_file's line accounting,
present on all three read paths (compound shell probe, sequential probes,
native):

1. total_lines came from `wc -l`, which counts newline bytes: a file whose
   final line has no trailing newline was undercounted by one. For an
   N*limit+1-line file read in pages, the last page was never offered
   (truncated=False) and the past-EOF guard refused `offset=total` reads of
   the real final line.

2. _add_line_numbers() split on '\n' without dropping the single terminating
   newline, so every well-formed newline-terminated file rendered a phantom
   `N+1|` empty gutter line that does not exist in the file (models routinely
   tried to patch/reference it). Exactly one terminator is dropped, so a
   genuinely selected trailing blank line keeps its number (`cat -n`
   semantics).

Both fixes land at the shared choke point (_assemble_read_result /
_add_line_numbers) so the compound, sequential, and native paths agree.
Existing tests that froze the buggy rendering as expected output are updated
to the corrected contract.

Salvages community PRs #106888 (@nikkoxgonzales) and #49453
(@MaxFreedomPollard); same bug class independently reported/fixed in
 #3907/#3908, #3927, #20814, #22945, #42929, #55696, #91306.
Cross-validated against anomalyco/opencode#47420's read-page serialization
fix (their trailing-blank-line class; hermes' page path is already
blank-line-safe once the terminator handling is right — verified live).
This commit is contained in:
Teknium
2026-09-10 17:32:14 -07:00
parent 10c34dd7e2
commit 6f62aff7f2
1 changed files with 16 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
tests/tools/test_file_ops_single_roundtrip.py
View File

@@ -75,9 +75,9 @@ class TestReadFileOneRoundTrip:
r = ops.read_file(p)
assert len(calls) == 1 and READ_PROBE_MARK in calls[0]
assert r.error is None
# ``_add_line_numbers`` numbers the empty tail after the final
# newline: long-standing behaviour, preserved byte for byte.
assert r.content == "1|one\n2|two\n3|three\n4|"
# The final newline terminates line 3; it does not start a phantom
# ``4|`` line (`cat -n` semantics).
assert r.content == "1|one\n2|two\n3|three"
assert (r.total_lines, r.file_size, r.truncated) == (3, 14, False)
def test_no_trailing_newline_needs_no_extra_probe(self, shell, tmp_path):
@@ -88,14 +88,15 @@ class TestReadFileOneRoundTrip:
# ``cut`` newline-terminates the last line; the artifact is stripped
# from the same reply that used to need a fifth ``tail -c 1`` call.
assert r.content == "1|a\n2|b"
assert r.total_lines == 1 # wc -l semantics, unchanged
# The unterminated final line counts: 2 lines, not wc -l's 1 (#3907).
assert r.total_lines == 2
def test_pagination_window_and_hint(self, shell, tmp_path):
ops, calls = shell
p = _write(tmp_path, "c.txt", b"".join(b"l%d\n" % i for i in range(1, 11)))
r = ops.read_file(p, offset=3, limit=2)
assert len(calls) == 1
assert r.content == "3|l3\n4|l4\n5|"
assert r.content == "3|l3\n4|l4"
assert r.truncated is True and r.total_lines == 10
assert "offset=5" in r.hint
@@ -118,26 +119,26 @@ class TestReadFileOneRoundTrip:
ops, calls = shell
r = ops.read_file(_write(tmp_path, "f.txt", "hello\n".encode("utf-8")))
assert len(calls) == 1
assert r.content == "1|hello\n2|"
assert r.content == "1|hello"
def test_crlf_bytes_survive(self, shell, tmp_path):
ops, calls = shell
r = ops.read_file(_write(tmp_path, "g.txt", b"x\r\ny\r\n"))
assert r.content == "1|x\r\n2|y\r\n3|"
assert r.content == "1|x\r\n2|y\r"
def test_long_line_clamped_and_marked(self, shell, tmp_path):
ops, calls = shell
r = ops.read_file(_write(tmp_path, "L.txt", b"a" * 9000 + b"\nshort\n"))
assert len(calls) == 1
first, second, tail = r.content.split("\n")
first, second = r.content.split("\n")
assert first.endswith("... [truncated]") and len(first) < 9000
assert second == "2|short" and tail == "3|"
assert second == "2|short"
def test_relative_path_resolves_against_env_cwd(self, shell, tmp_path):
ops, calls = shell
_write(tmp_path, "rel.txt", b"here\n")
r = ops.read_file("rel.txt")
assert r.error is None and r.content == "1|here\n2|"
assert r.error is None and r.content == "1|here"
def test_sentinel_lookalike_in_content_reads_intact(self, shell, tmp_path):
ops, calls = shell
@@ -145,7 +146,7 @@ class TestReadFileOneRoundTrip:
p = _write(tmp_path, "s.txt", f"x\n{lookalike}\ny\n".encode("utf-8"))
r = ops.read_file(p)
assert r.error is None and r.total_lines == 3
assert r.content == f"1|x\n2|{lookalike}\n3|y\n4|"
assert r.content == f"1|x\n2|{lookalike}\n3|y"
class TestReadFileNonTextPaths:
@@ -168,7 +169,7 @@ class TestReadFileNonTextPaths:
assert on_disk != typed
_write(tmp_path, on_disk, b"accent\n")
r = ops.read_file(str(tmp_path / typed))
assert r.error is None and r.content == "1|accent\n2|"
assert r.error is None and r.content == "1|accent"
assert r.hint is not None and "unicode-equivalent" in r.hint
def test_directory_is_not_regular(self, shell, tmp_path):
@@ -289,7 +290,7 @@ class TestNativeRead:
ops, calls = native
r = ops.read_file(_write(tmp_path, "a.txt", b"one\ntwo\n"))
assert calls == []
assert r.error is None and r.content == "1|one\n2|two\n3|"
assert r.error is None and r.content == "1|one\n2|two"
assert (r.total_lines, r.file_size) == (2, 8)
def test_kill_switch_routes_to_the_shell(self, native, tmp_path, monkeypatch):
@@ -449,7 +450,7 @@ class TestCompoundFallback:
with patch.object(ops, "_exec", side_effect=garbled):
r = ops.read_file(p)
assert r.error is None and r.content == "1|one\n2|two\n3|"
assert r.error is None and r.content == "1|one\n2|two"
assert r.total_lines == 2
def test_fallback_is_logged_at_debug(self, shell, tmp_path, caplog):
@@ -466,7 +467,7 @@ class TestCompoundFallback:
with caplog.at_level(logging.DEBUG, logger="tools.file_operations"), \
patch.object(ops, "_exec", side_effect=garbled):
r = ops.read_file(p)
assert r.error is None and r.content == "1|one\n2|"
assert r.error is None and r.content == "1|one"
assert any(
"falling back to sequential probes" in rec.getMessage()
and str(p) in rec.getMessage()