mirror of
https://github.com/usestrix/strix.git
synced 2026-08-21 10:48:59 +02:00
fix(context): keep tool-output retrieval reliable and stream pages
Exempt read_tool_output from result-bounding so paging a large stored output isn't re-spilled under a new id, byte-cap each page in place, and stream the requested window with islice instead of loading the whole file per page.
This commit is contained in:
@@ -528,7 +528,12 @@ def build_strix_agent(
|
||||
tools = [*_BASE_TOOLS, *agent_tools, agent_finish]
|
||||
_ensure_unique_tool_names(tools)
|
||||
tools = [
|
||||
_with_bounded_result(tool) if isinstance(tool, FunctionTool) else tool for tool in tools
|
||||
# read_tool_output pages already-stored output; re-bounding it would
|
||||
# re-spill a large page under a new id and make retrieval unusable.
|
||||
_with_bounded_result(tool)
|
||||
if isinstance(tool, FunctionTool) and tool is not read_tool_output
|
||||
else tool
|
||||
for tool in tools
|
||||
]
|
||||
|
||||
logger.info(
|
||||
|
||||
@@ -10,6 +10,7 @@ truncated detail is bounded in history but never lost.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import itertools
|
||||
import logging
|
||||
import re
|
||||
import uuid
|
||||
@@ -27,6 +28,12 @@ _SPILL_NOTICE = (
|
||||
_OUTPUT_ID_RE = re.compile(r"^[0-9a-f]{32}$")
|
||||
_DEFAULT_STORE_DIR = Path.home() / ".strix" / "tool-output"
|
||||
|
||||
# Ceilings for a single retrieval page, so a page of very long lines can't
|
||||
# itself overflow history. Applied without spilling (no new output_id), or
|
||||
# paging would loop forever.
|
||||
_PAGE_MAX_LINES = 2_000
|
||||
_PAGE_MAX_BYTES = 50 * 1024
|
||||
|
||||
# Single-key holder so the configured path can be swapped per scan without a
|
||||
# module-level ``global`` rebind.
|
||||
_config: dict[str, Path] = {}
|
||||
@@ -183,11 +190,18 @@ def read_stored_output(output_id: str, *, offset: int = 0, limit: int = 2_000) -
|
||||
if not path.is_file():
|
||||
return f"No stored output for output_id={output_id!r} (it may have expired)."
|
||||
|
||||
lines = path.read_text(encoding="utf-8").split("\n")
|
||||
start = max(0, offset)
|
||||
window = lines[start : start + max(1, limit)]
|
||||
shown = "\n".join(window)
|
||||
remaining = len(lines) - (start + len(window))
|
||||
count = min(max(1, limit), _PAGE_MAX_LINES)
|
||||
with path.open(encoding="utf-8") as handle:
|
||||
# Stream to the window instead of materialising the whole file per page.
|
||||
for _ in itertools.islice(handle, start):
|
||||
pass
|
||||
window = [line.rstrip("\n") for line in itertools.islice(handle, count)]
|
||||
remaining = sum(1 for _ in handle)
|
||||
|
||||
# Bound the page's byte size with a plain notice (never a spill id) so a
|
||||
# page of very long lines stays within history without re-triggering spill.
|
||||
shown = bound_text("\n".join(window), max_lines=_PAGE_MAX_LINES, max_bytes=_PAGE_MAX_BYTES)
|
||||
if remaining > 0:
|
||||
shown += f"\n\n[... {remaining} more lines; call read_tool_output(output_id="
|
||||
shown += f'"{output_id}", offset={start + len(window)}) to continue ...]'
|
||||
|
||||
@@ -110,3 +110,14 @@ async def test_chat_completions_filesystem_custom_tool_becomes_function_tool() -
|
||||
factory._configure_filesystem_tools(toolset, chat_completions=True)
|
||||
|
||||
assert isinstance(toolset.read_file, FunctionTool)
|
||||
|
||||
|
||||
def test_read_tool_output_is_not_result_bounded() -> None:
|
||||
# Every other FunctionTool gets the result-bounding wrapper, but the
|
||||
# retrieval tool must be exempt or paging a large stored output would be
|
||||
# re-spilled under a new id.
|
||||
agent = factory.build_strix_agent(is_root=True)
|
||||
by_name = {t.name: t for t in agent.tools}
|
||||
|
||||
assert getattr(by_name["read_tool_output"], "_strix_bounded", False) is False
|
||||
assert getattr(by_name["think"], "_strix_bounded", False) is True
|
||||
|
||||
@@ -114,6 +114,25 @@ def test_read_stored_output_paginates(tmp_path: Path) -> None:
|
||||
assert "offset=10" in page
|
||||
|
||||
|
||||
def test_read_stored_output_page_is_bounded_without_new_spill(tmp_path: Path) -> None:
|
||||
# A page of very long lines must be byte-capped in place, never re-spilled
|
||||
# under a fresh output_id (which would make paging loop forever).
|
||||
configure_output_store(tmp_path)
|
||||
text = "\n".join("z" * 5_000 for _ in range(50))
|
||||
output_id = re.search(
|
||||
r'output_id="([0-9a-f]{32})"',
|
||||
bound_and_store(text, max_lines=4, max_bytes=1_000),
|
||||
)
|
||||
assert output_id is not None
|
||||
|
||||
page = read_stored_output(output_id.group(1), offset=0, limit=2_000)
|
||||
|
||||
assert len(page.encode("utf-8")) <= 60 * 1024
|
||||
# No brand-new spill id in the returned page.
|
||||
ids = re.findall(r'output_id="([0-9a-f]{32})"', page)
|
||||
assert ids == [output_id.group(1)] or ids == []
|
||||
|
||||
|
||||
def test_read_stored_output_rejects_traversal(tmp_path: Path) -> None:
|
||||
configure_output_store(tmp_path)
|
||||
assert "Invalid output_id" in read_stored_output("../../etc/passwd")
|
||||
|
||||
Reference in New Issue
Block a user