fix(documents): PDF-aware documentTokens, and couple DOCUMENT_READ_ENABLED to the paths that promise the tool

[philmerrell]

Two independent defects found while validating the document-offload epic (PRs #1137–#1143) end to end — static review, 41 purpose-written probes, and a live clickthrough on dev. They share files, so they ship together rather than as a stack (an inner stack PR would get no CI).

---

1. documentTokens counted bytes, not PDF pages

documentTokens (offload spec §6.1) and the DOCUMENT_OFFLOAD_MIN_TOKENS floor (§4C) both read a document's weight from the compaction estimator's bytes/4. That is wrong for PDFs by an order of magnitude: Bedrock dual-encodes each page as an image and a text layer — the very property §3 relies on when it insists on native blocks over flattened text — and the image channel has nothing to do with file size. A 27 KB 60-page PDF and a 27 MB 60-page scan cost about the same; bytes/4 sees only the bytes.

Measured on dev (session 61de2256, 2026-09-16):

	value	vs measured
measured document weight	~94,485 tok	109.1K first write − 14.6K static prefix
old (bytes/4)	6,894 tok	13.7× low
new	90,000 tok	1.05× low
document share of prefix	6.3% → 82.5%	truth ~87%

This is not cosmetic. §6.1 is explicitly the instrument the evaluation spec's §4.2 ship/abandon rule reads off. At a 14× error those rows report documents as ~6% of the prefix when they were ~87% — which argues abandon ("the prefix is dominated by non-document content") for a change that in the same session dropped the prefix from 109.1K to ~15K.

Change. New apis/shared/files/document_tokens.estimate_document_tokens: PDFs are max(pages × PDF_PAGE_TOKEN_ESTIMATE, bytes/4) (default 1,500/page, env-tunable so the constant can be re-fit from measured rows without a deploy); other formats keep bytes/4, since Bedrock extracts those to text and there is no image channel to miss. max() rather than a replacement, so a byte-heavy scan can only score higher than before; an unparseable PDF falls back to today's number.

1,500 is compaction_policy.IMAGE_TOKEN_ESTIMATE's own rationale (image tokens ≈ (w·h)/750; a letter page at ~1000×1100 lands there). Against the measurement it is 1,500 vs 1,573 actual per page — ~5% low, deliberately, because this number gates an eviction and over-stating it would evict documents whose re-write has not earned it.

No memoization: page counting measured ~0.1 ms even at 200 pages, while hashing 2 MB for a cache key costs 1.5 ms — the cache would be 15× more expensive than the thing it avoids.

Wired into every consumer of that quantity, because Candidate.tokens feeds the floor — the same weight used for a decision. Fixing the row but not the floor would leave a row saying "90K tokens" beside an offloader saying "3,750, below the floor, skip": document_context (documentTokens, documentSliceTokens), document_offload (Candidate.tokens, now a field rather than a derived property; the floor; the aged-slice ledger), document_rehydration (document_stripped).

⚠️ Behaviour change: PDFs previously stuck below the 5,000-token floor forever are now offload candidates. That is the bug working correctly — a 10-page scan is ~15K tokens, not 3,750 — but it is a real change in what gets evicted.

compaction_policy._block_tokens is deliberately NOT changed. Same blind spot, but it rescales to measured history and drives cut thresholds, so it needs its own decision.

---

2. DOCUMENT_READ_ENABLED did not reach the paths that promise the tool

The flag was read in exactly one place — _document_tools_gate, which decides whether document_read is injected. Neither the restore path nor the live offload consulted it, so pulling the one kill switch an operator would actually reach for left two worse-than-before behaviours:

1. restore kept emitting <document-digest upload_id=…>, advertising a retrieval handle for a tool that would not be injected — inviting a call the model cannot make;
2. the live offload kept evicting document bytes with the only recovery path switched off — strictly worse than the pre-offload world, where live bytes never left.

Spec §5 states the rule this violated: "a digest that points at a tool nobody has is no better than today's placeholder."

Fix, split by whether dropping the bytes is optional:

• Restore has to drop them either way (Bedrock rejects duplicate document names), and a digest is still strictly more than the placeholder — so it still renders, but render_digest gains include_handle and restore passes document_read_enabled(). Outline and abstract survive; the false affordance does not.
• The live offload is optional, so it stops entirely: offload_enabled_for now also requires document_read_enabled(). Both callers (head-of-turn offload, restore-path slice ageing) already route through that single gate, so one check covers both and there is no second flag read to drift.

With the flag on, every rendered byte is identical to before.

---

Verification

• 31 new tests: tests/shared/test_document_tokens.py (24) and tests/agents/main_agent/session/test_document_read_flag_coupling.py (7), each stating its defect directly as a test
• Full backend suite: 9050 passed, 3 skipped, 0 failed
• Import boundaries green (agents/ → apis.shared is the allowed direction)
• Live dev clickthrough confirmed the whole lifecycle before these fixes: turn 1 full document → turn 2 digest-only → turn 3 document_read(page_range="44") → slice aged on schedule

Still open from the same sweep (not in this PR)

• The epic's quality gate (docs/specs/document-offload-evaluation.md) is still Status: Draft with no harness, while DOCUMENT_OFFLOAD_ROLLOUT_PERCENT defaults to 100 — which is that spec's own "Ship" state, so its B/C arms cannot exist
• ReDoS in document_read pattern mode: a model-supplied (a+)+$ measured 0.87 s at 24 chars, 14 s at 28, exponential; real PDF lines run 60–100
• INLINE_ATTACHMENTS_MAX_TOTAL_BYTES (7,500,000) base64-encodes to exactly 10,000,000 — the quota itself — leaving zero headroom for the JSON envelope
• render_digest XML-escapes the abstract after slicing to the remaining room, so the 1,500-token cap is soft
• prefixTokens.tools looks off-by-one on cache-bypassed turns (reported 106,756 against a 12.5K static prefix)

🤖 Generated with Claude Code