mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-09 13:21:42 +00:00
Port from nearai/ironclaw#5029: graceful char-budget truncation for read_file
read_file previously hard-rejected any read whose formatted output exceeded the ~100K char safety limit, returning an error with zero content. A file with few but very long lines (logs, wide CSV rows, minified data) sails past the line-count limit and then trips the char guard, so the model gets nothing and must guess a smaller limit — wasting a full round-trip. Now the read is trimmed to the last complete line that fits the budget and returns the partial content plus truncated_by="bytes" and a next_offset, so the model paginates forward instead of starting over. A single line larger than the whole budget is clamped on a code-point boundary (never empty) and the cursor still advances. Applies at both read paths (normal + extracted documents). Adapted from IronClaw's Rust dual line/byte cap to hermes's Python tool-layer char guard, which is the single uniform chokepoint over the gutter-rendered content for every backend.
This commit is contained in:
parent
2f2e60801c
commit
25f0cecf5e
2 changed files with 177 additions and 40 deletions
|
|
@ -226,7 +226,8 @@ class TestDevicePathBlocking(unittest.TestCase):
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCharacterCountGuard(unittest.TestCase):
|
||||
"""Large reads should be rejected with guidance to use offset/limit."""
|
||||
"""Oversized reads are truncated on a line boundary (nearai/ironclaw#5029),
|
||||
not rejected — the model gets the head of the file plus a next_offset."""
|
||||
|
||||
def setUp(self):
|
||||
_read_tracker.clear()
|
||||
|
|
@ -235,28 +236,54 @@ class TestCharacterCountGuard(unittest.TestCase):
|
|||
_read_tracker.clear()
|
||||
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
@patch("tools.file_tools._get_max_read_chars", return_value=_DEFAULT_MAX_READ_CHARS)
|
||||
def test_oversized_read_rejected(self, _mock_limit, mock_ops):
|
||||
"""A read that returns >max chars is rejected."""
|
||||
big_content = "x" * (_DEFAULT_MAX_READ_CHARS + 1)
|
||||
@patch("tools.file_tools._get_max_read_chars", return_value=1000)
|
||||
def test_oversized_multiline_read_truncated_with_continuation(self, _mock_limit, mock_ops):
|
||||
"""A read whose many lines exceed the char budget is trimmed to the
|
||||
last complete line and offers a next_offset, instead of returning an
|
||||
error with no content."""
|
||||
# 50 lines of 100 chars each = ~5050 chars, well over the 1000 budget.
|
||||
big_content = "\n".join(f"{i}|" + "z" * 98 for i in range(1, 51))
|
||||
mock_ops.return_value = _make_fake_ops(
|
||||
content=big_content,
|
||||
total_lines=5000,
|
||||
file_size=len(big_content) + 100, # bigger than content
|
||||
total_lines=50,
|
||||
file_size=len(big_content),
|
||||
)
|
||||
result = json.loads(read_file_tool("/tmp/huge.txt", task_id="big"))
|
||||
self.assertIn("error", result)
|
||||
self.assertIn("safety limit", result["error"])
|
||||
self.assertIn("offset and limit", result["error"])
|
||||
self.assertIn("total_lines", result)
|
||||
# No hard rejection — content is present.
|
||||
self.assertNotIn("error", result)
|
||||
self.assertIn("content", result)
|
||||
self.assertTrue(result["content"])
|
||||
# Truncation metadata for the model to paginate.
|
||||
self.assertTrue(result["truncated"])
|
||||
self.assertEqual(result["truncated_by"], "bytes")
|
||||
self.assertIn("next_offset", result)
|
||||
self.assertGreater(result["next_offset"], 1)
|
||||
# Body fits the budget (allowing for redaction not growing it).
|
||||
self.assertLessEqual(len(result["content"]), 1000)
|
||||
self.assertIn("offset", result["hint"])
|
||||
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
def test_small_read_not_rejected(self, mock_ops):
|
||||
"""Normal-sized reads pass through fine."""
|
||||
@patch("tools.file_tools._get_max_read_chars", return_value=1000)
|
||||
def test_single_oversized_line_clamped_not_empty(self, _mock_limit, mock_ops):
|
||||
"""A single line larger than the whole budget is clamped (never empty)
|
||||
and the cursor still advances by one line."""
|
||||
big_content = "1|" + "q" * 5000 # one line, no newline, > budget
|
||||
mock_ops.return_value = _make_fake_ops(
|
||||
content=big_content, total_lines=1, file_size=len(big_content),
|
||||
)
|
||||
result = json.loads(read_file_tool("/tmp/oneline.txt", task_id="oneline"))
|
||||
self.assertNotIn("error", result)
|
||||
self.assertTrue(result["content"]) # not empty
|
||||
self.assertEqual(result["next_offset"], 2) # advanced past line 1
|
||||
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
def test_small_read_not_truncated(self, mock_ops):
|
||||
"""Normal-sized reads pass through fine with no truncation flag."""
|
||||
mock_ops.return_value = _make_fake_ops(content="short\n", file_size=6)
|
||||
result = json.loads(read_file_tool("/tmp/small.txt", task_id="small"))
|
||||
self.assertNotIn("error", result)
|
||||
self.assertIn("content", result)
|
||||
self.assertNotEqual(result.get("truncated_by"), "bytes")
|
||||
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
@patch("tools.file_tools._get_max_read_chars", return_value=_DEFAULT_MAX_READ_CHARS)
|
||||
|
|
@ -271,6 +298,49 @@ class TestCharacterCountGuard(unittest.TestCase):
|
|||
self.assertIn("content", result)
|
||||
|
||||
|
||||
class TestTruncateToCharBudget(unittest.TestCase):
|
||||
"""Unit tests for the line-boundary char-budget trimmer."""
|
||||
|
||||
def _fn(self):
|
||||
from tools.file_tools import _truncate_to_char_budget
|
||||
return _truncate_to_char_budget
|
||||
|
||||
def test_fits_unchanged(self):
|
||||
fn = self._fn()
|
||||
text = "1|a\n2|b\n3|c"
|
||||
out, lines, trunc = fn(text, 1000)
|
||||
self.assertEqual(out, text)
|
||||
self.assertEqual(lines, 3)
|
||||
self.assertFalse(trunc)
|
||||
|
||||
def test_trims_on_line_boundary(self):
|
||||
fn = self._fn()
|
||||
# 3 lines of 10 chars; budget fits ~2 lines.
|
||||
text = "\n".join("x" * 10 for _ in range(5)) # 5 lines, 54 chars
|
||||
out, lines, trunc = fn(text, 25)
|
||||
self.assertTrue(trunc)
|
||||
# Output ends on a complete line (no partial line at the tail).
|
||||
self.assertFalse(out.endswith("x" * 3) and len(out.split("\n")[-1]) != 10)
|
||||
self.assertEqual(lines, out.count("\n") + 1)
|
||||
self.assertLessEqual(len(out), 25)
|
||||
|
||||
def test_single_line_over_budget_clamped(self):
|
||||
fn = self._fn()
|
||||
text = "y" * 500 # single line, no newline
|
||||
out, lines, trunc = fn(text, 100)
|
||||
self.assertTrue(trunc)
|
||||
self.assertEqual(lines, 1)
|
||||
self.assertEqual(len(out), 100) # clamped to budget
|
||||
self.assertNotEqual(out, "") # never empty
|
||||
|
||||
def test_empty_content(self):
|
||||
fn = self._fn()
|
||||
out, lines, trunc = fn("", 100)
|
||||
self.assertEqual(out, "")
|
||||
self.assertEqual(lines, 0)
|
||||
self.assertFalse(trunc)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# File deduplication
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -711,12 +781,15 @@ class TestConfigOverride(unittest.TestCase):
|
|||
@patch("tools.file_tools._get_file_ops")
|
||||
@patch("hermes_cli.config.load_config", return_value={"file_read_max_chars": 50})
|
||||
def test_custom_config_lowers_limit(self, _mock_cfg, mock_ops):
|
||||
"""A config value of 50 should reject reads over 50 chars."""
|
||||
"""A config value of 50 should trigger truncation for reads over 50 chars,
|
||||
with the configured limit reflected in the continuation hint."""
|
||||
mock_ops.return_value = _make_fake_ops(content="x" * 60, file_size=60)
|
||||
result = json.loads(read_file_tool("/tmp/cfgtest.txt", task_id="cfg1"))
|
||||
self.assertIn("error", result)
|
||||
self.assertIn("safety limit", result["error"])
|
||||
self.assertIn("50", result["error"]) # should show the configured limit
|
||||
self.assertNotIn("error", result)
|
||||
self.assertTrue(result["truncated"])
|
||||
self.assertEqual(result["truncated_by"], "bytes")
|
||||
self.assertIn("50", result["hint"]) # should show the configured limit
|
||||
self.assertLessEqual(len(result["content"]), 50)
|
||||
|
||||
@patch("tools.file_tools._get_file_ops")
|
||||
@patch("hermes_cli.config.load_config", return_value={"file_read_max_chars": 500_000})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue