fix(ai): stop inferring Anthropic prompt caching from Claude model ids - #3935
fix(ai): stop inferring Anthropic prompt caching from Claude model ids#3935probepark wants to merge 2 commits into
Conversation
Claude-family model ids behind non-canonical Anthropic-compatible gateways were opted into top-level automatic caching, but such a gateway may already attach its own block-level cache_control markers. The generated top-level marker then became a fifth breakpoint and Anthropic rejected the whole request with HTTP 400 "A maximum of 4 blocks with cache_control may be provided. Found 5." A model id cannot tell us what a proxy adds downstream, so the capability now has to be declared instead of guessed. Lore-id: 989c3170 Constraint: canonical api.anthropic.com automatic caching must not change Constraint: cacheRetention none and request-over-model precedence stay intact Rejected: clamp generated breakpoints before send | gateway-added markers are not visible in our params Rejected: strip caller-supplied cache_control | silently disables a working proxy cache strategy Confidence: high Scope-risk: narrow Reversibility: easy Tested: bun test packages/ai/test/anthropic-cache.test.ts packages/ai/test/anthropic-stream-envelope.test.ts -- 37 pass Not-tested: live traffic against the reporting gateway Supersedes: 8893c34
…er source The eval artifact pins the provider source blob oid and sha256 so the recorded evidence cannot drift from the code it describes. Changing the cache-mode resolution invalidates that binding by design, so the artifact is regenerated against the new source instead of leaving the integration test red. Lore-id: 989c3170 Tested: bun test packages/ai/test/anthropic-cache-eval.integration.test.ts -- 2 pass
|
Superseded by #3933, merged as 42f1091. The accepted policy keeps canonical Anthropic on automatic caching but defaults noncanonical |
yazzang-homelab
left a comment
There was a problem hiding this comment.
Independent architect review. The bug is real and well-diagnosed, but dev moved underneath this branch and now disagrees with it. Rebasing will not resolve that automatically — it needs a decision.
The conflict. This branch's base is 1f735b7b, which predates two merges that touch the exact same expression:
538cb43d1— fix(ai): advance explicit Anthropic cache through tool loops (#3930)42f1091c6— fix(ai): default compatible Claude gateways to explicit caching (#3933)
Same defaulting block, three different answers for "Claude-family model on a non-canonical endpoint, no explicit promptCacheMode":
| result | |
|---|---|
8893c346a (#3923, your base) |
automatic — the top-level marker that causes the 400 |
current dev (#3933) |
explicit |
| this PR | none |
So this is not a rebase conflict to resolve mechanically — merging it would revert #3933's default and possibly interact with #3930's tool-loop handling.
And #3933 may not actually close your bug. Your failure is a gateway that already emits its own cache_control blocks; ours becomes the fifth and Anthropic rejects the request with A maximum of 4 blocks with cache_control may be provided. Found 5. explicit mode still emits block-level breakpoints — it changes where the markers go, not how many we add. If the gateway contributes 4 on its own, explicit overflows exactly like automatic did. If that is right, #3933 narrowed the blast radius without removing it, and your none default is the actually-correct one.
Please confirm that against the gateway that produced the 400 and say so in the description. That reframes this from "revert of #3933" to "#3933 was incomplete", which is a much easier call for the maintainer.
On the design itself, two things I'd push on:
-
|| cacheRetention !== undefined→automaticis a surprising opt-in. Setting a retention is a statement about how long to cache, not which framing to use. A user who setscacheRetention: "long"on a gateway that rejects top-level markers gets the 400 back, and nothing in the name suggests they opted into automatic framing. If retention should imply caching,explicitis the safer implication; if the point is an escape hatch,promptCacheMode: "automatic"already is one and is explicit about what it does. -
noneas the default silently disables caching for working gateways. Many Anthropic-compatible proxies passcache_controlthrough untouched and cache fine today. Defaulting them tononeis a real cost regression with no error to notice — the opposite failure mode from the 400, and a quieter one. Both "Rejected" alternatives in your commit message are correctly rejected, but the third option — declare the capability per provider preset, defaulting unknown gateways tonone— matches your own conclusion that "a model id cannot tell us what a proxy adds downstream". Acompat.promptCacheModeon the known-good presets would keep them working while making unknown gateways safe.
What is good: the constraints are the right ones (canonical api.anthropic.com unchanged; cacheRetention: none and request-over-model precedence intact), isCanonicalApi still gates supportsLongCacheRetention so a gateway cannot claim 1h TTL off canonical metadata, and both rejected alternatives are rejected for correct reasons — clamping cannot see gateway-added markers, and stripping caller-supplied cache_control would break a working proxy strategy.
Also note this PR updates artifacts/issue-3670-anthropic-cache-eval.json provider-source hashes. Those pin a fetched upstream doc; after the rebase please re-derive them on the new base rather than carrying the values forward, or the evidence artifact will attest to a page you did not re-fetch.
gajae.pr-review-verdict.v1 merge-blocked sha256:bb570da948cf52235a2d76119175400ec95abeb9 reviewer:architect evidence:git merge-base is 1f735b7b (predates 42f1091c/#3933 and 538cb43d/#3930); compared anthropic.ts defaulting block on dev vs this head
What
Non-canonical Anthropic-compatible endpoints no longer receive a generated
cache_controlmarker based on the model id alone. The capability must now be declared:api.anthropic.comclaude-*idcompat.promptCacheMode: automaticcompat.promptCacheMode: explicitcacheRetention: short|longcacheRetention: none, request-over-model precedence, andsupportsLongCacheRetentionare unchanged.Why
Fixes #3934.
8893c346aaddedisClaudeFamilyModel()togetCacheControl(), opting every Claude-named model on every unknown gateway into top-level caching. A gateway that already attaches its own block-level markers then sees a fifth breakpoint, and Anthropic rejects the request:A model id cannot describe what a proxy adds downstream, and those extra breakpoints are not visible in our serialized params — so clamping before send is not possible either. The capability has to be declared rather than guessed.
The second commit regenerates
artifacts/issue-3670-anthropic-cache-eval.json, whose recorded evidence is pinned to the provider source blob oid/sha256 by design.Testing
Two pre-existing failures reproduce identically on unmodified
devand are unrelated to this change:no-strict-env.test.ts > keeps strict mode on when neither name is set— the test spawns a probe without clearing an inheritedPI_NO_STRICT=1from the shell environmentcheck:ts > check:sdk-closure—verify-gjc-sdk-canonicalization.ts --self-testexpects exit 1, gets 0Not tested: live traffic against the reporting gateway.
GJC verdict
No independent architect/critic review was run; this needs a human reviewer before merge.
devbun checkpasses — blocked by the pre-existingcheck:sdk-closureself-test failure documented above;bun --cwd=packages/ai run checkpasses