refactor(context): trim verbose comments

This commit is contained in:
Ahmed Allam 2026-07-26 19:26:51 +00:00
parent 95046a6cea
commit 10376b412b
6 changed files with 14 additions and 59 deletions

View file

@ -124,12 +124,7 @@ def _format_tool_error(exc: Exception) -> str:
def _with_bounded_result(tool: FunctionTool) -> FunctionTool:
"""Cap the size of a tool's result before it enters agent history.
Idempotent: base tools are shared singletons reused across every agent, so
the guard prevents stacking the wrapper on repeated ``build_strix_agent``
calls.
"""
"""Cap a tool's result size before it enters history (idempotent)."""
if getattr(tool, "_strix_bounded", False):
return tool
invoke_tool = tool.on_invoke_tool
@ -195,13 +190,7 @@ def _custom_tool_as_function_tool(tool: CustomTool) -> FunctionTool:
def _bound_custom_tool(tool: CustomTool) -> CustomTool:
"""Bound a native ``CustomTool`` result in place.
Chat-completions mode converts filesystem ``CustomTool``s to ``FunctionTool``s
(which bounds the result), but the Responses path keeps them native, so a
large ``read_file``/directory listing would otherwise append unbounded text
to history. Wrap ``on_invoke_tool`` so the same head+tail bound applies.
"""
"""Bound a native ``CustomTool`` result in place (Responses path)."""
invoke_tool = tool.on_invoke_tool
async def invoke(ctx: Any, raw_input: str) -> Any:
@ -218,8 +207,6 @@ def _configure_filesystem_tools(toolset: Any, *, chat_completions: bool) -> None
setattr(toolset, name, _custom_tool_as_function_tool(tool))
elif isinstance(tool, FunctionTool):
setattr(toolset, name, _function_tool_with_error_result(tool))
# Responses-API path: keep tools native but still bound their output so
# filesystem reads can't exhaust the context window on later turns.
elif isinstance(tool, CustomTool):
setattr(toolset, name, _bound_custom_tool(tool))
elif isinstance(tool, FunctionTool):
@ -272,13 +259,8 @@ def _format_validation_error(tool_name: str, exc: ValidationError) -> str:
def _apply_shell_output_cap(parsed: dict[str, Any]) -> None:
"""Bound the SDK shell tools' own token cap so a single command can't dump
unbounded output into history. The SDK truncates head+tail from this value.
The configured cap is a ceiling: a missing value defaults to it, and a
larger model-supplied value is clamped down to it. A smaller explicit value
is respected, so the model can still ask for less.
"""
"""Clamp the SDK shell tools' ``max_output_tokens`` to the configured
ceiling; a smaller explicit value is respected."""
ceiling = load_settings().context.tool_output_max_tokens
requested = parsed.get("max_output_tokens")
parsed["max_output_tokens"] = (

View file

@ -69,8 +69,7 @@ class ContextSettings(BaseSettings):
summary_max_tokens: int = Field(default=4_096, gt=0, alias="STRIX_CONTEXT_SUMMARY_TOKENS")
tool_output_max_tokens: int = Field(default=8_000, gt=0, alias="STRIX_TOOL_OUTPUT_MAX_TOKENS")
tool_output_max_lines: int = Field(default=2_000, gt=0, alias="STRIX_TOOL_OUTPUT_MAX_LINES")
# Floor comfortably above the truncation-notice size so a preview
# (head+tail+notice) always fits within the configured ceiling.
# Floor above the truncation-notice size so a preview always fits.
tool_output_max_bytes: int = Field(
default=50 * 1024, ge=1024, alias="STRIX_TOOL_OUTPUT_MAX_BYTES"
)

View file

@ -1,10 +1,7 @@
"""Bound oversized tool results before they enter agent history.
A single verbose tool result (a recursive ``find``, a noisy scanner, a full
page dump) can otherwise pin the whole conversation near the model's context
limit for the rest of the scan. This keeps a head + tail slice of the output
and drops the middle, mirroring how the shell capability truncates its own
output the agent still sees the start and end plus how much was removed.
Keeps a head + tail slice and drops the middle, replacing it with a notice of
how much was removed.
"""
from __future__ import annotations
@ -45,22 +42,16 @@ def _take_suffix(text: str, max_bytes: int) -> str:
def bound_text(text: str, *, max_lines: int, max_bytes: int) -> str:
"""Return ``text`` unchanged when small, else a head+tail preview.
Truncation happens on whichever limit is hit first (line count or UTF-8
byte size). The removed middle is replaced with a notice recording how
many lines and bytes were dropped so the agent knows output was elided.
``max_bytes`` bounds the *entire* joined result, notice and separators
included, and must be large enough to hold the notice itself (guaranteed by
the ``tool_output_max_bytes`` config floor).
Truncates on whichever limit is hit first (line count or UTF-8 byte size).
``max_bytes`` bounds the entire joined result, notice and separators
included.
"""
lines = text.split("\n")
total_bytes = _byte_len(text)
if len(lines) <= max_lines and total_bytes <= max_bytes:
return text
# Reserve room for the notice and its two blank-line separators so the
# head+tail slices can't consume the whole budget and push the persisted
# value over max_bytes. Upper-bound the notice with the largest possible
# counts; the real notice is never longer. ``+ 4`` covers the separators.
# Reserve notice + separator bytes up front; ``+ 4`` covers the two "\n\n".
notice_overhead = _byte_len(_TRUNCATION_NOTICE.format(lines=len(lines), bytes=total_bytes)) + 4
byte_budget = max(2, max_bytes - notice_overhead)
@ -69,16 +60,13 @@ def bound_text(text: str, *, max_lines: int, max_bytes: int) -> str:
head = "\n".join(lines[:head_lines])
tail = "\n".join(lines[len(lines) - tail_lines :]) if tail_lines > 0 else ""
# Enforce the byte budget even when the line count alone was fine.
half_bytes = max(1, byte_budget // 2)
if _byte_len(head) > half_bytes:
head = _take_prefix(head, half_bytes)
if tail and _byte_len(tail) > half_bytes:
tail = _take_suffix(tail, half_bytes)
# Count kept lines from the final slices: the byte pass above may have
# dropped whole lines from head/tail, so deriving this from the original
# head_lines/tail_lines would undercount what was actually removed.
# Count from the final slices; the byte pass may have dropped whole lines.
kept_lines = len(head.split("\n")) + (len(tail.split("\n")) if tail else 0)
dropped_lines = max(0, len(lines) - kept_lines)
dropped_bytes = max(0, total_bytes - _byte_len(head) - _byte_len(tail))

View file

@ -82,8 +82,6 @@ async def test_wrap_exec_command_preserves_explicit_shell(shell: str) -> None:
@pytest.mark.asyncio
async def test_responses_filesystem_custom_tool_output_is_bounded() -> None:
# In Responses-API mode filesystem tools stay native CustomTools; a large
# read must still be head+tail bounded before it enters history.
async def invoke(_ctx: Any, _inp: str) -> str:
return "line\n" * 50_000

View file

@ -121,14 +121,7 @@ def test_read_json_overrides_uses_json_when_no_alias_in_environ(tmp_path: Path)
assert loader._read_json_overrides(path) == {"llm": {"api_key": "sk-file"}}
# --------------------------------------------------------------------------- #
# ContextSettings validation
# --------------------------------------------------------------------------- #
def test_tool_output_max_bytes_rejects_sub_notice_values() -> None:
# A ceiling below the truncation notice can't fit a bounded preview, so it
# is rejected at load time rather than producing over-cap persisted output.
with pytest.raises(ValidationError):
ContextSettings(STRIX_TOOL_OUTPUT_MAX_BYTES=64)

View file

@ -19,7 +19,6 @@ def test_line_limit_keeps_head_and_tail() -> None:
assert bounded.startswith("0\n1\n2\n3\n4")
assert bounded.rstrip().endswith("999")
assert "truncated" in bounded
# Head + tail only, far fewer than the original 1000 lines.
assert len(bounded.splitlines()) < 30
@ -28,8 +27,6 @@ def test_byte_limit_enforced_on_single_long_line() -> None:
bounded = bound_text(text, max_lines=2_000, max_bytes=1_000)
assert "truncated" in bounded
# The whole joined result (head + tail + notice + separators) honours the
# configured maximum, not just the head/tail slices.
assert len(bounded.encode("utf-8")) <= 1_000
@ -37,7 +34,7 @@ def test_multibyte_characters_not_split() -> None:
text = "😀" * 50_000
bounded = bound_text(text, max_lines=2_000, max_bytes=1_000)
# Must remain valid UTF-8 (no broken surrogate halves from a mid-char cut).
# Must remain valid UTF-8 (no mid-character cut).
assert bounded == bounded.encode("utf-8").decode("utf-8")
assert "truncated" in bounded
@ -51,8 +48,7 @@ def test_notice_reports_dropped_counts() -> None:
def test_dropped_line_count_accounts_for_byte_trimming() -> None:
# A tight byte budget forces the byte pass to drop whole lines from the
# head/tail slices; the notice must count those, not just the middle.
# Tight byte budget drops whole lines from head/tail; the notice must count them.
text = "\n".join(f"line-{i}" for i in range(200))
bounded = bound_text(text, max_lines=20, max_bytes=40)
@ -61,5 +57,4 @@ def test_dropped_line_count_accounts_for_byte_trimming() -> None:
dropped = int(match.group(1))
kept = [ln for ln in bounded.splitlines() if ln and "truncated" not in ln]
assert dropped == 200 - len(kept)
# The naive middle-only count (max_lines split evenly) would under-report.
assert dropped > 200 - 20