From 74f334cb935c78111e242b8941656dff259391e6 Mon Sep 17 00:00:00 2001 From: Ahmed Allam Date: Sun, 26 Jul 2026 19:26:51 +0000 Subject: [PATCH] refactor(context): trim verbose comments --- strix/agents/factory.py | 26 ++++---------------------- strix/config/settings.py | 3 +-- strix/tools/output_store.py | 26 +++++++------------------- tests/test_agent_factory_shell.py | 2 -- tests/test_config_loader.py | 7 ------- tests/test_output_store.py | 9 ++------- 6 files changed, 14 insertions(+), 59 deletions(-) diff --git a/strix/agents/factory.py b/strix/agents/factory.py index d6b36722..872e3564 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -129,12 +129,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 @@ -200,13 +195,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: @@ -223,8 +212,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): @@ -277,13 +264,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"] = ( diff --git a/strix/config/settings.py b/strix/config/settings.py index d7c795b6..0d910e60 100644 --- a/strix/config/settings.py +++ b/strix/config/settings.py @@ -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" ) diff --git a/strix/tools/output_store.py b/strix/tools/output_store.py index 6e6e6202..ed65dfa8 100644 --- a/strix/tools/output_store.py +++ b/strix/tools/output_store.py @@ -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)) diff --git a/tests/test_agent_factory_shell.py b/tests/test_agent_factory_shell.py index 22b662c7..814da60c 100644 --- a/tests/test_agent_factory_shell.py +++ b/tests/test_agent_factory_shell.py @@ -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 diff --git a/tests/test_config_loader.py b/tests/test_config_loader.py index 0d37e1e8..7e14662a 100644 --- a/tests/test_config_loader.py +++ b/tests/test_config_loader.py @@ -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) diff --git a/tests/test_output_store.py b/tests/test_output_store.py index 14bff19c..4db8ccf7 100644 --- a/tests/test_output_store.py +++ b/tests/test_output_store.py @@ -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