P2 [CRF-2] The test is named "Counts every part type toward the request estimate" but the parts slice omits ReasoningPart and ToolResultOutputContentMedia. Both branches in production code (estimatePartBytes line 203, toolResultOutputBytes line 235) have zero byte-counting coverage. Delete either branch and every test still passes.

"Reasoning text can dominate request size in extended conversations (Claude's chain-of-thought output runs to thousands of tokens). An undercounting regression here silently defeats the payload cap for reasoning-heavy prompts, which is exactly the class of request most likely to hit the 32 MiB limit." (Bisky)

The typed-nil test at line 176 exercises (*fantasy.ReasoningPart)(nil), which proves nil-safety but not size contribution.

Fix: add to the parts slice:

fantasy.ReasoningPart{Text: strings.Repeat("r", 30)},
fantasy.ToolResultPart{Output: fantasy.ToolResultOutputContentMedia{Data: strings.Repeat("m", 30)}},

(Bisky P2, Chopper P3, Meruem P3)

🤖

P3 [CRF-1] When multiple small PDFs together exceed the cap (e.g., three 40-page PDFs with a 100-page cap), this branch returns "Split the PDF and retry." The user has no single large PDF to split. The single-file branch (line 160) correctly names the oversized file; this aggregate branch gives inapplicable guidance.

Something like "Remove some PDF attachments or reduce total pages and retry" would match the actual situation.

(Netero, Meruem)

🤖

P3 [CRF-3] "Remove or shrink attachments" assumes the request is large because of user-uploaded files. In an extended agentic session, tool results (file reads, grep output) accumulate across the conversation; 320 tool results averaging 100 KB each reach 32 MB with zero user-uploaded files. The user sees "Remove or shrink attachments," sees none, and has no idea what to do.

"The PDF-specific errors are good. They name the file, state the violation, and prescribe an applicable action. This message should match that standard." (Leorio)

Consider: "Remove attachments or start a shorter conversation and retry," or detect whether the prompt actually contains file parts and branch the advice, the way pageCapMessage already branches on single-file vs. aggregate.

(Chopper P3, Leorio P3)

🤖

P3 [CRF-4] The regex counts every physical /Type /Page marker in the file, including superseded objects from incremental saves. A 5-page PDF that went through 20 incremental-save cycles accumulates ~105 physical markers, tripping the 100-page cap despite having 5 logical pages. Enterprise documents cycling through approval workflows are realistic targets.

"The heuristic is acknowledged as approximate, and a full parser has its own risks. The question is whether the approx label is doing enough work when the overcount can be 20x." (Hisoka)

The fail-open design (zero matches returns ok=false, skipping the check) prevents one class of false positives, but the overcount class remains. One mitigation without a full parser: scope the regex scan to after the final startxref, which points to the live object graph.

(Hisoka)

🤖

Nit [CRF-5] This comment restates bytes.HasPrefix(data, pdfHeader). The same pattern appears on pageCapMessage (line 156), the limits struct (line 45), and reject (line 247). Per project standard: comments should be substantive, not restatements of the code they annotate.

(Gon)

🤖

Nit [CRF-6] pdfLargeContextMinToken reads as an inclusive lower bound ("min"), but the check at line 64 uses strict >, so a 200k-token model does NOT qualify for the larger page cap. pdfLargeContextThreshold would make the boundary behavior unambiguous without needing the comment.

(Gon)

🤖

Nit [CRF-7] The full .Error() chain reads: prepare prompt: preflight prompt: prompt preflight failed: reason=invalid_pdf .... Three layers, each a paraphrase of "the prompt preparation step." The structured Detail and Message fields are clean (chaterror.Classify recovers them), but if anyone logs err.Error(), they get three referrals before the actual diagnostic.

Consider "validate prompt limits: %w" (names the specific substep) or dropping the text entirely and relying on the stack frame.

(Leorio)

🤖

P3 [CRF-8] This test proves Bedrock triggers validation and uses the Bedrock provider name. No test verifies Bedrock's distinct 20 MiB payload cap vs. Anthropic's 32 MiB, or that user-facing messages say "AWS Bedrock." The validateWithCaps helper always hardcodes fantasyanthropic.Name (line 210), so walker-level tests never exercise Bedrock-specific behavior. If limitsFor gave Bedrock Anthropic's 32 MiB cap, every test would still pass.

Sketch: call ValidatePromptLimits(fantasybedrock.Name, 200_000, oversizedPrompt) where the prompt is between 20 MiB and 32 MiB, and assert rejection with "AWS Bedrock" in the classified message.

(Bisky)

🤖