feat: doc-converter + CCR — PDF/DOCX/XLSX/PPTX reading and large doc token efficiency#66
feat: doc-converter + CCR — PDF/DOCX/XLSX/PPTX reading and large doc token efficiency#66stealthwhizz wants to merge 5 commits into
Conversation
- SmartCrusher: compress JSON tool outputs (removes nulls, deduplicates arrays, truncates strings) - CodeCompressor: strip comments and blank lines from source files (TS, JS, Py, Go, Rust, Java, C++) - CacheAligner: order system prompt static-first to maximize provider KV cache hits - doc-converter: convert PDF, DOCX, XLSX, PPTX to clean markdown via pdf-parse, mammoth, xlsx, jszip - doc-store + CCR: chunk large docs into sections, return outline first, agent fetches on demand - Disk cache for converted docs at memory/.doc-cache/ with source-newer-than-cache invalidation - Fix: check isConvertible() by extension before isBinary() — PDFs without null bytes were bypassing conversion - Add benchmark scripts: test-compression.mjs, test-live-benchmark.mjs, test-token-proof.mjs
…on errors - read_doc_section now returns has_refs: list of section IDs referenced in fetched content so agent can decide follow-up fetches in one hop - read(path, mode="outline") dry-run returns outline + token estimates without loading content — lets agent plan section fetches upfront - DocStore: LRU soft cap (10MB) evicts least-recently-accessed docs to prevent OOM in long sessions reading many large files - doc-converter: hard cap at 50MB — returns explicit error instead of stalling silently on very large files - doc-converter: surfaces explicit error message on conversion failure instead of returning null (agent can now tell the user what went wrong) - read.ts: cache invalidation comment for mtime+inode edge case
…pic, OpenAI, Lyzr)
shreyas-lyzr
left a comment
There was a problem hiding this comment.
Solid implementation — the doc-converter + CCR pipeline is well-structured and matches the agreed spec. Four issues below that need fixing before merge: one is a runtime crash, one is a key-collision bug, one is a section-ID false-positive, and one is a path mismatch that will silently break read_doc_section on relative paths. Everything else is in good shape.
There was a problem hiding this comment.
Pull request overview
This PR adds a document-reading and context-cost optimization pipeline to gitagent: binary office/PDF docs can be converted to readable markdown on read(), and large documents can be served via CCR (outline-first + section fetch) to avoid repeatedly flooding prompt context across turns.
Changes:
- Added a doc converter for PDF/DOCX/XLSX/PPTX (plus HTML) that outputs cleaned markdown with a 50MB guard and caching.
- Added CCR chunking + an in-session
DocStore, and registered aread_doc_sectionretrieval tool. - Added multiple token benchmark/proof scripts and introduced JSON/code/system-prompt “compression” utilities.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/doc-converter.ts |
New document conversion + cleanup pipeline to markdown. |
src/tools/doc-store.ts |
New CCR chunk store with outline generation, section fetch, and LRU eviction. |
src/tools/read.ts |
Wires conversion + CCR into read(), adds caching and optional compression passes. |
src/tools/shared.ts |
Adds mode="outline" option to the read tool schema. |
src/tools/index.ts |
Registers read_doc_section tool and wires DocStore/CostTracker into read. |
src/sdk.ts |
Instantiates DocStore and passes it into builtin tool creation for sessions. |
src/loader.ts |
Builds system prompt via CacheAligner to improve KV-cache efficiency. |
src/tools/cli.ts |
Compresses JSON CLI output via SmartCrusher before returning to the model. |
src/cost-tracker.ts |
Tracks doc-conversion “token savings” at the session level. |
src/compression/smart-crusher.ts |
New JSON compression utility (SmartCrusher). |
src/compression/code-compressor.ts |
New source-code noise stripping utility (CodeCompressor). |
src/compression/cache-aligner.ts |
New system-prompt prefix alignment utility (CacheAligner). |
src/compression/index.ts |
Barrel exports for the compression utilities. |
test-live-benchmark.mjs |
New live benchmark script (provider-aware) comparing CCR vs no-CCR. |
test-token-proof.mjs |
New “meeting demo” token proof script for CCR savings. |
test-compression.mjs |
New benchmark covering SmartCrusher/CodeCompressor/doc-converter/CacheAligner. |
package.json |
Adds new dependencies for conversion + compression features. |
package-lock.json |
Lockfile updates for the newly added dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n, legacy formats, logging - read.ts: cacheKey uses hash to prevent collisions between paths that differ only in separators - doc-store.ts: has_refs regex requires [sN] bracket syntax to avoid false positives from words like session3 - index.ts: use resolve() for altPath so relative paths match stored keys - read.ts: mode=outline now converts and chunks the doc if not already in store instead of returning an error - doc-converter.ts: remove .doc/.ppt — mammoth/JSZip cannot reliably parse legacy binary formats - loader.ts: gate system prompt metrics behind GITAGENT_DEBUG env var - package.json: remove unused headroom-ai dependency
shreyas-lyzr
left a comment
There was a problem hiding this comment.
All four issues from the previous review are resolved:
-
PDFParse import — pdf-parse v2.4.5 exports PDFParse as a named class, so the import and constructor call are correct for the pinned version. The previous concern was based on v1 behavior; no longer applies.
-
Cache key collision — fixed with a djb2 hash appended to the path. /tmp/a/b.pdf and /tmp/a_b.pdf now produce distinct keys.
-
has_refs false positives — fixed by requiring bracket syntax [sN] and filtering against actual chunk IDs from the store.
-
Path mismatch in read_doc_section — fixed by using resolve(config.dir, path) for altPath and trying both the raw and resolved paths.
Implementation matches the agreed spec. Good to merge.
Summary
read(). Previously returned[Binary file].read_doc_section, preventing large docs from flooding context on every turn.test-live-benchmark.mjscompares two successful runs: full markdown in prompt (no CCR) vs outline + fetch (with CCR). Supports Anthropic, OpenAI, and Lyzr.What changed
src/tools/doc-converter.tssrc/tools/doc-store.tsread_doc_sectionwithhas_refs, LRU soft cap (10MB)src/tools/read.tsmode="outline"dry-runsrc/tools/shared.tsmode="outline"added to read schemasrc/tools/index.tsread_doc_sectiontool registered withhas_refsresponsetest-live-benchmark.mjsAPI surface (locked)
has_refslets the agent decide follow-up fetches in one hop without re-parsing content.Known limitations (v2)
Closes #65