diff --git a/src/agent_tools/document_tools.py b/src/agent_tools/document_tools.py index c83cec2b0..9a3790335 100644 --- a/src/agent_tools/document_tools.py +++ b/src/agent_tools/document_tools.py @@ -4,7 +4,7 @@ import html import difflib import logging 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_utils import _parse_tool_args, get_upload_handler from src.upload_handler import reserve_upload_references @@ -1175,14 +1175,23 @@ class ManageDocumentTool: except (TypeError, ValueError): offset = 0 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)) truncated = end < len(body) preview = body[offset:end] if truncated: preview += f"\n... (truncated, {len(body)} chars total; next_offset={end})" - anchor = f"[{doc.title}](#document-{doc.id})" return { - "response": f"{anchor} — click to open in editor.\n\n```{doc.language or ''}\n{preview}\n```", + "response": f"{header}{preview}\n```", "document": { "id": doc.id, "title": doc.title, diff --git a/src/tool_execution.py b/src/tool_execution.py index d81309947..f31728e97 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -2562,6 +2562,16 @@ def _compact_binary_like_output(value: object) -> str: def format_tool_result(description: str, result: Dict) -> str: """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}"] if "stdout" in result: diff --git a/tests/test_document_read_pagination.py b/tests/test_document_read_pagination.py new file mode 100644 index 000000000..af57633f0 --- /dev/null +++ b/tests/test_document_read_pagination.py @@ -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"]