fix(anthropic): preserve client cache_control through the OpenAI-to-Anthropic bridge - #804
Conversation
…nthropic bridge The bridge flattened every message through the concatenated-text path and rebuilt content as fresh text blocks, silently stripping the prompt-cache markers a caller attached to its system prompt, message blocks, or tool definitions - the upstream cached nothing while the caller paid full input price every turn believing its caching strategy was active. - system: new AnthropicSystem enum (string | block array). All-plain- text input keeps the historical "\n\n"-joined string form byte-for- byte; block-array form is emitted only when a caller's system message itself carried typed blocks. - messages: caller-sent text blocks forward verbatim, 1:1, preserving block-level fields and positions. Non-text blocks are still skipped (pre-existing cross-provider limitation). Empty survivors degrade to a single empty text block. - tools: cache_control at the tool top level or inside function is forwarded onto the translated Anthropic tool. Shared chokepoint: split_system/build_request also serve /v1/responses and the Bedrock Anthropic-invoke path, so all sibling paths pick the fix up together. The native /v1/messages passthrough already preserved these fields and is untouched. Phase 0 of AISIX-Cloud#1110 (Gap A).
📝 WalkthroughWalkthroughAnthropic translation now preserves typed text blocks and ChangesAnthropic translation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProxyClient
participant AnthropicWire
participant AnthropicUpstream
ProxyClient->>AnthropicWire: Send system, message, and tool blocks
AnthropicWire->>AnthropicWire: Filter text blocks and preserve cache_control
AnthropicWire->>AnthropicUpstream: Serialize Anthropic messages request
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…e blocks
Audit round on the cache_control preservation change: the initial
verbatim clone forwarded empty text blocks and stray caller metadata
(e.g. a replayed streaming index) that Anthropic's strict request
validator rejects per-block, 400ing array-form requests that worked
before. Forwarded text blocks now keep only {type, text, cache_control,
citations} and blocks with missing or whitespace-only text are filtered,
restoring the old flatten path's clean-request guarantee. e2e
strengthened: exact tool shape, markers on every turn incl. replayed
assistant blocks, stray-field strip, plain-string system byte pin.
Independent cold-audit triage (merge gate)A 6-angle cold audit (each finding adversarially re-verified against the head SHA) produced 12 confirmed findings (5 rejected as false positives / out-of-scope). Triage below; all MEDIUMs are either fixed in Fixed in
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/src/cases/anthropic-upstream-e2e.test.ts (1)
543-621: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTool test only covers one of the two "supported locations" for tool
cache_control.The PR objective states cache_control is preserved "on tool definitions at both supported locations," but this test only places
cache_controlat the top level of the tool object (sibling totype/function), not nested insidefunction.cache_control. Consider adding a second tool (or asserting a second field) to also verify the nested-location case, closing the coverage gap for the stated dual-location behavior.As per coding guidelines, "Tests must cover boundary cases (empty values, min/max), invalid inputs, combination scenarios, and extreme cases" for `**/*.{test,spec}.{js,ts,jsx,tsx}`.♻️ Example addition to cover the nested location
tools: [ { type: "function", function: { name: "get_weather", parameters: { type: "object", properties: {} }, + cache_control: { type: "ephemeral" }, }, cache_control: { type: "ephemeral" }, }, ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/src/cases/anthropic-upstream-e2e.test.ts` around lines 543 - 621, Extend the tool translation test around the existing get_weather fixture and sentBody.tools assertions to cover cache_control nested inside function as well as the current top-level location. Add a second tool or equivalent nested-location fixture, then assert the translated Anthropic tool preserves its cache_control marker while retaining the expected name and input_schema shape.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/e2e/src/cases/anthropic-upstream-e2e.test.ts`:
- Around line 543-621: Extend the tool translation test around the existing
get_weather fixture and sentBody.tools assertions to cover cache_control nested
inside function as well as the current top-level location. Add a second tool or
equivalent nested-location fixture, then assert the translated Anthropic tool
preserves its cache_control marker while retaining the expected name and
input_schema shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe6dabc6-7156-4e24-a5be-ea39088f8276
📒 Files selected for processing (2)
crates/aisix-provider-anthropic/src/wire.rstests/e2e/src/cases/anthropic-upstream-e2e.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/aisix-provider-anthropic/src/wire.rs
… a model Phase 1 (DP half) of the prompt-caching work. A direct Anthropic model with auto_prompt_caching enabled makes the gateway inject cache_control breakpoints into requests carrying none of their own — one on the last system block, one on the last content block of the final message — so callers get provider-side prompt-cache discounts with no client change. Callers that set their own markers win (full stand-down, which also keeps the request within Anthropic's 4-breakpoint cap). Injection lives only in the Anthropic bridge (chat + chat_stream), not in the shared build_request, so the Bedrock-invoke and Vertex rawPredict paths do not inject — gateway-authored markers there carry provider hazards (Bedrock per-model 5m-only tiers, Vertex per-project reject mode) deferred to Phase 2. Marker preservation (#804) already covers all three. Model gains auto_prompt_caching: {enabled, ttl(5m|1h)}. DP reads it here; the paired CP knob (cp-admin.yaml + dashboard) is tracked in the umbrella issue and MUST deploy after this, since Model is deny_unknown_fields.
What
Client-supplied
cache_controlprompt-cache markers now survive the OpenAI→Anthropic bridge translation. Previously the bridge flattened every message through the concatenated-text path (content_str()) and rebuilt content as fresh{"type":"text"}blocks, silently stripping the markers a caller attached to its system prompt, message blocks, or tool definitions — the upstream cached nothing while the caller believed its caching strategy was active, paying full input price every turn.Phase 0 of the prompt-caching work tracked in AISIX-Cloud#1110 (Gap A).
Changes
AnthropicSystemenum (wire.rs): the top-levelsystemfield is nowstring | block-array, per Anthropic's documented equivalence (Messages API — system). All-plain-text system messages keep the historical"\n\n"-joined string form byte-for-byte; the block-array form is emitted only when a caller's system message itself carried typed blocks.Message content blocks map 1:1: when a caller sent array-form content, its
{type:"text", …}blocks now map 1:1 onto the Anthropic wire — block-level fields and block positions ride along (a marker means "cache through here", so its position is the payload). Two per-block guarantees, preserving what the old flatten path guaranteed implicitly:text,cache_control,citations) forward unchanged.indexreplayed from assembled history) would 400 at Anthropic's strict request validator, and degenerate blocks (missing or whitespace-onlytext) are rejected per-block upstream; both vanished in the old flatten, so forwarding them would break requests that worked before.Non-text blocks (images/audio) are still skipped — the pre-existing, documented cross-provider content limitation, unchanged. Empty survivors degrade to a single empty text block (Anthropic rejects
content: []).Tool definitions preserve
cache_control(translate_openai_tools_to_anthropic): callers attach the marker at the tool's top level or insidefunction; either spelling is forwarded. Tool definitions sit first in the prompt-cache prefix hierarchy (tools → system → messages, per the prompt-caching doc), so a stripped tool marker disables the caller's whole cache.Chokepoint coverage — precise scope:
split_system+build_requestserve the OpenAI-shape bridge to three upstream families — Anthropic direct, Bedrock Anthropic-invoke (chat_anthropic), and Vertex Anthropic:rawPredict/:streamRawPredict— so OpenAI-shape/v1/chat/completionscallers routed to any of them keep their markers. Two entry points do not yet benefit, because their ingress flattens block structure before the chokepoint (pre-existing, unchanged by this PR): the/v1/responsesinput translation (responses_bridge.rsbuilds every message withcontent_blocks: None) and the native/v1/messagesinbound parse when bridged cross-provider to Bedrock/Vertex Claude (parse_inbound_requestrebuilds clean blocks). Both are tracked as Phase-0 follow-ups in AISIX-Cloud#1110. The native/v1/messagespassthrough (direct Anthropic upstream) already preserved these fields byte-for-byte and is untouched.Ecosystem comparison (per repo rule 7)
Marker-stripping in the OpenAI→Anthropic transform is a recurring bug class across mainstream AI gateways: among the gateways surveyed, one shipped and fixed exactly this drop on a model-ID edge case, one still drops the caller's TTL on its transformed route (with an open issue for markers vanishing on a second provider path), and one drops markers entirely on its OpenAI-format path while supporting them elsewhere. Gateways that avoid the class do so the way this PR does: forward the caller's typed blocks verbatim instead of rebuilding content from flattened text. Our design lands on verbatim block forwarding for
type:"text"blocks only, keeping the string-formsystembyte-stable when no blocks were sent — a divergence from gateways that always emit array form, chosen to keep existing traffic's wire bytes (and any provider-side cache prefixes built on them) unchanged.Testing
wire.rsunit tests (7 new): system-marker preservation (pure/mixed), user/assistant block preservation incl. positions, image-only degrade invariant, tool markers (both spellings + absence), byte-stability pin for the plain-stringsystemform.anthropic-upstream-e2e.test.ts, new scenario): black-box wire-shape assertion — OpenAI-shape request in, captured gateway→upstream/v1/messagesbody must carry the markers on system (block-array form), the exact marked message block (withttl), and the translated tool.cargo test+ full local e2e suite green;cargo fmt+ clippy clean.Deliberately out of scope
role:"tool"(tool_result) message content — no clean OpenAI-shape spelling exists for that position; the flattened-string path there is unchanged.cachePoint(streaming path) — AISIX-Cloud#1110 Phase 2./v1/responsesingress and native-/v1/messages-cross-provider ingress marker preservation — flattened upstream of this chokepoint; Phase-0 follow-ups in AISIX-Cloud#1110 (see coverage scope above).ttl:"1h"marker reaches Bedrock-invoke/Vertex models that may support only the 5-minute tier — the marker is the caller's own request content and per-model TTL gating belongs to the Phase-1 injector (AISIX-Cloud#1110), not to pass-through fidelity.Audit round
An independent multi-angle cold audit (12 confirmed findings) drove three changes after the initial push: (1) the per-block whitelist + degenerate-block filter above — the initial verbatim clone would have forwarded empty text blocks and stray caller metadata that Anthropic's strict validator 400s, breaking previously-working array-form traffic; (2) e2e strengthened — exact tool-shape equality (no OpenAI wrapper leakage,
input_schemapresence), markers asserted on every turn incl. replayed-assistant blocks, message-count pin, stray-field strip, and a plain-stringsystembyte-stability assertion on the live wire; (3) this coverage-scope section corrected — the initial body overclaimed/v1/responsesbenefit and omitted the Vertex paths.Summary by CodeRabbit
New Features
cache_controlmarkers are carried through for system content, message blocks, and translated tool/function definitions.Tests