feat(llm): PDF document input to document-capable models (#255 Phase 3) - #534
Conversation
Forge now forwards an inbound PDF file part to the model as a native document
block, alongside the Phase 2 image path. Builds on the Phase 2 gate + DoS
controls.
- Capability: ModelSupportsPDF (Anthropic Claude 3.5+; the bare claude-3.0
models are correctly excluded) + IsDocumentMIME (application/pdf).
- Anthropic provider: anthropicBlocksFromParts emits a {type:document,
source:{base64,application/pdf}} block for document parts; the shared source
struct is renamed anthropicMediaSource (image + document).
- Projection (a2aMessageToLLM): PDF file parts -> ContentPartDocument, next to
the image projection; text stays the text-of-record for the scanners.
- Gate (checkInboundMedia): PDFs pass on a document-capable model; a PDF on a
non-document model rejects (model_not_document_capable); non-PDF documents
and other unsupported types reject (unsupported_media_type). Per-document
byte cap (32 MiB) + count (5) + a %PDF- format sniff, with reason codes
too_many_document_parts / document_limit_exceeded.
No new dependency (native document blocks, no extraction). OpenAI Responses
input_file, Gemini documents, and text-extraction fallback for non-native
models are deferred follow-ups.
Tests: ModelSupportsPDF / IsDocumentMIME; CheckDocumentLimits (valid/empty/
oversized/mislabeled); Anthropic document-block serialization; projection
(PDF->document, unsupported not projected); gate matrix (pdf accept on
claude, reject on gpt-4o, mislabeled, unsupported type). Docs: runtime-engine
multimodal + DoS table, audit reason codes, forge.md skill (synced).
initializ-mk
left a comment
There was a problem hiding this comment.
Self-review — verified against source ✅
Phase-3 PDF input on the established Phase-2/#532 pattern; posting as a COMMENT (own PR). No blocking issues.
Verified end-to-end:
- Capability:
pdfCapablePrefixescorrectly includes 3.5/3.7 + 4.x/5 families and excludes bare Claude 3.0 (the-3-infix keepsclaude-3-{opus,sonnet,haiku}from matching the family prefixes) — traced each case. - Gate: document branch with distinct reason codes (
model_not_document_capable/too_many_document_parts/document_limit_exceeded), loud reject. CheckDocumentLimits: byte cap +%PDF-sniff; no decode-bomb surface (PDF isn't decoded here).- Anthropic
{type:document, source:{base64, media_type:application/pdf, data}}; cleananthropicImageSource→anthropicMediaSourcerename (wire unchanged). - Projection routes PDF→document part; semaphore covers documents (
FileParts()is type-agnostic); byte-identical text-only preserved. - No new scan-bypass — native PDF bytes, not extracted text (tracked on #255 item 9).
Two LOW/informational notes inline.
| // a loud reject. OpenAI Responses input_file and Gemini document support are | ||
| // deferred follow-ups. | ||
| var pdfCapablePrefixes = []string{ | ||
| "claude-3-5", "claude-3-7", "claude-opus", "claude-sonnet", "claude-haiku", |
There was a problem hiding this comment.
LOW (accuracy, same class as #531's -mini note): verify the haiku family actually supports PDF. claude-3-5 and claude-haiku match claude-3-5-haiku / claude-haiku-*; Anthropic's PDF (document) support was Sonnet-first, so if a matched haiku model lacks native PDF, a document gets sent and yields a provider error rather than forge's clean model_not_document_capable reject. Loud, not silent (so LOW) — worth confirming against Anthropic's current PDF matrix and narrowing the prefixes (e.g. sonnet/opus only) if haiku isn't covered.
| // MaxDocumentPartBytes caps a single document (PDF) part's raw bytes, | ||
| // matching Anthropic's ~32 MiB per-document limit. The request-body cap | ||
| // bounds the total; this bounds any one document. | ||
| MaxDocumentPartBytes = 32 << 20 |
There was a problem hiding this comment.
Informational (not a bug): this 32 MiB per-document cap isn't actually reachable. A 32 MiB PDF base64-inflates to ~43 MiB in the JSON body, which exceeds the 32 MiB request-body cap → MaxBytesReader rejects it before the gate. So the effective max PDF is ~24 MiB raw and the body cap is the binding constraint. Not a correctness issue (the body cap correctly bounds total), just noting the per-document number is aspirational — it ties to the envelope/media-split item on the #255 checklist (base64 inflation). No action needed now.
…ap (#534 review) - ModelSupportsPDF now scopes to the Sonnet & Opus families only (claude-3-5-sonnet, claude-3-7-sonnet, claude-opus, claude-sonnet). Haiku is excluded — Anthropic's PDF support was Sonnet-first and Haiku's native-PDF coverage is unconfirmed, so a Haiku PDF now gets forge's clean model_not_document_capable reject instead of risking an opaque provider error. Fail closed; widen once confirmed. The 3.5/3.7 prefixes name "sonnet" explicitly so they no longer catch claude-3-5-haiku. - Clarifying comment on MaxDocumentPartBytes: the 32 MiB request-body cap is the binding constraint (base64 inflation ~33% → ~24 MiB raw max), so the per-doc number is provider-aligned but not independently reachable today. - Tests + docs + skill updated to the narrowed set.
|
Both notes addressed in LOW (Haiku PDF support) — narrowed to Sonnet + Opus. You're right that Anthropic's PDF support was Sonnet-first and Haiku's native-PDF coverage is unconfirmed. Rather than risk a Haiku PDF slipping through to an opaque provider error, Informational (per-doc cap) — clarified in a comment, no behavior change. Correct that 32 MiB raw base64-inflates past the 32 MiB body cap (~24 MiB effective max), so the body cap is the binding constraint. I left Full sweep re-run green: |
…view) Confirmed against Anthropic's PDF matrix: Claude Haiku 4.5 fully supports native PDF document input, and Fable 5 supports it too. Add "claude-haiku" (matches the 4.x/5 family naming claude-haiku-4-5…, not the older claude-3-5-haiku, so Haiku 3.5 stays excluded pending confirmation) and "claude-fable" to pdfCapablePrefixes. Tests + docs + skill updated.
|
Correction applied in |
initializ-mk
left a comment
There was a problem hiding this comment.
Re-review — commit 82c2ae52 (Haiku 4.5 + Fable 5): PDF widening correct, one new inconsistency
The PDF-capability widening is right — I traced it: claude-haiku matches claude-haiku-4-5 (4.x/5 family naming) but NOT claude-3-5-haiku (version-first 3.5, still excluded) nor claude-3-haiku (3.0); claude-fable matches claude-fable-5; test updated to match.
But it introduced a vision/PDF table mismatch — requesting one change (inline). claude-fable was added to pdfCapablePrefixes but not visionCapablePrefixes, so Fable 5 now accepts a PDF while rejecting an image — the opposite of every other multimodal Claude.
| // Unknown-model default is fail-closed (loud reject, not a provider error). | ||
| // OpenAI Responses input_file and Gemini document support are deferred follow-ups. | ||
| var pdfCapablePrefixes = []string{ | ||
| "claude-3-5-sonnet", "claude-3-7-sonnet", "claude-opus", "claude-sonnet", "claude-haiku", "claude-fable", |
There was a problem hiding this comment.
Requesting change (LOW, introduced by this commit): mirror claude-fable into visionCapablePrefixes. This adds claude-fable to the PDF list but the vision list (claude-3/claude-opus/claude-sonnet/claude-haiku) has no claude-fable, so ModelSupportsVision("claude-fable-5") is false → an image to Fable 5 is rejected model_not_vision_capable even though a PDF is accepted. For Claude, document support is built on vision infra, so a PDF-capable model is vision-capable — declaring Fable 5 document-capable here should come with vision. Add claude-fable to visionCapablePrefixes and a claude-fable-5 case to the vision test's accept list. (LOW — clean reject, not silent/insecure — but a functional gap for a model this PR declares multimodal.)
… review) 82c2ae5 added claude-fable to pdfCapablePrefixes but not visionCapablePrefixes, so Fable 5 accepted a PDF while rejecting an image — the inverse of every other multimodal Claude. Claude document support is built on vision infra, so a PDF-capable model is necessarily vision-capable. Add claude-fable to visionCapablePrefixes and, to stop this class of mismatch recurring, add TestPDFCapableImpliesVisionCapable asserting every pdfCapablePrefixes family is also vision-capable.
|
Good catch — fixed in
|
initializ-mk
left a comment
There was a problem hiding this comment.
Closing re-review — commit f8c6312e ✅ LGTM
Verified against source. All findings across the review cycle are resolved.
- Vision/PDF mismatch — resolved + guarded.
claude-fableadded tovisionCapablePrefixes(Fable 5 now vision- and PDF-capable), vision test updated, and a newTestPDFCapableImpliesVisionCapableencodes the invariant (everypdfCapablePrefixesfamily must be vision-capable) — structurally prevents this class of bug from recurring. Better than the point fix I asked for. No regression: PDF list unchanged (3.5-haiku and Claude 3.0 still excluded). - PDF capability — correct after the narrow-then-widen. Sonnet 3.5/3.7 + Opus/Sonnet 4.x/5 + Haiku 4.5 + Fable 5 included;
claude-3-5-haiku(version-first naming) and bare Claude 3.0 excluded; fail-closed on unknown. Traced each case. - Per-document cap — documented (32 MiB body cap is the binding constraint at ~24 MiB raw; value stays provider-aligned for a future envelope/media-split).
Phase-3 flow verified end-to-end (unchanged and sound): document gate with distinct reason codes, CheckDocumentLimits (byte cap + %PDF- sniff, no decode-bomb surface), Anthropic document block, projection routing, semaphore coverage via FileParts(), byte-identical text-only, no new scan-bypass (native bytes, tracked on #255 item 9).
Verdict: LGTM. Needs a second reviewer to merge (own PR).
Part of #255. Phase 3 — document input. Builds on Phase 2 (image input) + the DoS controls (#532). Forge now forwards an inbound PDF file part to the model as a native document block, on the same
Partsplumbing as images.What this does
ModelSupportsPDF(Anthropic Claude 3.5+: the3-5/3-7releases + theopus/sonnet/haiku4.x/5 families; bare Claude 3.0 is correctly excluded since it predates PDF support) +IsDocumentMIME(application/pdf).anthropicBlocksFromPartsemits{type:document, source:{base64, media_type:application/pdf, data}}for document parts. The shared source struct is renamedanthropicMediaSource(now image + document).a2aMessageToLLM) — PDF file parts →ContentPartDocument, alongside the image projection; the flattened text stays inContentas the text-of-record for the scanners.checkInboundMedia) — PDFs pass on a document-capable model; a PDF on a non-document model rejects (model_not_document_capable); non-PDF documents and other types reject (unsupported_media_type). Per-document byte cap (32 MiB) + count (5) + a%PDF-format sniff, with reason codestoo_many_document_parts/document_limit_exceeded. Documents count toward the media concurrency semaphore.No new dependency — native document blocks, no PDF parsing/extraction.
Scope boundary (deferred, matching the plan)
input_fileand Gemini documents — the chat/Responses providers don't yet carry a document part; trailing-commit follow-ups.PromptText/scanText; deferred. Because we only send native PDF bytes (not extracted text), there's no scan-bypass in this PR (PDF bytes aren't text-scannable — same accepted limitation as images, documented).Tests
ModelSupportsPDF(Claude 3.5+ yes; Claude 3.0 / OpenAI / Gemini no) +IsDocumentMIME.CheckDocumentLimits: valid PDF, empty, oversized, mislabeled non-PDF (zip magic).claude-sonnet-5, reject ongpt-4o(does not support PDF), mislabeled PDF, unsupported document type.go test(core + cli runtime + server),golangci-lint0 issues,make sync-knowledge, nogo.work.sumchurn.Docs
runtime-engine.md(multimodal section now image+document + DoS table with per-document rows),audit-logging.md(new reason codes),forge.mdskill (synced).#255 remaining
Phase 4 persist-inbound-for-tools (+ cross-turn media replay) · Phase 5 model-generated image output · document follow-ups (OpenAI Responses / Gemini / extraction).