fix(documents): preserve read pagination within tool output budget (#6810)

This commit is contained in:
red person
2026-11-03 05:37:48 +00:00
committed by GitHub
parent d47d85d8a4
commit f814018098
3 changed files with 117 additions and 3 deletions
+12 -3
View File
@@ -4,7 +4,7 @@ import html
import difflib import difflib
import logging import logging
import re import re
from src.constants import MAX_READ_CHARS from src.constants import MAX_OUTPUT_CHARS, MAX_READ_CHARS
from src.tool_approvals import document_content_digest from src.tool_approvals import document_content_digest
from src.tool_utils import _parse_tool_args, get_upload_handler from src.tool_utils import _parse_tool_args, get_upload_handler
from src.upload_handler import reserve_upload_references from src.upload_handler import reserve_upload_references
@@ -1175,14 +1175,23 @@ class ManageDocumentTool:
except (TypeError, ValueError): except (TypeError, ValueError):
offset = 0 offset = 0
offset = min(offset, len(body)) offset = min(offset, len(body))
anchor = f"[{doc.title}](#document-{doc.id})"
header = f"{anchor} — click to open in editor.\n\n```{doc.language or ''}\n"
# Reserve the closing fence and the longest possible paging
# notice before selecting the page. The common output cap must
# never hide content that next_offset claims was delivered.
notice = f"\n... (truncated, {len(body)} chars total; next_offset={len(body)})"
page_budget = MAX_OUTPUT_CHARS - len(header) - len(notice) - len("\n```")
if page_budget < 1:
return {"error": "Document title or language is too long to display a read page", "exit_code": 1}
preview_limit = min(preview_limit, page_budget)
end = min(offset + preview_limit, len(body)) end = min(offset + preview_limit, len(body))
truncated = end < len(body) truncated = end < len(body)
preview = body[offset:end] preview = body[offset:end]
if truncated: if truncated:
preview += f"\n... (truncated, {len(body)} chars total; next_offset={end})" preview += f"\n... (truncated, {len(body)} chars total; next_offset={end})"
anchor = f"[{doc.title}](#document-{doc.id})"
return { return {
"response": f"{anchor} — click to open in editor.\n\n```{doc.language or ''}\n{preview}\n```", "response": f"{header}{preview}\n```",
"document": { "document": {
"id": doc.id, "id": doc.id,
"title": doc.title, "title": doc.title,
+10
View File
@@ -2562,6 +2562,16 @@ def _compact_binary_like_output(value: object) -> str:
def format_tool_result(description: str, result: Dict) -> str: def format_tool_result(description: str, result: Dict) -> str:
"""Format a tool result into text for feeding back to the LLM.""" """Format a tool result into text for feeding back to the LLM."""
if (
description.startswith("manage_documents:")
and isinstance(result.get("document"), dict)
and "response" in result
):
# Document reads already budget their complete page, anchor and
# next_offset notice. A heading or a second JSON copy of the content
# would exhaust that budget and obscure the continuation again.
return _truncate(result["response"])
parts = [f"### {description}"] parts = [f"### {description}"]
if "stdout" in result: if "stdout" in result:
+95
View File
@@ -0,0 +1,95 @@
"""Large document pages survive the model-facing tool-output boundary."""
import asyncio
import json
import pytest
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from src.constants import MAX_OUTPUT_CHARS
from src.tool_execution import format_tool_result
from core import database
from src.agent_tools import TOOL_HANDLERS
from src.agent_tools.document_tools import get_active_document, set_active_document
@pytest.fixture
def documents(tmp_path, monkeypatch):
engine = create_engine(f"sqlite:///{tmp_path / 'documents.db'}")
database.Base.metadata.create_all(engine)
factory = sessionmaker(bind=engine)
monkeypatch.setattr(database, "SessionLocal", factory)
saved_active = get_active_document()
with factory() as db:
db.add(database.Document(id="owned-document", owner="fixture-owner", title="Owned",
language="text", current_content=""))
db.commit()
yield
set_active_document(saved_active)
engine.dispose()
@pytest.mark.parametrize("limit", [None, 37, 20_000])
def test_formatted_document_pages_reconstruct_the_entire_body(documents, limit):
body = "".join(f"Row {i}: α, quote=\" and slash=\\\n" for i in range(1200))
with database.SessionLocal() as db:
db.query(database.Document).filter_by(id="owned-document").first().current_content = body
db.commit()
offset = 0
chunks = []
while True:
args = {"action": "read", "document_id": "owned-document", "offset": offset}
if limit is not None:
args["limit"] = limit
content = json.dumps(args)
result = asyncio.run(TOOL_HANDLERS["manage_documents"](content, {"owner": "fixture-owner"}))
page = result["document"]
rendered = format_tool_result(f"manage_documents: {content[:80]}", result)
end = page["next_offset"] if page["truncated"] else len(body)
chunk = body[offset:end]
assert chunk in rendered
assert len(rendered) <= MAX_OUTPUT_CHARS
assert rendered.endswith("\n```")
if page["truncated"]:
assert end > offset
assert f"next_offset={end}" in rendered
chunks.append(chunk)
if not page["truncated"]:
break
offset = end
assert "".join(chunks) == body
def test_document_result_cannot_bypass_the_output_cap():
rendered = format_tool_result("manage_documents: read", {
"response": "x" * (MAX_OUTPUT_CHARS * 2),
"document": {"offset": 0, "next_offset": None},
"exit_code": 0,
})
assert len(rendered) < MAX_OUTPUT_CHARS + 100
@pytest.mark.parametrize("body", ["", "short document"])
@pytest.mark.parametrize("offset", [0, 100])
def test_short_or_exhausted_read_has_no_continuation(documents, body, offset):
with database.SessionLocal() as db:
db.query(database.Document).filter_by(id="owned-document").first().current_content = body
db.commit()
result = asyncio.run(TOOL_HANDLERS["manage_documents"](json.dumps({
"action": "read", "document_id": "owned-document", "offset": offset,
}), {"owner": "fixture-owner"}))
assert result["document"]["content"] == body[offset:]
assert result["document"]["next_offset"] is None
assert not result["document"]["truncated"]
assert format_tool_result("manage_documents: read", result) == result["response"]
def test_oversized_header_fails_without_a_nonadvancing_page(documents):
with database.SessionLocal() as db:
db.query(database.Document).filter_by(id="owned-document").first().title = "x" * MAX_OUTPUT_CHARS
db.commit()
result = asyncio.run(TOOL_HANDLERS["manage_documents"](
'{"action":"read","document_id":"owned-document"}', {"owner": "fixture-owner"}))
assert result["exit_code"] == 1
assert "too long" in result["error"]