fix(agent): snap tool-output tail forward so fallback truncation respects max_chars (#4017)

* fix(agent): snap tool-output tail forward so fallback truncation respects max_chars

_snap_to_line_boundary() moves an offset backwards to the preceding newline.
That is correct for the head's end offset, but _build_fallback() also applied it
to the tail's start offset, where moving backwards lengthens the tail. On output
whose last newline sits in the second half (log lines followed by one long
unbroken line: minified JSON, base64 artifacts), the returned string exceeded
max_chars by up to 17x, defeating the guard that exists to stop a single large
tool result from blowing the model context.

Add _snap_start_to_line_boundary(), the forward-snapping mirror, and use it for
the tail offset. The existing head_end guard becomes redundant because a forward
snap can only move the tail start away from the head.

test_result_never_exceeds_max_chars already asserted this invariant but passed
newline-free content, so the snapping branch was never exercised.

* test(agent): pin the forward-snap direction of the fallback tail

The bound test only asserts that the result fits max_chars. Its content has
no newline inside the tail's snap window, so _snap_start_to_line_boundary
returns pos unchanged and the test stays green even with the snap removed.

Place a newline in the window so the snap has to fire: the tail must begin
after it, which the pre-fix backward snap does not do.
This commit is contained in:
Aari
2026-07-10 08:07:25 +08:00
committed by GitHub
parent ebc09ce130
commit 3be3969f8f
2 changed files with 84 additions and 4 deletions
@@ -77,6 +77,9 @@ def _snap_to_line_boundary(text: str, pos: int) -> int:
Used so that previews and truncations end on a complete line when
possible. If no newline exists in the second half of ``text[:pos]``
the original *pos* is returned unchanged.
Only valid for an *end* offset: moving backwards shortens the slice that
ends here. Use :func:`_snap_start_to_line_boundary` for a start offset.
"""
if pos <= 0 or pos >= len(text):
return pos
@@ -87,6 +90,23 @@ def _snap_to_line_boundary(text: str, pos: int) -> int:
return pos
def _snap_start_to_line_boundary(text: str, pos: int) -> int:
"""Return *pos* or the nearest following newline+1, whichever is closer.
The start-offset mirror of :func:`_snap_to_line_boundary`. Snapping a start
backwards would *lengthen* the slice beginning there, so the tail of a
budgeted preview must snap forward instead. If no newline exists in the
first half of ``text[pos:]`` the original *pos* is returned unchanged.
"""
if pos <= 0 or pos >= len(text):
return pos
half = pos + (len(text) - pos) // 2
nl = text.find("\n", pos, half)
if nl >= 0:
return nl + 1
return pos
# ---------------------------------------------------------------------------
# Disk persistence
# ---------------------------------------------------------------------------
@@ -258,10 +278,7 @@ def _build_fallback(
effective_tail = min(tail_chars, max(0, budget - effective_head))
head_end = _snap_to_line_boundary(content, min(effective_head, total))
tail_start = max(head_end, total - effective_tail)
tail_start_snapped = _snap_to_line_boundary(content, tail_start)
if tail_start_snapped > head_end:
tail_start = tail_start_snapped
tail_start = _snap_start_to_line_boundary(content, max(head_end, total - effective_tail))
head = content[:head_end]
tail = content[tail_start:] if tail_start < total else ""
@@ -27,6 +27,7 @@ from deerflow.agents.middlewares.tool_output_budget_middleware import (
_needs_budget,
_patch_model_messages,
_sanitize_tool_name,
_snap_start_to_line_boundary,
_snap_to_line_boundary,
_tool_message_over_budget,
)
@@ -39,6 +40,20 @@ from deerflow.config.tool_output_config import ToolOutputConfig
# ---------------------------------------------------------------------------
def _lines_then_long_line(total: int, newline_ratio: float = 0.6) -> str:
"""Content that is line-oriented for the first *newline_ratio*, then one unbroken line.
Mirrors real bash/web_fetch output that logs progress lines and then dumps a
single-line artifact (minified JSON, base64 blob). The last newline lands in
the second half of the content, which is what exercises line snapping around
the tail offset.
"""
head_len = int(total * newline_ratio)
lines = "".join(f"[info] step {i} ok\n" for i in range(head_len // 18 + 1))[:head_len]
lines = lines[:-1] + "\n" if not lines.endswith("\n") else lines
return lines + "A" * (total - len(lines))
def _make_request(tool_name: str = "remote_executor", tool_call_id: str = "tc-1", outputs_path: str | None = None) -> SimpleNamespace:
thread_data = {"outputs_path": outputs_path} if outputs_path else None
state = {"thread_data": thread_data} if thread_data else {}
@@ -99,6 +114,28 @@ class TestSnapToLineBoundary:
assert _snap_to_line_boundary("abc", 10) == 10
class TestSnapStartToLineBoundary:
def test_snaps_forward_to_newline(self):
text = "line1\nline2\nline3"
result = _snap_start_to_line_boundary(text, 2) # inside "line1"
assert text[result - 1] == "\n"
assert result >= 2
def test_never_moves_backwards(self):
text = "aaaa\n" + "b" * 20
for pos in range(1, len(text)):
assert _snap_start_to_line_boundary(text, pos) >= pos
def test_no_snap_when_no_newline_in_range(self):
assert _snap_start_to_line_boundary("abcdefghij", 2) == 2
def test_zero_pos(self):
assert _snap_start_to_line_boundary("a\nbc", 0) == 0
def test_pos_beyond_length(self):
assert _snap_start_to_line_boundary("abc", 10) == 10
class TestExternalize:
def test_writes_file_and_returns_virtual_path(self):
with tempfile.TemporaryDirectory() as tmpdir:
@@ -281,6 +318,32 @@ class TestBuildFallback:
result = _build_fallback(content, tool_name="long_tool_name", max_chars=max_chars, head_chars=max_chars // 2, tail_chars=max_chars // 4)
assert len(result) <= max_chars, f"max_chars={max_chars}: got {len(result)}"
def test_result_never_exceeds_max_chars_with_newlines(self):
"""Same guarantee as above, on content that actually exercises line snapping.
``test_result_never_exceeds_max_chars`` passes newline-free content, so the
tail offset is never snapped. Real bash/web_fetch output has newlines.
"""
for total in [50_000, 200_000, 1_000_000]:
content = _lines_then_long_line(total)
result = _build_fallback(content, tool_name="bash", max_chars=30_000, head_chars=8_000, tail_chars=3_000)
assert len(result) <= 30_000, f"total={total}: got {len(result)}"
def test_fallback_forward_snaps_tail_onto_line_boundary(self):
"""The tail must begin *after* the newline, never before it.
The bound test above never moves the tail offset: its content has no
newline inside the snap window, so it would pass even with the snap
removed. Placing a newline in the window pins the direction instead —
a backward snap leaves the tail starting mid-line.
"""
total, newline_pos = 100_000, 98_000 # window is [97_000, 98_500)
content = "A" * newline_pos + "\n" + "B" * (total - newline_pos - 1)
result = _build_fallback(content, tool_name="bash", max_chars=30_000, head_chars=8_000, tail_chars=3_000)
assert len(result) <= 30_000
tail = result.rsplit("]\n\n", 1)[1]
assert tail.startswith("B"), f"tail begins mid-line: {tail[:20]!r}"
def test_very_small_max_chars_does_not_crash(self):
content = "x" * 1000
result = _build_fallback(content, tool_name="t", max_chars=50, head_chars=20, tail_chars=10)