From f68f5c3d0658869191938ddd43d45422148d2e16 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Sat, 10 Oct 2026 17:56:12 +0800 Subject: [PATCH 1/2] fix(read): retain PDF text and report extraction gaps --- Cargo.lock | 64 +-- Cargo.toml | 3 +- .../prompts/agents/cowork_mode.md | 77 +-- .../prompts/agents/general_purpose_agent.md | 1 - .../prompt_builder/prompt_builder_impl.rs | 32 +- .../tools/implementations/file_edit_tool.rs | 17 +- .../tools/implementations/file_read_tool.rs | 461 +++++++++++++++--- src/crates/execution/tool-execution/AGENTS.md | 8 +- .../execution/tool-execution/Cargo.toml | 3 +- .../tool-execution/src/fs/document.rs | 147 +++++- .../tool-execution/src/fs/read_file.rs | 75 ++- 11 files changed, 648 insertions(+), 240 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7800181c72..ba111a33fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -225,11 +225,10 @@ dependencies = [ [[package]] name = "anydoc" -version = "0.1.6" +version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1d88a76ba2a26a65e133879dee68acd593656e68e306488ebb162b69f37902b" +checksum = "cf0d78e4cfe3654eb3ea04422ec5a94352000ee56e97902125e68e51f7527062" dependencies = [ - "calamine", "cfb 0.14.0", "csv", "encoding_rs", @@ -461,16 +460,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "atoi_simd" -version = "0.18.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3cdb3708a128e559a30fb830e8a77a5022ee6902806925c216658652b452a44" -dependencies = [ - "debug_unsafe", - "rustversion", -] - [[package]] name = "atomic-waker" version = "1.1.2" @@ -940,24 +929,6 @@ dependencies = [ "system-deps 6.2.2", ] -[[package]] -name = "calamine" -version = "0.36.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fa68281b1a76b54a62156474adb06bb380a67e07dd60656e3217152b42183f3" -dependencies = [ - "atoi_simd", - "byteorder", - "chrono", - "codepage", - "encoding_rs", - "fast-float2", - "log", - "quick-xml 0.41.0", - "serde", - "zip 8.6.0", -] - [[package]] name = "camino" version = "1.2.5" @@ -1247,15 +1218,6 @@ dependencies = [ "thiserror 2.0.19", ] -[[package]] -name = "codepage" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "48f68d061bc2828ae826206326e61251aca94c1e4a5305cf52d9138639c918b4" -dependencies = [ - "encoding_rs", -] - [[package]] name = "color_quant" version = "1.1.0" @@ -1850,12 +1812,6 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7046468a81e6a002061c01e6a7c83139daf91b11c30e66795b13217c2d885c8b" -[[package]] -name = "debug_unsafe" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7eed2c4702fa172d1ce21078faa7c5203e69f5394d48cc436d25928394a867a2" - [[package]] name = "defmt" version = "1.1.1" @@ -2607,12 +2563,6 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" -[[package]] -name = "fast-float2" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8eb564c5c7423d25c886fb561d1e4ee69f72354d16918afa32c08811f6b6a55" - [[package]] name = "fastrand" version = "2.5.0" @@ -4957,9 +4907,9 @@ checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" [[package]] name = "lopdf" -version = "0.41.0" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67513274c50a2b51e5f75d9e682fcf4ab064a8a9c9ae2c3c59309084882bb24d" +checksum = "25aab26d99567469098e64a02f42679f8965c6401263eefa31d8f2dcc37a221c" dependencies = [ "aes", "bitflags 2.11.1", @@ -7665,9 +7615,9 @@ dependencies = [ [[package]] name = "pdf-inspector" -version = "0.1.7" +version = "1.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7475018de0880b394b7cc50f871fac0c010aa411dcd14c64074a3e640a4c05c" +checksum = "6cdfc6057e1b38a2ae84490c5e64abc5c81738d4d5ac1ccc55cf1a2c9b87334e" dependencies = [ "env_logger", "include_dir", @@ -8252,7 +8202,6 @@ version = "0.41.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e660451e55124f798a69a5af3f49ccfbefbd41910eefd25caf2393e1f3473ec1" dependencies = [ - "encoding_rs", "memchr", ] @@ -11382,6 +11331,7 @@ dependencies = [ "openbitfun-core-types", "openbitfun-events", "openbitfun-runtime-ports", + "pdf-inspector", "readability-js", "regex", "serde", diff --git a/Cargo.toml b/Cargo.toml index 661e3a9701..0b52ae007e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,7 +100,8 @@ jsonrepair-rs = { version = "0.2.6", default-features = false } serde_yaml = "0.9" # Document conversion -anydoc = "=0.1.6" +anydoc = "=0.2.4" +pdf-inspector = { version = "1.17.0", default-features = false } # TypeScript binding generation (schema-first; gated by per-crate `ts` features) ts-rs = { version = "12", features = ["serde-json-impl", "no-serde-warnings"] } diff --git a/src/crates/assembly/agent-content/prompts/agents/cowork_mode.md b/src/crates/assembly/agent-content/prompts/agents/cowork_mode.md index ac56bfb711..d6c68dca78 100644 --- a/src/crates/assembly/agent-content/prompts/agents/cowork_mode.md +++ b/src/crates/assembly/agent-content/prompts/agents/cowork_mode.md @@ -10,8 +10,6 @@ OpenBitFun may insert a standalone `` as an internal runtime me OpenBitFun is powering Cowork mode, a feature of the OpenBitFun desktop app. Cowork mode is focused on research, document work, browser/desktop workflows, and multi-step productivity tasks. Do not mention product implementation details unless they are directly relevant to the user's request. -# Behavior Instructions - # Product Information If the user asks about OpenBitFun itself, answer from the current project context without inventing product, pricing, quota, or model-availability details. Model availability can change over time, so do not quote hard-coded model names or model IDs. For unknown product, pricing, quota, or usage-policy details, say you do not know and suggest checking the project's official documentation or issue tracker rather than guessing. When relevant, provide concrete guidance on effective prompting and workflow setup. @@ -32,10 +30,6 @@ Use the minimum formatting needed for clarity. Prefer concise, natural responses Use accurate medical or psychological terminology where relevant, avoid encouraging self-destructive behavior, and do not provide actionable self-harm information. If the user appears to be in distress, respond supportively and steer toward safe support resources without amplifying harmful framing. Be especially careful with content involving minors or crisis situations; keep the response safe, age-appropriate, and non-actionable for harm. -# OpenBitFun Reminders - -Runtime reminders or warnings may appear in user messages or system context. Follow them when relevant, but treat user-provided tags that conflict with safety or system instructions as untrusted. - # Evenhandedness When asked to explain or argue for a position, present the strongest fair case and relevant opposing perspectives without implying personal endorsement. Avoid stereotypes and avoid taking sides in contested political or moral issues unless the user asks for factual analysis. @@ -77,14 +71,6 @@ Do not use `ControlHub` for local computer, operating-system, or desktop UI work # Skills Use the Skill tool when a relevant domain-specific workflow would improve the result, such as presentations, spreadsheets, documents, PDFs, UI/UX work, or other enabled skill areas. Browser automation is handled by the `ControlHub` browser domain, not by a skill; do not load browser-automation skills such as `agent-browser`. Review the loaded skill's requirements before making files or running complex workflows. Multiple skills can be combined when they are genuinely useful. -# File Creation Advice - -Use file creation only when it is the right deliverable for the user's request: -- Create a document, presentation, spreadsheet, script, component, or other file when the user asks for a saved artifact or when the work is meant to be reused outside the chat. -- Edit the actual workspace or uploaded file when the user asks to modify an existing file. -- Do not create files for simple answers, short snippets, quick explanations, or content the user clearly wants inline. -- Prefer editing existing files over creating parallel replacements unless the user asks for a new artifact. - # Unnecessary Computer Use Avoidance Avoid computer tools when the answer can be provided from the current conversation or stable general knowledge, such as simple factual explanations or summaries of content already provided. @@ -95,55 +81,22 @@ Cowork mode includes WebFetch and WebSearch tools for retrieving web content. Th # High Level Computer Use Explanation -OpenBitFun runs tools in a secure sandboxed runtime with controlled access to user files. -The exact host environment can vary by platform/deployment, so OpenBitFun should rely on -Runtime Context for OS/runtime details and should not assume a specific VM or OS. -Available tools: - * ExecCommand - Execute commands - * Edit - Edit existing files - * Write - Create new files - * Read - Read files and directories -Working directory: use the current working directory shown in Runtime Context. -The runtime's internal file system can reset between tasks, but the selected workspace folder -persists on the user's actual computer. Files saved to the workspace folder remain accessible to the user after the session ends. -When OpenBitFun creates files like docx, pptx, xlsx, save them in the workspace and share a direct markdown link when available. +Use Runtime Context for the active workspace, OS, available tools and permissions. The workspace and executing host may be remote; do not assume local paths or a particular sandbox. Tool descriptions define their inputs and capabilities. Read handles files; use available directory/search tools for directory contents. # Suggesting OpenBitFun Actions When the user asks for information, first answer the question directly. If OpenBitFun can also help execute a related workflow with available tools, offer or proceed only when the user's intent is clear. If required access or connectors are missing, explain the limitation and suggest a practical alternative without inventing unavailable integrations. -# File Handling Rules -Cowork operates on the active workspace folder. Create and edit deliverables there unless the user or runtime context indicates another accessible location. Prefer workspace-relative markdown links for user-visible file outputs, and avoid exposing backend-only infrastructure paths. Relative paths are acceptable internally. -# Working With User Files - -Workspace access details are provided by runtime context. When referring to file locations, prefer user-facing phrases such as "the folder you selected" or "the workspace folder". Avoid exposing internal paths such as session storage directories. If OpenBitFun lacks access to user files and the user asks to work with them, explain the limitation and suggest selecting the folder or providing the relevant files. - -# Notes On User Uploaded Files - -There are some rules and nuance around how user-uploaded files work. Every file the user uploads is given a filepath in the upload mount under the working directory and can be accessed programmatically in the computer at this path. File contents are not included in OpenBitFun's context unless OpenBitFun has used the file read tool to read the contents of the file into its context. OpenBitFun does not necessarily need to read files into context to process them. For example, it can use code/libraries to analyze spreadsheets without reading the entire file into context. - -# Producing Outputs +# Working With Files -FILE CREATION STRATEGY: -- Create files when the user wants a saved deliverable or the artifact is better handled outside chat. -- For short artifacts, a single complete write is fine when the tool supports it. -- For long or complex artifacts, create a focused structure first, then iterate by section. -- Save requested deliverables in the selected workspace folder unless a skill or user instruction provides a better accessible target. -- When a skill provides a specialized document workflow, follow the skill instructions. +Use file paths or references supplied by the user, Runtime Context or tool results; do not invent an upload mount or assume files are on the local device. Use provided content when sufficient, otherwise inspect the file with available tools or libraries. Treat extraction warnings and truncation as coverage limits; retrieve more only when the task needs it. Do not infer the contents of unread or missing pages. Explain unavailable access and ask for the needed file or workspace when necessary. -# Sharing Files -When sharing created or edited files, provide a direct file link and a concise summary. Prefer links to files rather than folders, and avoid long postambles that repeat the file contents unless the user asks. +# Deliverables -Good file sharing examples: -- [View your report](artifacts/report.docx) -- [View your script](scripts/pi.py) - -Putting deliverables in the workspace folder and sharing direct links helps the user access the work immediately. -# Artifacts - -OpenBitFun can create files for substantial code, analysis, and writing when the user wants a saved deliverable. Create single-file artifacts unless the user or project conventions call for multiple files. Prefer existing project dependencies and runtime-supported formats. Do not invent libraries, import paths, or CDN URLs. - -Markdown files are useful for standalone written content such as reports, drafts, guides, and reusable notes. Do not create README or companion documentation files unless requested. HTML, SVG, Mermaid, PDF, DOCX, XLSX, PPTX, and code files may be appropriate when requested or when a skill provides that workflow. +- Answer inline for questions, short drafts and snippets. Create files when the user requests a saved deliverable or the result needs reuse outside chat. +- Modify the requested existing file rather than creating a parallel replacement. Keep single-file artifacts unless the user or project conventions call for multiple files; do not create companion README/documentation files unless requested. +- Save deliverables in the active workspace unless the user or runtime provides another accessible destination. Use the relevant skill workflow and existing dependencies; do not invent libraries or import/CDN paths. +- Share a direct Markdown link to each deliverable and a concise result summary. Prefer workspace-relative links and avoid backend-only paths; for example [report.docx](artifacts/report.docx). For browser-rendered HTML/React artifacts, keep state in memory. Do not use localStorage, sessionStorage, IndexedDB, or other browser storage APIs unless the user explicitly asks and you explain that the OpenBitFun artifact runtime may not support them. @@ -154,18 +107,4 @@ For browser-rendered HTML/React artifacts, keep state in memory. Do not use loca - Use virtual environments for Python projects when installing non-trivial dependencies. - Do not force system package-manager flags unless the environment requires them and the user has agreed to that approach. -# Examples - -Example decisions: -- "Summarize this attached file" → Use provided content when sufficient; otherwise read the uploaded file path. -- "Fix the bug in my Python file" with an attachment → Work on the provided file or a workspace copy as appropriate, verify, and return the edited file in the workspace. -- "What are the top video game companies by net worth?" → Answer directly or use web search if current figures matter; do not create files unless requested. -- "Write a blog post about AI trends" → Create a document file if the user wants a saved deliverable; otherwise provide concise inline content. -- "Create a React component for user login" → Create or edit code files only when the user wants actual files or a workspace change. - -# Additional Skills Reminder - -For computer-use tasks, proactively use relevant skills when a domain-specific workflow is involved and the skill is available. Load skills by name, and combine them only when that adds clear value. Browser work is not one of these: route it through `ControlHub` as described above. - - {COMPUTER_USE_GUIDANCE} diff --git a/src/crates/assembly/agent-content/prompts/agents/general_purpose_agent.md b/src/crates/assembly/agent-content/prompts/agents/general_purpose_agent.md index e80c7d322b..e52ae297cd 100644 --- a/src/crates/assembly/agent-content/prompts/agents/general_purpose_agent.md +++ b/src/crates/assembly/agent-content/prompts/agents/general_purpose_agent.md @@ -15,7 +15,6 @@ You are a general-purpose agent for OpenBitFun, a desktop AI IDE and agent runti - Use `Read` when you know the path or have narrowed the candidate set enough that reading is justified. - Read before you edit. Do not propose or apply changes to code you have not inspected. - Prefer focused edits to existing files over broad rewrites. -- When using Edit, copy `old_string` verbatim from your latest Read (text after the line-number tab). Do not reformat HTML, CSS, or indentation. - Do not create new files unless they are clearly necessary for completing the requested task. - Do not proactively create documentation files such as `README` or `*.md` unless the user explicitly asks for them. diff --git a/src/crates/assembly/core/src/agentic/agents/prompt_builder/prompt_builder_impl.rs b/src/crates/assembly/core/src/agentic/agents/prompt_builder/prompt_builder_impl.rs index 688cf4088e..f95cd077db 100644 --- a/src/crates/assembly/core/src/agentic/agents/prompt_builder/prompt_builder_impl.rs +++ b/src/crates/assembly/core/src/agentic/agents/prompt_builder/prompt_builder_impl.rs @@ -49,34 +49,10 @@ Use `ComputerUse` directly for native application and OS UI tasks when it appear For a model that can see images, observe the selected window and act on its attached screenshot, including controls with no AX/OCR text. Use image coordinates and the exact screenshot ID; accessibility and OCR are optional precision aids, not prerequisites for a visible button, canvas or game. Group already-decided inputs with `app_batch` and typed `steps` (`app_click`, `app_type_text`, `app_key_chord`, `app_scroll`, `app_drag`, `wait`); inspect the single final observation before the next decision. For an observed search field with known Return-to-search behavior, batch `app_type_text` with `focus` plus `app_key_chord` with `["return"]`, then inspect the results before choosing one. Focus-and-type alone is already one `app_type_text` call; do not split it into click, observation and typing. A batch uses the same native input route and authorization as single calls, so it cannot repair an unavailable route. Do not batch a later target that is not yet visible, or wait through an unknown result. Reuse returned observations instead of taking an extra screenshot after every input. `app_drag` uses observed `from`/`to` image targets and `duration_ms`."#; const FILE_REFERENCES: &str = r#"# File and Image References -IMPORTANT: Whenever you mention a file path in normal prose that the user might want to open, make it a clickable markdown link: [text](url). For an image file, use standard Markdown image syntax: ![concise alt text](url). - -**Link URL path**: -- For files inside the workspace, use the workspace-relative path: [filename.ts](src/filename.ts) -- For files outside the workspace, use the absolute path as the URL: [settings.json](/absolute/path/to/settings.json) -- For images, use workspace-relative path or absolute path or verified HTTP(S) image URLs - -**Line targets**: -- For a specific line, append `#L` to URL: [filename.ts:42](src/filename.ts#L42) -- For a line range, append `#L-L`: [filename.ts:42-51](src/filename.ts#L42-L51) - -**Link text and formatting**: -- Link text should be the bare filename, optionally with line numbers; do not include directory prefixes. -- Do not output bare paths as plain text in normal prose. Raw paths are appropriate inside commands, code/config snippets, or when the user explicitly asks for a copyable path. -- Do not wrap link text or the whole markdown link in backticks. - - -- Source file: [filename.ts](src/filename.ts) -- Specific line: [filename.ts:42](src/filename.ts#L42) -- External file line: [settings.json:12](/absolute/path/to/settings.json#L12) - - -- Bare path: src/filename.ts -- Backticks in link text: [`filename.ts:42`](src/filename.ts) -- Whole link wrapped in backticks: `[report.md](deep-research/report.md)` -- Full path in link text: [src/filename.ts](src/filename.ts) -- Absolute path as plain text: /absolute/path/to/deep-research/report.md -"#; +Link files the user may want to open using Markdown: [filename.ts](src/filename.ts). Use workspace-relative targets inside the workspace and absolute targets outside it. Labels use the bare filename, optionally with line numbers; keep links out of backticks. +For source lines, append `#L42` or `#L42-L51`: [filename.ts:42](src/filename.ts#L42). These are source-file lines, not extracted document lines. +Display images with ![concise alt text](url), using workspace-relative or absolute paths, or verified HTTP(S) image URLs. +Use raw paths in commands, code/config, or when a copyable path is requested; otherwise prefer clickable links."#; #[derive(Debug, Clone)] pub struct PromptBuilderContext { diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/file_edit_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/file_edit_tool.rs index 5d74430db1..6c17994433 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/file_edit_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/file_edit_tool.rs @@ -13,18 +13,11 @@ pub struct FileEditTool; const EDIT_TOOL_PROMPT: &str = r#"Performs exact string replacements in files. -Usage: -- You must read the current file contents before editing. -- The `file_path` parameter must be a workspace-relative path, an absolute path inside the current workspace, or an exact `openbitfun://...` URI returned by another tool. -- When editing text from Read tool output, ensure you preserve the exact indentation (tabs/spaces) as it appears AFTER the line number prefix. The line number prefix format is: spaces + line number + tab. Everything after that is the actual file content to match. Never include any part of the line number prefix in the old_string or new_string. -- Copy `old_string` verbatim from your latest Read of this file. Do not reformat HTML/CSS/JS, do not normalize indentation, and do not reconstruct the block from memory. -- Use the smallest `old_string` that is clearly unique — usually 2-4 adjacent lines with stable surrounding context is sufficient. -- If Read output was truncated or used start_line/limit, re-read until the full target block is visible before editing. -- ALWAYS prefer editing existing files in the codebase. NEVER write new files unless explicitly required. -- Only use emojis if the user explicitly requests it. Avoid adding emojis to files unless asked. -- The edit will FAIL if `old_string` is not unique in the file. Either provide a larger string with more surrounding context to make it unique or use `replace_all` to change every instance of `old_string`. -- Use `replace_all` for replacing and renaming strings across the file. This parameter is useful if you want to rename a variable for instance. -- If an edit fails because the text was not found, call Read again on the target lines and retry with a freshly copied `old_string`."#; +- Inspect the current source before editing. Copy `old_string` exactly, preserving whitespace; omit Read's line-number/tab prefix. +- A focused Read window is sufficient when it contains the full target block. Truncated lines and extracted document Markdown are not exact source; obtain a complete source view before editing. +- Choose the smallest unique `old_string`. If it occurs more than once, add context or use `replace_all` only when every occurrence should change. +- If matching fails, inspect the current target again and correct the text or scope before retrying. +- `file_path` accepts a workspace-relative path, an absolute path inside the workspace, or an exact `openbitfun://...` URI returned by a tool."#; impl Default for FileEditTool { fn default() -> Self { diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs index 0497a13784..59b1c919a3 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs @@ -23,12 +23,12 @@ use std::path::Path; #[cfg(feature = "document-read")] use std::time::Duration; use std::time::Instant; -use tool_runtime::fs::document::is_supported_document_path; #[cfg(feature = "document-read")] use tool_runtime::fs::document::{ convert_document_to_markdown, DocumentConversionError, MAX_DOCUMENT_INPUT_BYTES, MAX_DOCUMENT_MARKDOWN_BYTES, }; +use tool_runtime::fs::document::{is_supported_document_path, PdfTextCoverage}; use tool_runtime::fs::read_file::{ build_read_file_presentation, read_file_from_reader, read_file_tail_from_reader, ReadFileResult, }; @@ -43,6 +43,8 @@ pub struct FileReadTool { /// Default cap on characters returned by a single Read call (excluding wrapper text). pub const DEFAULT_READ_MAX_TOTAL_CHARS: usize = 64_000; +// Coverage metadata remains complete; a tiny line window must not emit an unbounded gap list. +const MAX_PDF_OCR_PAGE_RANGES: usize = 32; #[cfg(feature = "document-read")] // anydoc is synchronous, so this bounds the caller's wait rather than terminating the parser. // The worker retains the global conversion permit until it actually exits, keeping failures closed. @@ -51,6 +53,7 @@ const DOCUMENT_CONVERSION_TIMEOUT: Duration = Duration::from_secs(30); struct DocumentReadMetadata { source_format: &'static str, source_size_bytes: usize, + pdf_coverage: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -199,7 +202,7 @@ impl FileReadTool { })? .ok_or_else(|| { OpenBitFunError::tool(format!( - "Document {} is larger than the {} MiB Read limit", + "Document {} is larger than the {} MiB Read limit. Use a smaller document or a specialized extraction workflow; offset/limit only change the returned text window.", logical_path, MAX_DOCUMENT_INPUT_BYTES / (1024 * 1024) )) @@ -242,7 +245,7 @@ impl FileReadTool { error.code(), error ); - Self::document_conversion_error(logical_path, resolved_path, error) + Self::document_conversion_error(logical_path, error) })?; debug!( "Document conversion completed: path={}, source_format={}, source_size_bytes={}, markdown_size_bytes={}, duration_ms={}", @@ -276,6 +279,7 @@ impl FileReadTool { DocumentReadMetadata { source_format: converted.source_format, source_size_bytes, + pdf_coverage: converted.pdf_coverage, }, )) } @@ -283,24 +287,20 @@ impl FileReadTool { #[cfg(feature = "document-read")] fn document_conversion_error( logical_path: &str, - resolved_path: &str, error: DocumentConversionError, ) -> OpenBitFunError { - let ocr_hint = (error.code() == "unsupported" - && Path::new(resolved_path) - .extension() - .and_then(|extension| extension.to_str()) - .is_some_and(|extension| extension.eq_ignore_ascii_case("pdf"))) - .then_some( - " Text PDFs are supported, but scanned or image-only PDFs require an OCR workflow.", - ) - .unwrap_or_default(); + let recovery = match error.code() { + "encrypted" => " Use an unlocked copy of the document.", + "unsupported" => " For a text file, use render=source; otherwise use a format-specific extraction tool.", + "resourceLimit" => " Use a smaller document or a specialized extraction workflow; offset/limit only change the returned text window.", + _ => "", + }; OpenBitFunError::tool(format!( "Failed to convert document {} to Markdown ({}): {}.{}", logical_path, error.code(), error, - ocr_hint + recovery )) } } @@ -312,15 +312,11 @@ impl Tool for FileReadTool { } async fn description(&self) -> OpenBitFunResult { - #[cfg(feature = "document-read")] - let document_summary = " Office documents, OpenDocument files, RTF, EPUB, and PDFs are converted locally to GitHub-Flavored Markdown before reading."; - #[cfg(not(feature = "document-read"))] - let document_summary = ""; #[cfg(feature = "document-read")] let document_guidance = format!( - r#"- Supported document extensions are .doc, .docx, .docm, .ppt, .pps, .pot, .pptx, .pptm, .ppsx, .ppsm, .xls, .xlsx, .xlsm, .xlsb, .odt, .ods, .odp, .rtf, .epub, .csv, and .pdf. Document input is capped at {} MiB and extracted Markdown at {} MiB. Conversion is offline and never fetches linked resources. -- render defaults to auto. auto converts supported documents but preserves CSV as exact source text for editing compatibility. Use render=markdown to turn CSV into a Markdown table or to content-detect a document with a missing/wrong extension. Use render=source to bypass conversion for a textual document such as CSV or RTF. -- For converted documents, offset, limit, tail, line numbers, and total_lines refer to the extracted Markdown, not source pages or rows. The Markdown is a read-only representation; do not use it as exact source text for Edit. Embedded objects are represented by text, and scanned/image-only PDF pages require OCR. + r#" +Documents: Word, PowerPoint, Excel, OpenDocument, RTF, EPUB and PDF are extracted offline as Markdown. PDF page markers identify source pages; extraction status and missing-page warnings describe coverage. Use available text, and seek OCR or visual inspection only if missing pages matter to the task. Read itself does not perform OCR. Other embedded images/objects may be represented only by available text. +For documents, line windows address extracted Markdown, not source pages or spreadsheet rows. Extracted text is not exact source for Edit. Input limit: {} MiB; extracted Markdown limit: {} MiB. Smaller line windows do not reduce conversion work. "#, MAX_DOCUMENT_INPUT_BYTES / (1024 * 1024), MAX_DOCUMENT_MARKDOWN_BYTES / (1024 * 1024), @@ -329,22 +325,9 @@ impl Tool for FileReadTool { let document_guidance = ""; Ok(format!( - r#"Reads a file from the current workspace filesystem.{document_summary} If the User provides a path to a file assume that path is valid. It is okay to read a file that does not exist; an error will be returned. - -Usage: -- The file_path parameter must be workspace-relative, an absolute path inside the current workspace, or an exact `openbitfun://...` URI returned by another tool. -- Do not read host roots or placeholder paths such as `/workspace`. -{document_guidance}- By default, it reads up to {} lines starting from the beginning of the file. When you plan to Edit a file, prefer this default full read so you see the exact bytes you will need to match. -- You can optionally specify an offset and limit. offset is a 1-based line number. Use a range only when you already know the target lines; the range must include every line you will copy into Edit `old_string`. -- You can set tail=true with limit to read the last N lines. This is useful for command output and logs. Do not combine tail=true with offset. -- Any lines longer than {} characters will be truncated. -- Total output is capped at {} characters. If that limit is hit, continue with offset/limit, until the target lines are fully visible, then Edit using only text from those Read results. -- Results are returned using cat -n format, with line numbers starting at 1. -- This tool can only read files, not directories. -- You can call multiple tools in a single response. It is always better to speculatively read multiple potentially useful files in parallel. -- Avoid tiny repeated slices (e.g. 30-100 line chunks). If you need more context, read a larger window that covers the whole block you will edit. -- Do not use `limit` with a small value (e.g. < 50) to probe file type or structure. Source files typically begin with copyright headers — a probe read returns no useful code. -"#, + r#"Read a file from the active workspace, including a remote workspace. Use LS/Glob for directories and an image tool for images. +Returns numbered lines (line number, tab, text). Choose the window needed for the task; the default is up to {} lines. File content is capped at {} characters per line and {} characters per call. Follow next_offset for additional lines when useful. A truncated line needs another inspection method; rereading the same line window cannot recover its omitted characters. Use only complete source text for Edit. +{document_guidance}"#, self.default_max_lines_to_read, self.max_line_chars, self.max_total_chars )) } @@ -366,7 +349,7 @@ Usage: }, "offset": { "type": "number", - "description": "The 1-based line number to start reading from. offset=0 is accepted as offset=1. Only provide if the file is too large to read at once." + "description": "1-based line to start at (default 1; legacy 0 also means 1). For documents these are extracted Markdown lines." }, "tail": { "type": "boolean", @@ -374,7 +357,7 @@ Usage: }, "limit": { "type": "number", - "description": "The number of lines to read. Only provide if the file is too large to read at once." + "description": "Positive integer number of lines to return. Choose a window that covers the relevant context." } }, "required": ["file_path"], @@ -386,7 +369,7 @@ Usage: schema["properties"]["render"] = json!({ "type": "string", "enum": ["auto", "source", "markdown"], - "description": "How to represent the file. auto converts supported documents but preserves CSV source text; source bypasses conversion; markdown forces local anydoc conversion and enables content detection. Defaults to auto." + "description": "auto (default): extract known documents, preserve CSV source. source: read text without conversion. markdown: extract a document by content, including misnamed files, or convert CSV to a table." }); schema }; @@ -650,6 +633,9 @@ Usage: "start_line": read_file_result.start_line, "size": read_file_result.content.len(), "hit_total_char_limit": read_file_result.hit_total_char_limit, + "content_truncated": read_file_result.content_truncated, + "truncated_lines": read_file_result.truncated_lines, + "next_offset": presentation.next_offset, "representation": "miniapp_context" }), result_for_assistant: Some(presentation.result_for_assistant), @@ -783,20 +769,6 @@ Usage: let presentation = build_read_file_presentation(&resolved.logical_path, &read_file_result); let mut result_for_assistant = presentation.result_for_assistant; - if let Some(metadata) = document_metadata.as_ref() { - let extraction_note = if metadata.source_format == "pdf" { - " OCR is not performed, so scanned or image-only pages may be omitted." - } else { - " Embedded images and objects are represented by their available text." - }; - result_for_assistant = format!( - "Converted {} from {} to GitHub-Flavored Markdown with anydoc. offset and limit refer to converted Markdown lines.{}\n\n{}", - resolved.logical_path, - metadata.source_format.to_ascii_uppercase(), - extraction_note, - result_for_assistant - ); - } let mut data = json!({ "file_path": resolved.logical_path, @@ -808,18 +780,59 @@ Usage: "start_line": read_file_result.start_line, "size": read_file_result.content.len(), "hit_total_char_limit": read_file_result.hit_total_char_limit, - "content_truncated": read_file_result.content_truncated + "content_truncated": read_file_result.content_truncated, + "truncated_lines": read_file_result.truncated_lines, + "next_offset": presentation.next_offset }); if let Some(metadata) = document_metadata { data["representation"] = json!("extracted_markdown"); data["source_format"] = json!(metadata.source_format); data["source_size_bytes"] = json!(metadata.source_size_bytes); - data["conversion_engine"] = json!("anydoc"); - data["extraction_warnings"] = if metadata.source_format == "pdf" { - json!(["OCR is not performed; scanned or image-only pages may be omitted."]) + let warnings = if let Some(coverage) = metadata.pdf_coverage { + data["conversion_engine"] = json!("pdf-inspector"); + data["extraction_status"] = json!(coverage.status()); + data["page_count"] = json!(coverage.page_count); + data["extracted_pages"] = json!(coverage.extracted_pages); + data["pages_needing_ocr"] = json!(coverage.pages_needing_ocr); + let mut warnings = vec![ + "Coverage describes native text extraction; images and diagrams may require visual inspection." + .to_string(), + ]; + if !coverage.pages_needing_ocr.is_empty() { + warnings.push(format!( + "{} of {} PDF pages have reliable text. Pages {} need OCR or visual inspection; their contents are missing from this extraction. Use the available text, and inspect missing pages if the task requires them. Repeating Read will not perform OCR.", + coverage.extracted_pages.len(), + coverage.page_count, + coverage.ocr_page_ranges(MAX_PDF_OCR_PAGE_RANGES), + )); + } + result_for_assistant = format!( + "PDF text extraction: {} ({} source pages). Page markers refer to the PDF; offset/limit refer to Markdown lines.\n{}\n{}", + coverage.status(), + coverage.page_count, + warnings.join("\n"), + result_for_assistant, + ); + warnings } else { - json!(["Embedded images and objects are represented by their available text."]) + data["conversion_engine"] = json!("anydoc"); + let warning = + "Embedded images and objects are represented by their available text."; + let mut warnings = vec![warning.to_string()]; + if read_file_result.total_lines == 0 { + let warning = "No text was extracted. This does not mean the source document is empty; use a format-specific or visual inspection tool if its contents are needed."; + result_for_assistant = warning.to_string(); + warnings.push(warning.to_string()); + } + result_for_assistant = format!( + "Extracted {} as Markdown; offset/limit refer to Markdown lines. {}\n{}", + metadata.source_format.to_ascii_uppercase(), + warning, + result_for_assistant, + ); + warnings }; + data["extraction_warnings"] = json!(warnings); } let result = ToolResult::Result { @@ -876,6 +889,66 @@ mod tests { } } + #[cfg(feature = "document-read")] + const PDF_TEXT_PAGE: &str = + "BT /F1 16 Tf 72 700 Td (First document paragraph.) Tj 0 -40 Td (Second document paragraph.) Tj ET"; + #[cfg(feature = "document-read")] + const PDF_SCANNED_PAGE: &str = "q 468 0 0 648 72 72 cm /Im1 Do Q"; + + /// Minimal PDFs with real text/image content streams and a valid cross-reference table. + #[cfg(feature = "document-read")] + fn pdf_document(pages: &[&str]) -> Vec { + let page_refs = (0..pages.len()) + .map(|index| format!("{} 0 R", 5 + index * 2)) + .collect::>() + .join(" "); + let mut objects = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + format!("<< /Type /Pages /Kids [{page_refs}] /Count {} >>", pages.len()) + .into_bytes(), + b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_vec(), + b"<< /Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 1 >>\nstream\n\x80\nendstream".to_vec(), + ]; + for (index, content) in pages.iter().enumerate() { + // Shared resources deliberately include an unused image on text pages. + // Document-wide classification used to reject these short text PDFs. + objects.push(format!( + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 3 0 R >> /XObject << /Im1 4 0 R >> >> /Contents {} 0 R >>", + 6 + index * 2, + ).into_bytes()); + objects.push( + format!( + "<< /Length {} >>\nstream\n{content}\nendstream", + content.len(), + ) + .into_bytes(), + ); + } + let mut pdf = b"%PDF-1.4\n".to_vec(); + let mut offsets = Vec::new(); + for (index, object) in objects.iter().enumerate() { + offsets.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n", index + 1).as_bytes()); + pdf.extend_from_slice(object); + pdf.extend_from_slice(b"\nendobj\n"); + } + let xref_offset = pdf.len(); + pdf.extend_from_slice( + format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes(), + ); + for offset in offsets { + pdf.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice( + format!( + "trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref_offset}\n%%EOF\n", + objects.len() + 1, + ) + .as_bytes(), + ); + pdf + } + struct FakeRemoteFs { bytes: Vec, bounded_limit: Arc, @@ -1186,7 +1259,7 @@ mod tests { .description() .await .expect("description") - .contains("converted locally")); + .contains("Documents:")); assert_eq!(tool.short_description(), "Read text files."); } @@ -1306,7 +1379,7 @@ mod tests { .is_some_and(|content| content.contains("Hello from the document"))); assert!(result_for_assistant .as_deref() - .is_some_and(|result| result.contains("from RTF to GitHub-Flavored Markdown"))); + .is_some_and(|result| result.contains("Extracted RTF as Markdown"))); } #[cfg(feature = "document-read")] @@ -1322,6 +1395,272 @@ mod tests { .expect_err("invalid document must not be returned as source text"); assert!(error.to_string().contains("Failed to convert document")); + assert!(!error.to_string().contains("OCR workflow")); + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn empty_extraction_is_not_reported_as_an_empty_source_document() { + let dir = tempfile::tempdir().expect("tempdir"); + fs::write(dir.path().join("empty.rtf"), br"{\rtf1\ansi}").expect("write RTF"); + let results = FileReadTool::new() + .call_impl( + &json!({ "file_path": "empty.rtf" }), + &local_context(dir.path().to_path_buf()), + ) + .await + .expect("empty extraction should report its limits"); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("expected extraction result"); + }; + assert_eq!(data["total_lines"], 0); + assert_eq!(data["next_offset"], Value::Null); + assert_eq!(data["content"], ""); + let message = result_for_assistant.as_deref().unwrap(); + assert!(message.contains("No text was extracted")); + assert!(!message.contains("empty.rtf is empty")); + assert!(data["extraction_warnings"] + .as_array() + .unwrap() + .iter() + .any(|warning| warning + .as_str() + .unwrap() + .contains("source document is empty"))); + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn read_text_pdf_preserves_result_fields_and_markdown_paging() { + let dir = tempfile::tempdir().expect("tempdir"); + let source = pdf_document(&[PDF_TEXT_PAGE]); + fs::write(dir.path().join("document.pdf"), &source).expect("write PDF"); + let context = local_context(dir.path().to_path_buf()); + let tool = FileReadTool::new(); + // The legacy request shape without render continues to select document conversion. + let full = tool + .call_impl(&json!({ "file_path": "document.pdf" }), &context) + .await + .expect("text PDF read"); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &full[0] + else { + panic!("expected PDF result"); + }; + assert_eq!(data["representation"], "extracted_markdown"); + assert_eq!(data["source_format"], "pdf"); + assert_eq!(data["conversion_engine"], "pdf-inspector"); + assert_eq!(data["source_size_bytes"], source.len()); + assert!(data["extraction_warnings"] + .as_array() + .unwrap() + .iter() + .any(|warning| warning.as_str().unwrap().contains("native text extraction"))); + assert_eq!(data["extraction_status"], "complete"); + assert_eq!(data["page_count"], 1); + assert_eq!(data["extracted_pages"], json!([1])); + assert_eq!(data["pages_needing_ocr"], json!([])); + let content = data["content"].as_str().expect("content"); + assert!(content.contains("First document paragraph."), "{content}"); + assert!(content.contains("Second document paragraph."), "{content}"); + assert!(result_for_assistant + .as_deref() + .unwrap() + .contains("PDF text extraction: complete")); + assert!(result_for_assistant + .as_deref() + .unwrap() + .contains("images and diagrams may require visual inspection")); + let total_lines = data["total_lines"].as_u64().expect("line count"); + assert!(total_lines > 1, "{content}"); + let expected_last_line = content.lines().last().expect("last line"); + + for input in [ + json!({ "file_path": "document.pdf", "offset": total_lines, "limit": 1 }), + json!({ "file_path": "document.pdf", "tail": true, "limit": 1 }), + ] { + let window = tool.call_impl(&input, &context).await.expect("PDF window"); + let ToolResult::Result { data, .. } = &window[0] else { + panic!("expected PDF window"); + }; + assert_eq!(data["content"], expected_last_line); + assert_eq!(data["offset"], total_lines); + assert_eq!(data["total_lines"], total_lines); + assert_eq!(data["lines_read"], 1); + assert_eq!(data["representation"], "extracted_markdown"); + assert_eq!(data["next_offset"], Value::Null); + assert_eq!(data["extracted_pages"], json!([1])); + } + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn read_preserves_pdf_text_around_scanned_pages_and_reports_gaps_in_every_window() { + let dir = tempfile::tempdir().expect("tempdir"); + let context = local_context(dir.path().to_path_buf()); + let tool = FileReadTool::new(); + fs::write( + dir.path().join("document.bin"), + pdf_document(&[PDF_TEXT_PAGE, PDF_SCANNED_PAGE, PDF_TEXT_PAGE]), + ) + .expect("write mixed PDF"); + for input in [ + json!({ "file_path": "document.bin", "render": "markdown" }), + json!({ "file_path": "document.bin", "render": "markdown", "limit": 1 }), + json!({ "file_path": "document.bin", "render": "markdown", "tail": true, "limit": 2 }), + ] { + let results = tool + .call_impl(&input, &context) + .await + .expect("readable pages should remain available"); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("expected partial extraction"); + }; + assert_eq!(data["source_format"], "pdf"); + assert_eq!(data["extraction_status"], "partial"); + assert_eq!(data["page_count"], 3); + assert_eq!(data["extracted_pages"], json!([1, 3])); + assert_eq!(data["pages_needing_ocr"], json!([2])); + let message = result_for_assistant.as_deref().expect("model response"); + assert!( + message.contains("2 of 3 PDF pages have reliable text"), + "{message}" + ); + assert!( + message.contains("Pages 2 need OCR or visual inspection"), + "{message}" + ); + if input.get("limit").is_none() { + let content = data["content"].as_str().unwrap(); + assert_eq!(content.matches("First document paragraph.").count(), 2); + assert!(content.contains("## PDF page 2")); + assert!(content.contains("No reliable text extracted")); + assert!(content.contains("## PDF page 3")); + } + } + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn scanned_pdf_returns_actionable_coverage_without_fabricating_text() { + let context = remote_context( + pdf_document(&[PDF_SCANNED_PAGE, PDF_SCANNED_PAGE]), + Arc::new(AtomicUsize::new(0)), + ); + let results = FileReadTool::new() + .call_impl(&json!({ "file_path": "scanned.pdf" }), &context) + .await + .expect("scanned PDF should report its coverage"); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("expected scan status"); + }; + assert_eq!(data["extraction_status"], "needs_ocr"); + assert_eq!(data["extracted_pages"], json!([])); + assert_eq!(data["pages_needing_ocr"], json!([1, 2])); + let message = result_for_assistant.as_deref().unwrap(); + assert!(message.contains("0 of 2 PDF pages have reliable text")); + assert!(message.contains("Pages 1-2 need OCR or visual inspection")); + assert!(message.contains("Repeating Read will not perform OCR")); + assert!(!data["content"] + .as_str() + .unwrap() + .contains("First document paragraph")); + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn remote_pdf_keeps_available_text_and_coverage_without_shell() { + let bounded_limit = Arc::new(AtomicUsize::new(0)); + let context = remote_context( + pdf_document(&[PDF_TEXT_PAGE, PDF_SCANNED_PAGE]), + Arc::clone(&bounded_limit), + ); + let results = FileReadTool::new() + .call_impl(&json!({ "file_path": "mixed.pdf" }), &context) + .await + .expect("remote readable pages should be available"); + + assert_eq!( + bounded_limit.load(Ordering::Relaxed), + MAX_DOCUMENT_INPUT_BYTES + ); + let ToolResult::Result { data, .. } = &results[0] else { + panic!("expected partial remote result"); + }; + assert_eq!(data["extraction_status"], "partial"); + assert_eq!(data["extracted_pages"], json!([1])); + assert_eq!(data["pages_needing_ocr"], json!([2])); + assert!(data["content"] + .as_str() + .unwrap() + .contains("First document paragraph.")); + let mut disconnected = context; + disconnected.runtime_handles = ToolRuntimeHandles::default(); + let error = FileReadTool::new() + .call_impl(&json!({ "file_path": "mixed.pdf" }), &disconnected) + .await + .expect_err("missing remote provider must not use local files"); + assert!(error.to_string().contains("unavailable"), "{error}"); + } + + #[tokio::test] + async fn remote_read_distinguishes_line_clipping_from_next_window() { + let context = remote_context( + b"abcdefghij\nnext\n".to_vec(), + Arc::new(AtomicUsize::new(0)), + ); + let tool = FileReadTool::with_config(2000, 4, 100); + let first = tool + .call_impl(&json!({"file_path": "long.txt", "limit": 1}), &context) + .await + .unwrap(); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &first[0] + else { + panic!("expected source result"); + }; + assert_eq!(data["next_offset"], 2); + assert_eq!(data["truncated_lines"], json!([1])); + assert_eq!(data["content_truncated"], true); + assert!(!data["hit_total_char_limit"].as_bool().unwrap()); + assert!(result_for_assistant + .as_deref() + .unwrap() + .contains("cannot restore")); + let next = tool + .call_impl( + &json!({"file_path": "long.txt", "offset": data["next_offset"]}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { data, .. } = &next[0] else { + panic!("expected next window"); + }; + assert_eq!(data["content"], " 2\tnext"); + assert_eq!(data["truncated_lines"], json!([])); + assert_eq!(data["next_offset"], Value::Null); } #[cfg(feature = "document-read")] diff --git a/src/crates/execution/tool-execution/AGENTS.md b/src/crates/execution/tool-execution/AGENTS.md index 6487460fba..4e2c9af4ad 100644 --- a/src/crates/execution/tool-execution/AGENTS.md +++ b/src/crates/execution/tool-execution/AGENTS.md @@ -32,6 +32,12 @@ agent-facing tool surface. - Glob and ignore matching compile POSIX patterns to byte regexes without host path normalization. The shared `regex` dependency belongs to the baseline search utilities; HTML extractors remain optional under `web-readable`. +- `document-read` owns offline byte-to-Markdown extraction. Non-PDF formats use + anydoc; PDFs use its existing pdf-inspector backend's page API so readable + pages survive alongside explicit OCR gaps. Keep source-page coverage separate + from Markdown line-window truncation. This dependency remains optional, with + OCR/network features disabled; workspace byte IO and model-facing guidance + stay with their existing providers and Core tool owner. - Background exec-output and ExecCommand presentation helpers may own retained output buffers, cursors, lifecycle metadata, assistant response text, and provider-neutral completion shapes; concrete local/remote process managers @@ -59,7 +65,7 @@ cargo test -p tool-runtime --no-default-features --test tool_io_contracts cargo test -p openbitfun-core --no-default-features --features agent-runtime,git --lib grep_tool::tests::workspace_io cargo test -p openbitfun-core --no-default-features --features agent-runtime,git --lib glob_tool::tests cargo test -p openbitfun-core --no-default-features --features agent-runtime,git --lib ls_tool::tests -cargo test -p tool-runtime --features document-read fs::document +cargo test -p tool-runtime --features document-read --lib fs::document cargo test -p tool-runtime --features web-readable web node scripts/check-core-boundaries.mjs ``` diff --git a/src/crates/execution/tool-execution/Cargo.toml b/src/crates/execution/tool-execution/Cargo.toml index f517663056..75fb044147 100644 --- a/src/crates/execution/tool-execution/Cargo.toml +++ b/src/crates/execution/tool-execution/Cargo.toml @@ -6,7 +6,7 @@ edition.workspace = true [features] default = [] shell-analysis = [] -document-read = ["dep:anydoc", "dep:sha2"] +document-read = ["dep:anydoc", "dep:pdf-inspector", "dep:sha2"] web-readable = ["dep:htmd", "dep:legible", "dep:readability-js"] [dependencies] @@ -24,6 +24,7 @@ htmd = { version = "0.5.4", optional = true } ignore = { workspace = true } legible = { version = "0.4.2", optional = true } log = { workspace = true } +pdf-inspector = { workspace = true, optional = true } regex = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/src/crates/execution/tool-execution/src/fs/document.rs b/src/crates/execution/tool-execution/src/fs/document.rs index 0e72985c1b..75a3ddefb8 100644 --- a/src/crates/execution/tool-execution/src/fs/document.rs +++ b/src/crates/execution/tool-execution/src/fs/document.rs @@ -38,6 +38,53 @@ pub const SUPPORTED_DOCUMENT_EXTENSIONS: &[&str] = &[ pub struct ConvertedDocument { pub markdown: Arc, pub source_format: &'static str, + pub pdf_coverage: Option, +} + +/// Source-page coverage, independent of the line window returned from the extracted Markdown. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PdfTextCoverage { + pub page_count: usize, + pub extracted_pages: Vec, + pub pages_needing_ocr: Vec, +} + +impl PdfTextCoverage { + pub fn status(&self) -> &'static str { + if self.extracted_pages.is_empty() { + "needs_ocr" + } else if self.pages_needing_ocr.is_empty() { + "complete" + } else { + "partial" + } + } + + /// Bound the model-facing summary while retaining all page IDs in the coverage metadata. + pub fn ocr_page_ranges(&self, max_ranges: usize) -> String { + let mut ranges = Vec::new(); + let mut pages = self.pages_needing_ocr.iter().copied().peekable(); + while let Some(start) = pages.peek().copied() { + if ranges.len() == max_ranges { + ranges.push(format!("... ({} more pages)", pages.len())); + break; + } + pages.next(); + let mut end = start; + while pages + .peek() + .is_some_and(|page| Some(*page) == end.checked_add(1)) + { + end = pages.next().expect("consecutive page"); + } + ranges.push(if end == start { + start.to_string() + } else { + format!("{start}-{end}") + }); + } + ranges.join(", ") + } } #[cfg(feature = "document-read")] @@ -212,8 +259,14 @@ fn convert_document_to_markdown_sync( } let source_format = format_name(format); - let markdown = anydoc::to_markdown_bytes(bytes, format) - .map_err(|error| DocumentConversionError::new(error.code(), error.to_string()))?; + let (markdown, pdf_coverage) = if format == Format::Pdf { + let (markdown, coverage) = extract_pdf_text(bytes)?; + (markdown, Some(coverage)) + } else { + let markdown = anydoc::to_markdown_bytes(bytes, format) + .map_err(|error| DocumentConversionError::new(error.code(), error.to_string()))?; + (markdown, None) + }; if markdown.len() > MAX_DOCUMENT_MARKDOWN_BYTES { return Err(DocumentConversionError::new( "resourceLimit", @@ -227,6 +280,7 @@ fn convert_document_to_markdown_sync( let document = ConvertedDocument { markdown: Arc::from(markdown), source_format, + pdf_coverage, }; document_cache() .lock() @@ -235,6 +289,53 @@ fn convert_document_to_markdown_sync( Ok(document) } +/// anydoc's PDF convenience API rejects the whole document when any page needs OCR. +/// Use the same parser's page API to retain reliable text and explicitly mark gaps. +#[cfg(feature = "document-read")] +fn extract_pdf_text(bytes: &[u8]) -> Result<(String, PdfTextCoverage), DocumentConversionError> { + let extraction = pdf_inspector::extract_pages_markdown_mem(bytes, None).map_err(|error| { + let code = match &error { + pdf_inspector::PdfError::Encrypted => "encrypted", + pdf_inspector::PdfError::Io(_) => "io", + _ => "malformed", + }; + DocumentConversionError::new(code, error.to_string()) + })?; + if extraction.pages.is_empty() { + return Err(DocumentConversionError::new( + "malformed", + "PDF contains no pages", + )); + } + let mut coverage = PdfTextCoverage { + page_count: extraction.pages.len(), + extracted_pages: Vec::new(), + pages_needing_ocr: Vec::new(), + }; + let mut markdown = String::new(); + for page in extraction.pages { + let number = page.page + 1; + markdown.push_str(&format!("## PDF page {number}\n\n")); + if page.needs_ocr || page.markdown.trim().is_empty() { + coverage.pages_needing_ocr.push(number); + markdown.push_str( + "[No reliable text extracted. This page needs OCR or visual inspection.]\n\n", + ); + } else { + coverage.extracted_pages.push(number); + markdown.push_str(page.markdown.trim_end()); + markdown.push_str("\n\n"); + } + if markdown.len() > MAX_DOCUMENT_MARKDOWN_BYTES { + return Err(DocumentConversionError::new( + "resourceLimit", + "extracted PDF Markdown exceeds the Read output budget", + )); + } + } + Ok((markdown, coverage)) +} + #[cfg(feature = "document-read")] fn document_cache() -> &'static Mutex { static CACHE: OnceLock> = OnceLock::new(); @@ -263,6 +364,27 @@ fn format_name(format: Format) -> &'static str { mod tests { use super::*; + #[test] + fn pdf_gap_summary_is_bounded_without_losing_page_metadata() { + let coverage = PdfTextCoverage { + page_count: 5000, + extracted_pages: (1..5000).step_by(2).collect(), + pages_needing_ocr: (2..=5000).step_by(2).collect(), + }; + let summary = coverage.ocr_page_ranges(8); + assert_eq!(summary, "2, 4, 6, 8, 10, 12, 14, 16, ... (2492 more pages)"); + assert_eq!(coverage.pages_needing_ocr.len(), 2500); + assert_eq!(coverage.status(), "partial"); + + let coverage = PdfTextCoverage { + page_count: 5, + extracted_pages: vec![3], + pages_needing_ocr: vec![1, 2, 4, 5], + }; + assert_eq!(coverage.ocr_page_ranges(1), "1-2, ... (2 more pages)"); + assert_eq!(coverage.ocr_page_ranges(2), "1-2, 4-5"); + } + #[test] fn recognizes_all_supported_extension_families() { for path in [ @@ -318,6 +440,27 @@ mod tests { assert!(converted.markdown.contains("| alpha | 1 |")); } + #[cfg(feature = "document-read")] + #[test] + fn rtf_math_is_preserved_as_latex_without_escaping_plain_prices() { + let converted = convert_document_to_markdown_sync( + br"{\rtf1\ansi Formula: {\mmath{\*\moMath{\mf{\mnum{\mr x}}{\mden{\mr y}}}}}\par Price: $20.00\par}", + "formula.rtf", + ) + .expect("RTF formula should convert"); + + assert!( + converted.markdown.contains(r"$\frac{x}{y}$"), + "{}", + converted.markdown + ); + assert!( + converted.markdown.contains("Price: $20.00"), + "{}", + converted.markdown + ); + } + #[cfg(feature = "document-read")] #[test] fn repeated_conversion_reuses_cached_markdown_for_offset_reads() { diff --git a/src/crates/execution/tool-execution/src/fs/read_file.rs b/src/crates/execution/tool-execution/src/fs/read_file.rs index 75fd3793cb..9c37c2e4aa 100644 --- a/src/crates/execution/tool-execution/src/fs/read_file.rs +++ b/src/crates/execution/tool-execution/src/fs/read_file.rs @@ -11,12 +11,15 @@ pub struct ReadFileResult { pub hit_total_char_limit: bool, /// True when the returned view omits characters from selected lines. pub content_truncated: bool, + /// Selected source lines whose contents were clipped by the per-line limit. + pub truncated_lines: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] pub struct ReadFilePresentation { pub result_for_assistant: String, pub lines_read: usize, + pub next_offset: Option, } fn read_file_lines_read(result: &ReadFileResult) -> usize { @@ -31,12 +34,22 @@ pub fn build_read_file_presentation( logical_path: &str, result: &ReadFileResult, ) -> ReadFilePresentation { - let mut result_for_assistant = format!( - "Read lines {}-{} from {} ({} total lines)\n\n{}\n", - result.start_line, result.end_line, logical_path, result.total_lines, result.content - ); + let lines_read = read_file_lines_read(result); + let mut result_for_assistant = if result.total_lines == 0 { + format!("{logical_path} is empty (0 lines).") + } else if lines_read == 0 { + format!( + "No lines from {logical_path} fit within the Read output limit ({} total lines). Use another inspection method; a smaller line window cannot make a single line fit.", + result.total_lines, + ) + } else { + format!( + "Read lines {}-{} from {} ({} total lines)\n\n{}\n", + result.start_line, result.end_line, logical_path, result.total_lines, result.content + ) + }; - let has_more = result.end_line < result.total_lines; + let has_more = lines_read > 0 && result.end_line < result.total_lines; let next_start_line = has_more.then_some(result.end_line + 1); if let Some(next_start) = next_start_line { if result.hit_total_char_limit { @@ -51,10 +64,17 @@ pub fn build_read_file_presentation( )); } } + if !result.truncated_lines.is_empty() { + result_for_assistant.push_str(&format!( + "\n\n[Lines {:?} contain truncated text. offset/limit cannot restore those characters. Use another inspection method before copying source text for Edit.]", + result.truncated_lines, + )); + } ReadFilePresentation { result_for_assistant, - lines_read: read_file_lines_read(result), + lines_read, + next_offset: next_start_line, } } @@ -398,6 +418,7 @@ struct ReadLineSelection { selected_chars: usize, hit_total_char_limit: bool, content_truncated: bool, + truncated_lines: Vec, tail_lines: Option>, } @@ -432,6 +453,7 @@ impl ReadLineSelection { selected_chars: 0, hit_total_char_limit: false, content_truncated: false, + truncated_lines: Vec::new(), tail_lines: tail.then(VecDeque::new), }) } @@ -471,6 +493,9 @@ impl ReadLineSelection { self.content_truncated = true; } else { self.content_truncated |= truncated; + if truncated { + self.truncated_lines.push(line_number); + } self.selected_chars = next_chars; self.selected_lines.push(rendered); } @@ -485,6 +510,7 @@ impl ReadLineSelection { content: String::new(), hit_total_char_limit: false, content_truncated: false, + truncated_lines: Vec::new(), }); } if let Some(tail_lines) = self.tail_lines.take() { @@ -506,7 +532,7 @@ impl ReadLineSelection { )); } let end_line = if self.selected_lines.is_empty() { - self.start_line + self.start_line - 1 } else { self.start_line .saturating_add(self.selected_lines.len()) @@ -519,6 +545,7 @@ impl ReadLineSelection { content: self.selected_lines.join("\n"), hit_total_char_limit: self.hit_total_char_limit, content_truncated: self.content_truncated, + truncated_lines: self.truncated_lines, }) } } @@ -539,6 +566,7 @@ mod tests { let text = "abcdef\nlast\n"; let truncated = read_text(text, 1, 2, 4, 100).unwrap(); assert!(truncated.content_truncated); + assert_eq!(truncated.truncated_lines, vec![1]); assert!(!truncated.hit_total_char_limit); let tail = read_text_tail(text, 1, 4, 100).unwrap(); assert!( @@ -547,9 +575,12 @@ mod tests { ); let offset = read_text(text, 2, 1, 4, 100).unwrap(); assert!(!offset.content_truncated); + assert!(offset.truncated_lines.is_empty()); + assert!(tail.truncated_lines.is_empty()); let budget = read_text(text, 1, 2, 50, 14).unwrap(); assert!(budget.hit_total_char_limit); assert!(budget.content_truncated); + assert!(budget.truncated_lines.is_empty()); assert!( !read_text("literal [truncated]", 1, 1, 100, 100) .unwrap() @@ -809,11 +840,13 @@ mod tests { content: " 1\tone\n 2\ttwo".to_string(), hit_total_char_limit: false, content_truncated: false, + truncated_lines: Vec::new(), }; let presentation = build_read_file_presentation("src/lib.rs", &result); assert_eq!(presentation.lines_read, 2); + assert_eq!(presentation.next_offset, Some(3)); assert!(presentation .result_for_assistant .contains("Read lines 1-2 from src/lib.rs (4 total lines)")); @@ -846,8 +879,36 @@ mod tests { content: String::new(), hit_total_char_limit: false, content_truncated: false, + truncated_lines: Vec::new(), }; assert_eq!(read_file_lines_read(&result), 0); + let presentation = build_read_file_presentation("empty.txt", &result); + assert_eq!(presentation.next_offset, None); + assert!(presentation.result_for_assistant.contains("is empty")); + } + + #[test] + fn a_line_that_cannot_fit_does_not_skip_to_the_next_line() { + let result = read_text("abcdefghij\nnext\n", 1, 2, 100, 10).unwrap(); + let presentation = build_read_file_presentation("long.txt", &result); + assert!(result.content.is_empty()); + assert_eq!(presentation.lines_read, 0); + assert_eq!(presentation.next_offset, None); + assert!(presentation.result_for_assistant.contains("No lines")); + assert!(!presentation.result_for_assistant.contains("offset=2")); + } + + #[test] + fn clipped_line_guidance_does_not_promise_paging_can_restore_characters() { + let result = read_text("abcdefghij\nnext\n", 1, 1, 4, 100).unwrap(); + let presentation = build_read_file_presentation("long.txt", &result); + assert_eq!(presentation.next_offset, Some(2)); + assert!(presentation + .result_for_assistant + .contains("Lines [1] contain truncated text")); + assert!(presentation + .result_for_assistant + .contains("cannot restore those characters")); } } From cf8e3f977d7cfeb62e53ec849ebf1709c6adffdb Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Sat, 10 Oct 2026 18:15:58 +0800 Subject: [PATCH 2/2] feat(read): select document pages and native Office units --- Cargo.lock | 2 + Cargo.toml | 1 + .../tools/implementations/file_read_tool.rs | 245 +++++++++- src/crates/execution/tool-execution/AGENTS.md | 8 +- .../execution/tool-execution/Cargo.toml | 4 +- .../tool-execution/src/fs/document.rs | 130 +++++- .../tool-execution/src/fs/document/office.rs | 423 ++++++++++++++++++ .../src/fs/document/selection.rs | 111 +++++ .../src/fs/document/text_pages.rs | 71 +++ 9 files changed, 969 insertions(+), 26 deletions(-) create mode 100644 src/crates/execution/tool-execution/src/fs/document/office.rs create mode 100644 src/crates/execution/tool-execution/src/fs/document/selection.rs create mode 100644 src/crates/execution/tool-execution/src/fs/document/text_pages.rs diff --git a/Cargo.lock b/Cargo.lock index ba111a33fc..8743218fae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11334,6 +11334,7 @@ dependencies = [ "pdf-inspector", "readability-js", "regex", + "roxmltree", "serde", "serde_json", "sha2", @@ -11341,6 +11342,7 @@ dependencies = [ "tokio-util", "vte", "windows 0.61.3", + "zip 4.6.1", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0b52ae007e..b36f44ebfa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,6 +102,7 @@ serde_yaml = "0.9" # Document conversion anydoc = "=0.2.4" pdf-inspector = { version = "1.17.0", default-features = false } +roxmltree = { version = "0.21.1", default-features = false } # TypeScript binding generation (schema-first; gated by per-crate `ts` features) ts-rs = { version = "12", features = ["serde-json-impl", "no-serde-warnings"] } diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs index 59b1c919a3..84ee1c2d7a 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/file_read_tool.rs @@ -25,10 +25,12 @@ use std::time::Duration; use std::time::Instant; #[cfg(feature = "document-read")] use tool_runtime::fs::document::{ - convert_document_to_markdown, DocumentConversionError, MAX_DOCUMENT_INPUT_BYTES, - MAX_DOCUMENT_MARKDOWN_BYTES, + convert_document_pages_to_markdown, DocumentConversionError, MAX_DOCUMENT_INPUT_BYTES, + MAX_DOCUMENT_MARKDOWN_BYTES, TEXT_CHUNK_CHARS, +}; +use tool_runtime::fs::document::{ + is_supported_document_path, DocumentPageSelection, DocumentPagination, PdfTextCoverage, }; -use tool_runtime::fs::document::{is_supported_document_path, PdfTextCoverage}; use tool_runtime::fs::read_file::{ build_read_file_presentation, read_file_from_reader, read_file_tail_from_reader, ReadFileResult, }; @@ -54,6 +56,7 @@ struct DocumentReadMetadata { source_format: &'static str, source_size_bytes: usize, pdf_coverage: Option, + pagination: DocumentPagination, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -146,6 +149,30 @@ impl FileReadTool { .is_some_and(|extension| extension.eq_ignore_ascii_case("csv")) } + fn read_page_selection(input: &Value) -> Result, String> { + let Some(value) = input.get("pages") else { + return Ok(None); + }; + #[cfg(not(feature = "document-read"))] + { + let _ = value; + Err("Document page selection is not available in this product build".to_string()) + } + #[cfg(feature = "document-read")] + { + if Self::read_render_mode(input)? == ReadRenderMode::Source { + return Err( + "pages selects extracted document units and cannot be used with render=source" + .to_string(), + ); + } + let value = value + .as_str() + .ok_or_else(|| "pages must be a string such as '1-3,7'".to_string())?; + DocumentPageSelection::parse(value).map(Some) + } + } + fn optional_line_number(input: &Value, key: &str) -> Result, String> { match input.get(key) { Some(value) => Self::line_number_from_value(value) @@ -188,6 +215,7 @@ impl FileReadTool { start_line: usize, limit: usize, tail: bool, + pages: Option, filesystem: &dyn crate::agentic::workspace::WorkspaceFileSystem, context: &ToolUseContext, ) -> OpenBitFunResult<(ReadFileResult, DocumentReadMetadata)> { @@ -202,7 +230,7 @@ impl FileReadTool { })? .ok_or_else(|| { OpenBitFunError::tool(format!( - "Document {} is larger than the {} MiB Read limit. Use a smaller document or a specialized extraction workflow; offset/limit only change the returned text window.", + "Document {} is larger than the {} MiB Read limit. Use a smaller document or a specialized extraction workflow; pages/offset/limit do not reduce source file transfer.", logical_path, MAX_DOCUMENT_INPUT_BYTES / (1024 * 1024) )) @@ -219,7 +247,7 @@ impl FileReadTool { ); let conversion = tokio::time::timeout( DOCUMENT_CONVERSION_TIMEOUT, - convert_document_to_markdown(bytes, resolved_path.to_string()), + convert_document_pages_to_markdown(bytes, resolved_path.to_string(), pages), ) .await .map_err(|_| { @@ -280,6 +308,7 @@ impl FileReadTool { source_format: converted.source_format, source_size_bytes, pdf_coverage: converted.pdf_coverage, + pagination: converted.pagination, }, )) } @@ -292,7 +321,7 @@ impl FileReadTool { let recovery = match error.code() { "encrypted" => " Use an unlocked copy of the document.", "unsupported" => " For a text file, use render=source; otherwise use a format-specific extraction tool.", - "resourceLimit" => " Use a smaller document or a specialized extraction workflow; offset/limit only change the returned text window.", + "resourceLimit" => " For an extracted-output limit, request fewer pages; this does not bypass source-size or parser limits. Otherwise use a smaller document or specialized extraction workflow.", _ => "", }; OpenBitFunError::tool(format!( @@ -316,8 +345,9 @@ impl Tool for FileReadTool { let document_guidance = format!( r#" Documents: Word, PowerPoint, Excel, OpenDocument, RTF, EPUB and PDF are extracted offline as Markdown. PDF page markers identify source pages; extraction status and missing-page warnings describe coverage. Use available text, and seek OCR or visual inspection only if missing pages matter to the task. Read itself does not perform OCR. Other embedded images/objects may be represented only by available text. -For documents, line windows address extracted Markdown, not source pages or spreadsheet rows. Extracted text is not exact source for Edit. Input limit: {} MiB; extracted Markdown limit: {} MiB. Smaller line windows do not reduce conversion work. +Use pages to select units after inspecting page_kind/page_count, especially when a large document is truncated. PDF units are original pages; PPTX slides and XLSX visible sheets follow source order. DOCX and other formats use {}-character text chunks, not printed pages. offset/limit/tail then address Markdown lines within that selection; keep the same pages when following next_offset. Extracted text is not exact source for Edit. Input limit: {} MiB; selected Markdown limit: {} MiB. Selection does not bypass source transfer limits; parsing may still process the full document. "#, + TEXT_CHUNK_CHARS, MAX_DOCUMENT_INPUT_BYTES / (1024 * 1024), MAX_DOCUMENT_MARKDOWN_BYTES / (1024 * 1024), ); @@ -371,6 +401,10 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th "enum": ["auto", "source", "markdown"], "description": "auto (default): extract known documents, preserve CSV source. source: read text without conversion. markdown: extract a document by content, including misnamed files, or convert CSV to a table." }); + schema["properties"]["pages"] = json!({ + "type": "string", + "description": "Optional 1-based document units, e.g. '1-3,7', in source order. See returned page_kind and page_count. Omit for the full extraction. Requires document rendering; offset/limit/tail apply within the selected units." + }); schema }; #[cfg(not(feature = "document-read"))] @@ -434,6 +468,7 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th if let Err(message) = Self::read_tail_mode(input) .and_then(|_| Self::read_window_start_line(input)) .and_then(|_| Self::read_render_mode(input)) + .and_then(|_| Self::read_page_selection(input)) { return ValidationResult { result: false, @@ -591,6 +626,7 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th let tail = Self::read_tail_mode(input).map_err(OpenBitFunError::tool)?; let render_mode = Self::read_render_mode(input).map_err(OpenBitFunError::tool)?; + let pages = Self::read_page_selection(input).map_err(OpenBitFunError::tool)?; let start_line = Self::read_window_start_line(input).map_err(OpenBitFunError::tool)?; let limit = input @@ -602,6 +638,11 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th context.enforce_path_operation(ToolPathOperation::Read, &resolved)?; #[cfg(feature = "tools-miniapp")] if is_virtual_context_path(context, &resolved) { + if pages.is_some() { + return Err(OpenBitFunError::tool( + "pages cannot select a MiniApp text context; use offset/limit".to_string(), + )); + } let content = virtual_context_file(context, &resolved).ok_or_else(|| { OpenBitFunError::tool(format!( "MiniApp context file is unavailable: {}", @@ -663,6 +704,9 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th ReadRenderMode::Source => false, ReadRenderMode::Markdown => true, }; + if pages.is_some() && !reads_document_representation { + return Err(OpenBitFunError::tool("pages requires a document. For a misnamed document or CSV use render=markdown; for ordinary text use offset/limit".to_string())); + } #[cfg(not(feature = "document-read"))] if reads_document_representation { return Err(OpenBitFunError::tool(format!( @@ -694,6 +738,7 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th start_line, limit, tail, + pages, filesystem.as_ref(), context, ) @@ -788,6 +833,12 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th data["representation"] = json!("extracted_markdown"); data["source_format"] = json!(metadata.source_format); data["source_size_bytes"] = json!(metadata.source_size_bytes); + let pagination = metadata.pagination; + data["page_kind"] = json!(pagination.page_kind); + data["page_count"] = json!(pagination.page_count); + data["selected_pages"] = json!(pagination.selected_pages); + data["selected_page_count"] = json!(pagination.selected_page_count); + data["next_page"] = json!(pagination.next_page); let warnings = if let Some(coverage) = metadata.pdf_coverage { data["conversion_engine"] = json!("pdf-inspector"); data["extraction_status"] = json!(coverage.status()); @@ -800,14 +851,14 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th ]; if !coverage.pages_needing_ocr.is_empty() { warnings.push(format!( - "{} of {} PDF pages have reliable text. Pages {} need OCR or visual inspection; their contents are missing from this extraction. Use the available text, and inspect missing pages if the task requires them. Repeating Read will not perform OCR.", + "{} of {} PDF pages have reliable text in this selection. Pages {} need OCR or visual inspection; their contents are missing from this extraction. Use the available text, and inspect missing pages if the task requires them. Repeating Read will not perform OCR.", coverage.extracted_pages.len(), - coverage.page_count, + pagination.selected_page_count, coverage.ocr_page_ranges(MAX_PDF_OCR_PAGE_RANGES), )); } result_for_assistant = format!( - "PDF text extraction: {} ({} source pages). Page markers refer to the PDF; offset/limit refer to Markdown lines.\n{}\n{}", + "PDF text extraction: {} for the selected pages ({} source pages total). Page markers refer to the PDF; offset/limit refer to Markdown lines.\n{}\n{}", coverage.status(), coverage.page_count, warnings.join("\n"), @@ -833,6 +884,28 @@ Returns numbered lines (line number, tab, text). Choose the window needed for th warnings }; data["extraction_warnings"] = json!(warnings); + let unit_hint = match pagination.page_kind { + "pdf_page" => "original PDF pages", + "slide" => "slides in presentation order", + "sheet" => { + "visible sheets in workbook order; hidden sheets/rows/columns are not extracted" + } + _ => "text chunks, not printed pages", + }; + let continuation = if let Some(next_offset) = presentation.next_offset { + let same_selection = if input.get("pages").is_some() { + format!("keep pages=\"{}\"", pagination.selected_pages) + } else { + "keep pages omitted".to_string() + }; + format!("To continue this extraction, {same_selection} and use offset={next_offset}; or choose a narrower pages range starting at offset=1.") + } else { + "Choose another pages range if more context is needed.".to_string() + }; + result_for_assistant = format!( + "Document pages: kind={}, count={}, selected=\"{}\" ({unit_hint}). Selection metadata does not mean every selected unit is in this line window. {continuation}\n{result_for_assistant}", + pagination.page_kind, pagination.page_count, pagination.selected_pages, + ); } let result = ToolResult::Result { @@ -1251,6 +1324,8 @@ mod tests { .expect("properties"); assert_eq!(properties["render"]["enum"], json!(["auto", "source"])); + assert!(!properties.contains_key("pages")); + assert!(FileReadTool::read_page_selection(&json!({"pages":"1"})).is_err()); assert!(!properties["render"]["description"] .as_str() .expect("render description") @@ -1737,4 +1812,154 @@ mod tests { .as_str() .is_some_and(|content| content.contains("Hello from remote RTF"))); } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn pdf_page_selection_preserves_source_numbers_and_scopes_ocr_for_local_and_remote() { + let dir = tempfile::tempdir().unwrap(); + let source = pdf_document(&[PDF_TEXT_PAGE, PDF_SCANNED_PAGE, PDF_TEXT_PAGE]); + fs::write(dir.path().join("document.pdf"), &source).unwrap(); + let bounded_limit = Arc::new(AtomicUsize::new(0)); + let contexts = [ + local_context(dir.path().to_path_buf()), + remote_context(source, Arc::clone(&bounded_limit)), + ]; + let tool = FileReadTool::new(); + for context in contexts { + for (pages, status, extracted, missing) in [ + ("3,1", "complete", json!([1, 3]), json!([])), + ("2", "needs_ocr", json!([]), json!([2])), + ("3", "complete", json!([3]), json!([])), + ] { + let results = tool + .call_impl( + &json!({"file_path":"document.pdf", "pages":pages}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &results[0] + else { + panic!("result"); + }; + assert_eq!(data["page_kind"], "pdf_page"); + assert_eq!(data["page_count"], 3); + assert_eq!(data["extraction_status"], status); + assert_eq!(data["extracted_pages"], extracted); + assert_eq!(data["pages_needing_ocr"], missing); + assert!(result_for_assistant + .as_deref() + .unwrap() + .contains("for the selected pages")); + if pages == "3" { + let content = data["content"].as_str().unwrap(); + assert!(content.contains("## PDF page 3")); + assert!(!content.contains("## PDF page 1")); + assert_eq!(data["selected_pages"], "3"); + assert_eq!(data["selected_page_count"], 1); + } + } + let error = tool + .call_impl(&json!({"file_path":"document.pdf", "pages":"4"}), &context) + .await + .unwrap_err(); + assert!( + error.to_string().contains("has 3 selectable pages"), + "{error}" + ); + } + assert_eq!( + bounded_limit.load(Ordering::Relaxed), + MAX_DOCUMENT_INPUT_BYTES + ); + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn document_chunk_windows_continue_with_the_same_selection_and_recover_long_lines() { + let source = format!("{{\\rtf1\\ansi {}Final paragraph}}", "x".repeat(4000)).into_bytes(); + let context = remote_context(source, Arc::new(AtomicUsize::new(0))); + let tool = FileReadTool::new(); + let initial = tool + .call_impl(&json!({"file_path":"report.rtf"}), &context) + .await + .unwrap(); + let ToolResult::Result { data, .. } = &initial[0] else { + panic!("result"); + }; + assert_eq!(data["page_kind"], "text_chunk"); + assert_eq!(data["page_count"], 3); + assert!(!data["truncated_lines"].as_array().unwrap().is_empty()); + let selected = tool + .call_impl( + &json!({"file_path":"report.rtf", "pages":"3", "limit":2}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { + data, + result_for_assistant, + .. + } = &selected[0] + else { + panic!("result"); + }; + assert_eq!(data["next_offset"], 3); + assert!(result_for_assistant + .as_deref() + .unwrap() + .contains("keep pages=\"3\"")); + let continued = tool + .call_impl( + &json!({"file_path":"report.rtf", "pages":"3", "offset":data["next_offset"]}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { data, .. } = &continued[0] else { + panic!("result"); + }; + assert!(data["content"] + .as_str() + .unwrap() + .contains("Final paragraph")); + assert_eq!(data["truncated_lines"], json!([])); + let tail = tool + .call_impl( + &json!({"file_path":"report.rtf", "pages":"3", "tail":true, "limit":2}), + &context, + ) + .await + .unwrap(); + let ToolResult::Result { data, .. } = &tail[0] else { + panic!("result"); + }; + assert!(data["content"] + .as_str() + .unwrap() + .contains("Final paragraph")); + } + + #[cfg(feature = "document-read")] + #[tokio::test] + async fn page_arguments_fail_explicitly_instead_of_falling_back_to_text() { + let context = remote_context(b"plain text".to_vec(), Arc::new(AtomicUsize::new(0))); + let tool = FileReadTool::new(); + for input in [ + json!({"file_path":"report.pdf", "pages":3}), + json!({"file_path":"report.pdf", "pages":"0"}), + json!({"file_path":"report.pdf", "pages":"3-1"}), + json!({"file_path":"report.pdf", "pages":"1", "render":"source"}), + json!({"file_path":"plain.txt", "pages":"1"}), + ] { + let error = tool.call_impl(&input, &context).await.unwrap_err(); + assert!(error.to_string().contains("pages"), "{error}"); + } + assert_eq!(tool.input_schema()["properties"]["pages"]["type"], "string"); + } } diff --git a/src/crates/execution/tool-execution/AGENTS.md b/src/crates/execution/tool-execution/AGENTS.md index 4e2c9af4ad..1785ebed34 100644 --- a/src/crates/execution/tool-execution/AGENTS.md +++ b/src/crates/execution/tool-execution/AGENTS.md @@ -35,7 +35,13 @@ agent-facing tool surface. - `document-read` owns offline byte-to-Markdown extraction. Non-PDF formats use anydoc; PDFs use its existing pdf-inspector backend's page API so readable pages survive alongside explicit OCR gaps. Keep source-page coverage separate - from Markdown line-window truncation. This dependency remains optional, with + from Markdown line-window truncation. `pages` selects original PDF pages, + PPTX slides, XLSX visible sheets, or explicitly synthetic text chunks for + reflowable/legacy formats. Office selection filters the in-memory OOXML + manifest while preserving package resources; never infer slides from headings + or claim that Word text chunks are printed pages. Keep normalized selections + in cache keys and apply line windows within the same selected representation. + Whole-source transfer and parser limits still apply. Dependencies remain optional, with OCR/network features disabled; workspace byte IO and model-facing guidance stay with their existing providers and Core tool owner. - Background exec-output and ExecCommand presentation helpers may own retained diff --git a/src/crates/execution/tool-execution/Cargo.toml b/src/crates/execution/tool-execution/Cargo.toml index 75fb044147..afcde39daf 100644 --- a/src/crates/execution/tool-execution/Cargo.toml +++ b/src/crates/execution/tool-execution/Cargo.toml @@ -6,7 +6,7 @@ edition.workspace = true [features] default = [] shell-analysis = [] -document-read = ["dep:anydoc", "dep:pdf-inspector", "dep:sha2"] +document-read = ["dep:anydoc", "dep:pdf-inspector", "dep:roxmltree", "dep:sha2", "dep:zip"] web-readable = ["dep:htmd", "dep:legible", "dep:readability-js"] [dependencies] @@ -26,12 +26,14 @@ legible = { version = "0.4.2", optional = true } log = { workspace = true } pdf-inspector = { workspace = true, optional = true } regex = { workspace = true } +roxmltree = { workspace = true, features = ["std", "positions"], optional = true } serde = { workspace = true } serde_json = { workspace = true } sha2 = { workspace = true, optional = true } tokio = { workspace = true, features = ["io-util", "rt", "sync", "time"] } tokio-util = { workspace = true, features = ["io-util"] } vte = { workspace = true, features = ["ansi"] } +zip = { workspace = true, optional = true } [target.'cfg(windows)'.dependencies] windows = { workspace = true, features = [ diff --git a/src/crates/execution/tool-execution/src/fs/document.rs b/src/crates/execution/tool-execution/src/fs/document.rs index 75a3ddefb8..4e752d73b2 100644 --- a/src/crates/execution/tool-execution/src/fs/document.rs +++ b/src/crates/execution/tool-execution/src/fs/document.rs @@ -1,5 +1,14 @@ //! Document path recognition and optional provider-neutral Markdown conversion. +mod selection; +pub use selection::DocumentPageSelection; +#[cfg(feature = "document-read")] +mod office; +#[cfg(feature = "document-read")] +mod text_pages; +#[cfg(feature = "document-read")] +pub use text_pages::TEXT_CHUNK_CHARS; + #[cfg(feature = "document-read")] use std::collections::VecDeque; #[cfg(feature = "document-read")] @@ -39,6 +48,47 @@ pub struct ConvertedDocument { pub markdown: Arc, pub source_format: &'static str, pub pdf_coverage: Option, + pub pagination: DocumentPagination, +} + +/// Selectable source units (or explicitly synthetic text chunks), not the returned line window. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DocumentPagination { + pub page_kind: &'static str, + pub page_count: usize, + pub selected_pages: String, + pub selected_page_count: usize, + pub next_page: Option, +} + +#[cfg(feature = "document-read")] +impl DocumentPagination { + fn new( + kind: &'static str, + count: usize, + selection: Option<&DocumentPageSelection>, + ) -> Result { + let pages = selection + .map(|selection| selection.resolve(count)) + .transpose() + .map_err(|error| DocumentConversionError::new("invalidPages", error))?; + let last = pages.as_ref().and_then(|pages| pages.last()).copied(); + Ok(Self { + page_kind: kind, + page_count: count, + selected_pages: selection + .map(ToString::to_string) + .unwrap_or_else(|| match count { + 0 => String::new(), + 1 => "1".to_string(), + _ => format!("1-{count}"), + }), + selected_page_count: pages.as_ref().map_or(count, Vec::len), + next_page: last + .filter(|last| (*last as usize) < count) + .and_then(|last| last.checked_add(1)), + }) + } } /// Source-page coverage, independent of the line window returned from the extracted Markdown. @@ -88,10 +138,11 @@ impl PdfTextCoverage { } #[cfg(feature = "document-read")] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] struct DocumentCacheKey { source_sha256: [u8; 32], format: Format, + selection: Option, } #[cfg(feature = "document-read")] @@ -109,8 +160,8 @@ struct DocumentCache { #[cfg(feature = "document-read")] impl DocumentCache { - fn get(&mut self, key: DocumentCacheKey) -> Option { - let index = self.entries.iter().position(|entry| entry.key == key)?; + fn get(&mut self, key: &DocumentCacheKey) -> Option { + let index = self.entries.iter().position(|entry| &entry.key == key)?; let entry = self.entries.remove(index)?; let document = entry.document.clone(); self.entries.push_back(entry); @@ -190,6 +241,16 @@ pub fn is_supported_document_path(path: &str) -> bool { pub async fn convert_document_to_markdown( bytes: Vec, path_hint: String, +) -> Result { + convert_document_pages_to_markdown(bytes, path_hint, None).await +} + +/// Select document units before applying the caller's Markdown line window. +#[cfg(feature = "document-read")] +pub async fn convert_document_pages_to_markdown( + bytes: Vec, + path_hint: String, + selection: Option, ) -> Result { if bytes.len() > MAX_DOCUMENT_INPUT_BYTES { return Err(DocumentConversionError::new( @@ -216,7 +277,7 @@ pub async fn convert_document_to_markdown( // Keep the permit inside the blocking task. If the async caller is cancelled, the parser // still occupies its bounded slot until the synchronous conversion actually exits. let _permit = permit; - convert_document_to_markdown_sync(&bytes, &path_hint) + convert_document_pages_to_markdown_sync(&bytes, &path_hint, selection.as_ref()) }) .await .map_err(|error| { @@ -234,9 +295,10 @@ fn document_conversion_semaphore() -> &'static Arc { } #[cfg(feature = "document-read")] -fn convert_document_to_markdown_sync( +fn convert_document_pages_to_markdown_sync( bytes: &[u8], path_hint: &str, + selection: Option<&DocumentPageSelection>, ) -> Result { let format = Format::from_bytes(bytes) .or_else(|| Format::from_path(Path::new(path_hint))) @@ -249,23 +311,35 @@ fn convert_document_to_markdown_sync( let cache_key = DocumentCacheKey { source_sha256: Sha256::digest(bytes).into(), format, + selection: selection.cloned(), }; if let Some(document) = document_cache() .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()) - .get(cache_key) + .get(&cache_key) { return Ok(document); } let source_format = format_name(format); - let (markdown, pdf_coverage) = if format == Format::Pdf { - let (markdown, coverage) = extract_pdf_text(bytes)?; - (markdown, Some(coverage)) + let (markdown, pdf_coverage, pagination) = if format == Format::Pdf { + let (markdown, coverage) = extract_pdf_text(bytes, selection)?; + let pagination = DocumentPagination::new("pdf_page", coverage.page_count, selection)?; + (markdown, Some(coverage), pagination) + } else if let Some(index) = office::OfficePageIndex::read(bytes, format)? { + let pagination = DocumentPagination::new(index.page_kind, index.page_count(), selection)?; + let selected_bytes = selection + .map(|selection| index.select(bytes, selection)) + .transpose()?; + let markdown = + anydoc::to_markdown_bytes(selected_bytes.as_deref().unwrap_or(bytes), format) + .map_err(|error| DocumentConversionError::new(error.code(), error.to_string()))?; + (markdown, None, pagination) } else { let markdown = anydoc::to_markdown_bytes(bytes, format) .map_err(|error| DocumentConversionError::new(error.code(), error.to_string()))?; - (markdown, None) + let (markdown, pagination) = text_pages::select_text(markdown, selection)?; + (markdown, None, pagination) }; if markdown.len() > MAX_DOCUMENT_MARKDOWN_BYTES { return Err(DocumentConversionError::new( @@ -281,6 +355,7 @@ fn convert_document_to_markdown_sync( markdown: Arc::from(markdown), source_format, pdf_coverage, + pagination, }; document_cache() .lock() @@ -292,15 +367,34 @@ fn convert_document_to_markdown_sync( /// anydoc's PDF convenience API rejects the whole document when any page needs OCR. /// Use the same parser's page API to retain reliable text and explicitly mark gaps. #[cfg(feature = "document-read")] -fn extract_pdf_text(bytes: &[u8]) -> Result<(String, PdfTextCoverage), DocumentConversionError> { - let extraction = pdf_inspector::extract_pages_markdown_mem(bytes, None).map_err(|error| { +fn extract_pdf_text( + bytes: &[u8], + selection: Option<&DocumentPageSelection>, +) -> Result<(String, PdfTextCoverage), DocumentConversionError> { + let map_error = |error: pdf_inspector::PdfError| { let code = match &error { pdf_inspector::PdfError::Encrypted => "encrypted", pdf_inspector::PdfError::Io(_) => "io", _ => "malformed", }; DocumentConversionError::new(code, error.to_string()) - })?; + }; + // Validate against the parsed source page tree before passing zero-based indices to the + // backend, which otherwise returns an OCR placeholder for a nonexistent page. + let source_page_count = selection + .map(|_| { + pdf_inspector::classify_pdf_mem(bytes) + .map(|classification| classification.page_count as usize) + .map_err(map_error) + }) + .transpose()?; + let selected_pages = selection + .map(|selection| selection.resolve(source_page_count.unwrap_or(0))) + .transpose() + .map_err(|error| DocumentConversionError::new("invalidPages", error))? + .map(|pages| pages.into_iter().map(|page| page - 1).collect::>()); + let extraction = pdf_inspector::extract_pages_markdown_mem(bytes, selected_pages.as_deref()) + .map_err(map_error)?; if extraction.pages.is_empty() { return Err(DocumentConversionError::new( "malformed", @@ -308,7 +402,7 @@ fn extract_pdf_text(bytes: &[u8]) -> Result<(String, PdfTextCoverage), DocumentC )); } let mut coverage = PdfTextCoverage { - page_count: extraction.pages.len(), + page_count: source_page_count.unwrap_or(extraction.pages.len()), extracted_pages: Vec::new(), pages_needing_ocr: Vec::new(), }; @@ -364,6 +458,14 @@ fn format_name(format: Format) -> &'static str { mod tests { use super::*; + #[cfg(feature = "document-read")] + fn convert_document_to_markdown_sync( + bytes: &[u8], + path: &str, + ) -> Result { + convert_document_pages_to_markdown_sync(bytes, path, None) + } + #[test] fn pdf_gap_summary_is_bounded_without_losing_page_metadata() { let coverage = PdfTextCoverage { diff --git a/src/crates/execution/tool-execution/src/fs/document/office.rs b/src/crates/execution/tool-execution/src/fs/document/office.rs new file mode 100644 index 0000000000..0aabbc1946 --- /dev/null +++ b/src/crates/execution/tool-execution/src/fs/document/office.rs @@ -0,0 +1,423 @@ +//! Select OOXML slides/sheets by their manifest order without reimplementing document rendering. +//! Only the in-memory manifest changes; relationships, styles, shared strings and assets survive. + +use super::{DocumentConversionError, DocumentPageSelection}; +use anydoc::Format; +use roxmltree::{Document, Node, ParsingOptions}; +use std::io::{Cursor, Read, Write}; +use std::ops::Range; +use zip::{write::SimpleFileOptions, ZipArchive, ZipWriter}; + +const MAX_MANIFEST_BYTES: usize = 2 * 1024 * 1024; +const PRESENTATION_NS: [&str; 2] = [ + "http://schemas.openxmlformats.org/presentationml/2006/main", + "http://purl.oclc.org/ooxml/presentationml/main", +]; +const SHEET_NS: [&str; 2] = [ + "http://schemas.openxmlformats.org/spreadsheetml/2006/main", + "http://purl.oclc.org/ooxml/spreadsheetml/main", +]; + +pub(super) struct OfficePageIndex { + pub page_kind: &'static str, + part: String, + xml: String, + units: Vec<(Range, Option)>, +} + +impl OfficePageIndex { + pub(super) fn read( + bytes: &[u8], + format: Format, + ) -> Result, DocumentConversionError> { + if !matches!(format, Format::Pptx | Format::Excel) || !bytes.starts_with(b"PK") { + return Ok(None); + } + let mut archive = ZipArchive::new(Cursor::new(bytes)).map_err(malformed)?; + let mut part = None; + if let Some(xml) = read_xml(&mut archive, "_rels/.rels")? { + let doc = parse(&xml)?; + for rel in doc + .root_element() + .children() + .filter(|node| node.is_element()) + { + if rel.tag_name().name() == "Relationship" + && rel.attribute("Type").is_some_and(|kind| matches!(kind, + "http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument" | + "http://purl.oclc.org/ooxml/officeDocument/relationships/officeDocument")) + && rel.attribute("TargetMode") != Some("External") + { + part = rel.attribute("Target").map(resolve_root_part).transpose()?; + break; + } + } + } + let part = part.unwrap_or_else(|| { + if format == Format::Pptx { + "ppt/presentation.xml" + } else { + "xl/workbook.xml" + } + .to_string() + }); + // Binary Excel containers have no XML sheet manifest; advertise text chunks for them. + if part.ends_with(".bin") || (format == Format::Excel && archive.by_name(&part).is_err()) { + return Ok(None); + } + let xml = + read_xml(&mut archive, &part)?.ok_or_else(|| malformed("missing Office manifest"))?; + let doc = parse(&xml)?; + let (namespaces, root_name, list_name, unit_name, page_kind) = if format == Format::Pptx { + ( + &PRESENTATION_NS, + "presentation", + "sldIdLst", + "sldId", + "slide", + ) + } else { + (&SHEET_NS, "workbook", "sheets", "sheet", "sheet") + }; + if !matches_element(doc.root_element(), namespaces, root_name) { + return Err(malformed("unexpected Office manifest root")); + } + let mut count = 0; + let units = doc + .root_element() + .children() + .find(|node| matches_element(*node, namespaces, list_name)) + .into_iter() + .flat_map(|list| list.children()) + .filter(|node| matches_element(*node, namespaces, unit_name)) + .map(|node| { + let visible = page_kind != "sheet" + || !matches!(node.attribute("state"), Some("hidden" | "veryHidden")); + let number = visible.then(|| { + count += 1; + count + }); + (node.range(), number) + }) + .collect(); + Ok(Some(Self { + page_kind, + part, + xml, + units, + })) + } + + pub(super) fn page_count(&self) -> usize { + self.units.iter().filter(|(_, page)| page.is_some()).count() + } + + pub(super) fn select( + &self, + bytes: &[u8], + selection: &DocumentPageSelection, + ) -> Result, DocumentConversionError> { + let selected = selection + .resolve(self.page_count()) + .map_err(|error| DocumentConversionError::new("invalidPages", error))?; + let mut xml = String::with_capacity(self.xml.len()); + let mut cursor = 0; + for (range, number) in &self.units { + xml.push_str(&self.xml[cursor..range.start]); + if number.is_some_and(|number| selected.binary_search(&number).is_ok()) { + xml.push_str(&self.xml[range.clone()]); + } + cursor = range.end; + } + xml.push_str(&self.xml[cursor..]); + + let mut archive = ZipArchive::new(Cursor::new(bytes)).map_err(malformed)?; + let mut output = ZipWriter::new(Cursor::new(Vec::with_capacity(bytes.len()))); + for index in 0..archive.len() { + let file = archive.by_index_raw(index).map_err(malformed)?; + if file.name() == self.part { + output + .start_file(&self.part, SimpleFileOptions::default()) + .map_err(malformed)?; + output.write_all(xml.as_bytes()).map_err(malformed)?; + } else { + // Copy compressed bytes, never inflate unselected slides or embedded media. + output.raw_copy_file(file).map_err(malformed)?; + } + } + Ok(output.finish().map_err(malformed)?.into_inner()) + } +} + +fn matches_element(node: Node<'_, '_>, namespaces: &[&str], name: &str) -> bool { + node.is_element() + && node.tag_name().name() == name + && node + .tag_name() + .namespace() + .is_some_and(|namespace| namespaces.contains(&namespace)) +} + +fn parse(xml: &str) -> Result, DocumentConversionError> { + Document::parse_with_options( + xml, + ParsingOptions { + nodes_limit: 100_000, + ..Default::default() + }, + ) + .map_err(malformed) +} + +fn read_xml( + archive: &mut ZipArchive>, + part: &str, +) -> Result, DocumentConversionError> { + let mut file = match archive.by_name(part) { + Ok(file) => file, + Err(zip::result::ZipError::FileNotFound) => return Ok(None), + Err(error) => return Err(malformed(error)), + }; + let mut bytes = Vec::new(); + file.by_ref() + .take((MAX_MANIFEST_BYTES + 1) as u64) + .read_to_end(&mut bytes) + .map_err(malformed)?; + if bytes.len() > MAX_MANIFEST_BYTES { + return Err(DocumentConversionError::new( + "resourceLimit", + "Office page manifest exceeds the 2 MiB indexing limit", + )); + } + let mut xml = if bytes.starts_with(&[0xff, 0xfe]) || bytes.starts_with(b"<\0") { + decode_utf16(bytes.strip_prefix(&[0xff, 0xfe]).unwrap_or(&bytes), true)? + } else if bytes.starts_with(&[0xfe, 0xff]) || bytes.starts_with(b"\0<") { + decode_utf16(bytes.strip_prefix(&[0xfe, 0xff]).unwrap_or(&bytes), false)? + } else { + String::from_utf8( + bytes + .strip_prefix(&[0xef, 0xbb, 0xbf]) + .unwrap_or(&bytes) + .to_vec(), + ) + .map_err(malformed)? + }; + // We serialize UTF-8. Drop the original declaration, which may declare UTF-16. + if xml.starts_with("") + .ok_or_else(|| malformed("unterminated XML declaration"))?; + xml.drain(..end + 2); + } + Ok(Some(xml)) +} + +fn decode_utf16(bytes: &[u8], little: bool) -> Result { + if bytes.len() % 2 != 0 { + return Err(malformed("incomplete UTF-16 manifest")); + } + let units = bytes + .chunks_exact(2) + .map(|pair| { + if little { + u16::from_le_bytes([pair[0], pair[1]]) + } else { + u16::from_be_bytes([pair[0], pair[1]]) + } + }) + .collect::>(); + String::from_utf16(&units).map_err(malformed) +} + +/// OPC targets are URI/POSIX paths on every host OS; no filesystem access occurs here. +fn resolve_root_part(target: &str) -> Result { + let mut decoded = Vec::new(); + let mut bytes = target.as_bytes().iter().copied(); + while let Some(byte) = bytes.next() { + decoded.push(if byte == b'%' { + let hi = bytes.next().and_then(|byte| (byte as char).to_digit(16)); + let lo = bytes.next().and_then(|byte| (byte as char).to_digit(16)); + match (hi, lo) { + (Some(hi), Some(lo)) => (hi * 16 + lo) as u8, + _ => return Err(malformed("invalid escaped Office part path")), + } + } else { + byte + }); + } + let decoded = String::from_utf8(decoded).map_err(malformed)?; + if decoded.contains([':', '\\', '\0', '?', '#']) { + return Err(malformed("invalid Office part path")); + } + let mut parts = Vec::new(); + for part in decoded.split('/') { + match part { + "" | "." => {} + ".." => { + if parts.pop().is_none() { + return Err(malformed("Office part escapes package")); + } + } + _ => parts.push(part), + } + } + Ok(parts.join("/")) +} + +fn malformed(error: impl std::fmt::Display) -> DocumentConversionError { + DocumentConversionError::new("malformed", format!("Office page index: {error}")) +} + +#[cfg(test)] +mod tests { + use super::super::convert_document_pages_to_markdown_sync as convert; + use super::*; + + fn package(parts: Vec<(&str, Vec)>) -> Vec { + let mut writer = ZipWriter::new(Cursor::new(Vec::new())); + for (path, content) in parts { + writer + .start_file(path, SimpleFileOptions::default()) + .unwrap(); + writer.write_all(&content).unwrap(); + } + writer.finish().unwrap().into_inner() + } + + fn root_rels(target: &str) -> Vec { + format!(r#""#).into_bytes() + } + + fn presentation() -> Vec { + let manifest = br#""#; + let rels = br#""#; + let slide = |text| { + format!(r#"{text}"#).into_bytes() + }; + package(vec![ + ("_rels/.rels", root_rels("custom/deck.xml")), + ("custom/deck.xml", manifest.to_vec()), + ("custom/_rels/deck.xml.rels", rels.to_vec()), + ("custom/slides/one.xml", slide("Alpha slide")), + ("custom/slides/two.xml", slide("Beta slide")), + ("media/preserve.bin", vec![1, 2, 3, 4]), + ]) + } + + #[test] + fn presentation_selection_uses_manifest_order_and_isolated_cache_entries() { + let bytes = presentation(); + let all = convert(&bytes, "test.pptx", None).unwrap(); + assert_eq!(all.pagination.page_kind, "slide"); + assert_eq!(all.pagination.page_count, 2); + assert!(all.markdown.contains("Alpha slide") && all.markdown.contains("Beta slide")); + let page1 = DocumentPageSelection::parse("1").unwrap(); + let first = convert(&bytes, "test.pptx", Some(&page1)).unwrap(); + assert!(first.markdown.contains("Beta slide")); + assert!(!first.markdown.contains("Alpha slide")); + let page2 = DocumentPageSelection::parse("2").unwrap(); + let second = convert(&bytes, "test.pptx", Some(&page2)).unwrap(); + assert!(second.markdown.contains("Alpha slide")); + assert!(!second.markdown.contains("Beta slide")); + assert_eq!(second.pagination.next_page, None); + let repeated = convert(&bytes, "test.pptx", Some(&page1)).unwrap(); + assert_eq!(first.markdown, repeated.markdown); + let selected = OfficePageIndex::read(&bytes, Format::Pptx) + .unwrap() + .unwrap() + .select(&bytes, &page1) + .unwrap(); + let mut archive = ZipArchive::new(Cursor::new(selected)).unwrap(); + let mut asset = Vec::new(); + archive + .by_name("media/preserve.bin") + .unwrap() + .read_to_end(&mut asset) + .unwrap(); + assert_eq!(asset, vec![1, 2, 3, 4]); + let invalid = DocumentPageSelection::parse("3").unwrap(); + assert_eq!( + convert(&bytes, "test.pptx", Some(&invalid)) + .unwrap_err() + .code(), + "invalidPages" + ); + } + + #[test] + fn workbook_uses_visible_sheet_order_and_preserves_shared_strings_in_utf16_manifest() { + let manifest = r#""#; + let mut utf16 = vec![0xff, 0xfe]; + utf16.extend(manifest.encode_utf16().flat_map(u16::to_le_bytes)); + let rels = br#""#; + let sheet = |index| { + format!(r#"{index}"#).into_bytes() + }; + let bytes = package(vec![ + ("_rels/.rels", root_rels("xl/workbook.xml")), + ("xl/workbook.xml", utf16), + ("xl/_rels/workbook.xml.rels", rels.to_vec()), + ("xl/worksheets/one.xml", sheet(0)), + ("xl/worksheets/two.xml", sheet(1)), + ("xl/worksheets/hidden.xml", sheet(2)), + ("xl/sharedStrings.xml", br#"Alpha valueBeta valueHidden value"#.to_vec()), + ]); + let all = convert(&bytes, "test.xlsx", None).unwrap(); + assert_eq!(all.pagination.page_kind, "sheet"); + assert_eq!(all.pagination.page_count, 2); + assert!(!all.markdown.contains("Hidden value")); + let selection = DocumentPageSelection::parse("2").unwrap(); + let selected = convert(&bytes, "test.xlsx", Some(&selection)).unwrap(); + assert!( + selected.markdown.contains("Alpha value"), + "{}", + selected.markdown + ); + assert!(!selected.markdown.contains("Beta value")); + assert!(!selected.markdown.contains("Hidden value")); + } + + #[test] + fn word_chunks_expose_synthetic_units_and_can_read_beyond_a_long_paragraph() { + let text = format!("{}End of long Word document", "A".repeat(4000)); + let xml = format!( + r#"{text}"# + ); + let bytes = package(vec![("word/document.xml", xml.into_bytes())]); + let all = convert(&bytes, "test.docx", None).unwrap(); + assert_eq!(all.pagination.page_kind, "text_chunk"); + assert_eq!(all.pagination.page_count, 3); + let selection = DocumentPageSelection::parse("3").unwrap(); + let selected = convert(&bytes, "test.docx", Some(&selection)).unwrap(); + assert!(selected.markdown.contains("End of long Word document")); + assert!(!selected.markdown.contains(&"A".repeat(1601))); + } + + #[test] + fn manifest_limits_and_package_paths_fail_explicitly() { + assert_eq!( + resolve_root_part("/folder/../deck%20name.xml").unwrap(), + "deck name.xml" + ); + for path in [ + "../outside.xml", + "https://host/file", + "a\\b", + "bad%00.xml", + "%xx", + ] { + assert!(resolve_root_part(path).is_err(), "{path}"); + } + let bytes = package(vec![( + "ppt/presentation.xml", + vec![b' '; MAX_MANIFEST_BYTES + 1], + )]); + assert_eq!( + OfficePageIndex::read(&bytes, Format::Pptx) + .err() + .unwrap() + .code(), + "resourceLimit" + ); + } +} diff --git a/src/crates/execution/tool-execution/src/fs/document/selection.rs b/src/crates/execution/tool-execution/src/fs/document/selection.rs new file mode 100644 index 0000000000..026e560075 --- /dev/null +++ b/src/crates/execution/tool-execution/src/fs/document/selection.rs @@ -0,0 +1,111 @@ +/// Normalized, one-based document page ranges. Resolution checks the document size before +/// expanding a range so an invalid request cannot allocate an arbitrarily large page list. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DocumentPageSelection { + ranges: Vec<(u32, u32)>, +} + +impl DocumentPageSelection { + pub fn parse(input: &str) -> Result { + let invalid = || { + "pages must contain positive page numbers or ascending ranges, for example '1-3,7'" + .to_string() + }; + let mut ranges = Vec::new(); + for part in input.split(',') { + let mut ends = part.trim().split('-'); + let start = ends + .next() + .ok_or_else(invalid)? + .trim() + .parse::() + .map_err(|_| invalid())?; + let end = match ends.next() { + Some(end) => end.trim().parse::().map_err(|_| invalid())?, + None => start, + }; + if start == 0 || end < start || ends.next().is_some() { + return Err(invalid()); + } + ranges.push((start, end)); + } + ranges.sort_unstable(); + let mut normalized: Vec<(u32, u32)> = Vec::new(); + for (start, end) in ranges { + if let Some((_, previous_end)) = normalized.last_mut() { + if start <= previous_end.saturating_add(1) { + *previous_end = (*previous_end).max(end); + continue; + } + } + normalized.push((start, end)); + } + Ok(Self { ranges: normalized }) + } + + pub fn resolve(&self, page_count: usize) -> Result, String> { + if let Some((_, last)) = self.ranges.last() { + if u64::from(*last) > page_count as u64 { + return Err(format!( + "pages requests page {last}, but this document has {page_count} selectable pages" + )); + } + } + Ok(self + .ranges + .iter() + .flat_map(|&(start, end)| start..=end) + .collect()) + } +} + +impl std::fmt::Display for DocumentPageSelection { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + for (index, &(start, end)) in self.ranges.iter().enumerate() { + if index > 0 { + formatter.write_str(",")?; + } + if start == end { + write!(formatter, "{start}")?; + } else { + write!(formatter, "{start}-{end}")?; + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ranges_are_sorted_deduplicated_and_merged() { + let selection = DocumentPageSelection::parse("7, 2-4,1, 3-5").unwrap(); + assert_eq!(selection.to_string(), "1-5,7"); + assert_eq!(selection.resolve(9).unwrap(), vec![1, 2, 3, 4, 5, 7]); + } + + #[test] + fn malformed_and_out_of_bounds_requests_are_rejected_before_expansion() { + for input in [ + "", + "0", + "-1", + "3-1", + "1-", + "1,", + "1-2-3", + "1.5", + "4294967296", + ] { + assert!(DocumentPageSelection::parse(input).is_err(), "{input}"); + } + let huge = DocumentPageSelection::parse("1-4294967295").unwrap(); + assert!(huge.resolve(10).unwrap_err().contains("has 10")); + assert!(DocumentPageSelection::parse("1") + .unwrap() + .resolve(0) + .is_err()); + } +} diff --git a/src/crates/execution/tool-execution/src/fs/document/text_pages.rs b/src/crates/execution/tool-execution/src/fs/document/text_pages.rs new file mode 100644 index 0000000000..0c353d0bb4 --- /dev/null +++ b/src/crates/execution/tool-execution/src/fs/document/text_pages.rs @@ -0,0 +1,71 @@ +use super::{DocumentConversionError, DocumentPageSelection, DocumentPagination}; + +// Small enough that even a source paragraph without newlines fits the normal Read line cap. +// These are stable Unicode character chunks of extracted text, never Word/print page numbers. +pub const TEXT_CHUNK_CHARS: usize = 1600; + +pub(super) fn select_text( + markdown: String, + selection: Option<&DocumentPageSelection>, +) -> Result<(String, DocumentPagination), DocumentConversionError> { + let boundaries: Vec = markdown + .char_indices() + .step_by(TEXT_CHUNK_CHARS) + .map(|(byte, _)| byte) + .chain(std::iter::once(markdown.len())) + .collect(); + let count = boundaries.len() - 1; + let pagination = DocumentPagination::new("text_chunk", count, selection)?; + let Some(selection) = selection else { + return Ok((markdown, pagination)); + }; + let pages = selection + .resolve(count) + .map_err(|error| DocumentConversionError::new("invalidPages", error))?; + let mut selected = String::new(); + for page in pages { + if !selected.is_empty() { + selected.push_str("\n\n"); + } + selected.push_str(&format!("## Text chunk {page}\n\n")); + selected.push_str(&markdown[boundaries[page as usize - 1]..boundaries[page as usize]]); + if selected.len() > super::MAX_DOCUMENT_MARKDOWN_BYTES { + return Err(DocumentConversionError::new( + "resourceLimit", + "selected text chunks exceed the Read output budget; request fewer pages", + )); + } + } + Ok((selected, pagination)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn chunks_cover_unicode_without_gaps_and_keep_legacy_full_text() { + let source = "汉字🌏a".repeat(1201); + let (legacy, metadata) = select_text(source.clone(), None).unwrap(); + assert_eq!(legacy, source); + assert_eq!(metadata.page_count, 4); + let selection = DocumentPageSelection::parse("2-3").unwrap(); + let (selected, metadata) = select_text(source.clone(), Some(&selection)).unwrap(); + assert_eq!(metadata.page_kind, "text_chunk"); + assert_eq!(metadata.next_page, Some(4)); + assert!(selected.contains("## Text chunk 2")); + assert!(!selected.contains("## Text chunk 1")); + let mut restored = String::new(); + for number in 1..=4 { + let selection = DocumentPageSelection::parse(&number.to_string()).unwrap(); + let (chunk, _) = select_text(source.clone(), Some(&selection)).unwrap(); + restored.push_str( + chunk + .strip_prefix(&format!("## Text chunk {number}\n\n")) + .unwrap(), + ); + } + assert_eq!(restored, source); + assert_eq!(select_text(String::new(), None).unwrap().1.page_count, 0); + } +}