Repository navigation
fix: forward Anthropic prompt-caching cache_control through the gateway - #1152
Conversation
Hive's Anthropic-compatible /v1/messages endpoint rebuilds every request field by field on the way to the internal OpenAI-shaped dispatch path, and cache_control was never one of the fields it carried, so every Claude Code, Cline, Cursor and Aider agent session got zero prompt caching through Hive and paid full input price on every turn. Adds CacheControl to every documented placement (content block, system block, tool definition, request root) on both the Anthropic request types and their internal OAI-shaped counterparts, fixes the two spots in translate_request.go that were collapsing a typed content-block array down to a plain string (which cannot carry a per-block cache_control), and adds the exclusive-shape cache token counts to the Anthropic response and streaming usage objects so Claude Code's cost display shows the savings. Also preserves session_id passthrough (OpenRouter's sticky-routing hint) since it was dropped by the same field-by-field rebuild.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
sakibsadmanshajib
left a comment
There was a problem hiding this comment.
Independent adversarial review (security-reviewer pass). Read the pushed diff directly, ran the anthropic-package suite in the project's docker toolchain, and independently reproduced the author's mutation-check claim by reverting the flattening guard locally (test failed as claimed, then restored, confirmed clean). No blocking findings; two MEDIUM correctness/observability gaps and two LOW hardening nits, all inline below. I would not block this merge on them, but MEDIUM #1 should get a follow-up before this feature is called billing-complete.
Verified true (not just trusted):
- Request-side byte-for-byte forwarding claim: confirmed by reading chat_completions.go (raw body bytes, not a re-marshaled typed struct, flow into executeSync/executeStreaming) and litellm_client.go's rewriteModel (map[string]json.RawMessage passthrough, only the "model" key is touched). cache_control and session_id do survive this hop unchanged.
- Flattening regression guard: traced every touched branch in convertMessage/systemContent by hand for the no-cache_control case; all reproduce the prior output exactly. Also reproduced the author's mutation check myself (reverted the
parts[0].CacheControl == nilguard, TestToOAIRequest_CacheControl_SingleTextBlock_NotFlattenedToString failed with the exact flattened-string message the test asserts against, restored and confirmed clean). - No logging/persistence of raw cache_control or session_id values anywhere in the diff.
- Full
apps/edge-api/internal/anthropicsuite green in the docker toolchain, including all new cache_control tests. - No panics/nil-deref risk: every CacheControl pointer field is either passed straight through or nil-checked before use (systemContent's
bl.CacheControl != nil).
Would not block merge. Recommend addressing MEDIUM #1 (or explicitly tracking it) before calling cache billing observability done, and linking a tracking issue for MEDIUM #2's coordination gap so cache_creation_input_tokens isn't silently wrong in production between when this merges and when feat/cache-aware-billing lands.
…safe session_id truncation Addresses PR #1152 review threads. MEDIUM: freshInputTokens now logs a WARN naming the client alias and the upstream model whenever the inclusive-to-exclusive subtraction goes negative, instead of silently clamping to zero. A negative result only happens when OpenRouter's inclusive usage-shape assumption breaks, and this is the one place in the request lifecycle with the numbers already in hand to catch it. The equivalent alarm for the billed amount belongs to inference/pricing.go's CreditsForTokens on feat/cache-aware-billing, which is out of this package's scope and already briefed separately. LOW: cache_control is now validated at the trust boundary before any upstream round trip: type must be "ephemeral", ttl must be "5m" or "1h" if set, and at most 4 breakpoints per request, matching Anthropic's documented cap. Anthropic's real API already rejects a violation; this only saves a client mistake a full round trip to a paid upstream. LOW: session_id truncation is now rune-boundary-safe (utf8.RuneStart) rather than a raw byte-index cut, which could otherwise split a multibyte character and let json.Marshal silently substitute U+FFFD for the corrupted tail. Truncation now also logs a WARN so a support investigation has a trail.
…h the translator (#1163) ## Problem `apps/edge-api/internal/anthropic/translate_request.go` rebuilds the Anthropic `/v1/messages` request field by field into the internal OpenAI-shaped `OAIRequest`. Any field nobody explicitly carried is lost, silently, every time. `cache_control` was one of these and cost every agent client on the gateway its prompt caching until #1152 fixed it. This PR fixes the rest of the fields listed in #1153. ## Field-by-field disposition Per the task brief, each field below is deliberately dispositioned rather than reflexively carried through: 1. **`tool_choice: {"type": "none"}` — behaviour inversion, fixed first.** `convertToolChoice`'s switch had no `"none"` case, so it fell to the `default` branch and returned `"auto"`. A caller explicitly forbidding tool use got a model that could call them: not an omission, the opposite of the request. **Disposition: translated to the exact OpenAI equivalent** — `tool_choice: "none"` means the identical thing on both APIs. 2. **`metadata.user_id`** — parsed but never read by `ToOAIRequest`. **Disposition: translated to an equivalent** — lands on `OAIRequest.User`, OpenAI's own end-user tracking field (same purpose: abuse detection / per-user attribution on the provider side, different name). 3. **`tool_choice.disable_parallel_tool_use`** — no field at all. **Disposition: translated to an equivalent** — maps to OpenAI's `parallel_tool_calls`, inverted (`disable_parallel_tool_use: true` → `parallel_tool_calls: false`). Only ever emitted as `false`; Anthropic's default (`disable=false`, parallel allowed) already matches OpenAI's own default, so there's nothing to carry when the caller never asked to forbid it. 4. **`top_k`** — no field on `MessagesRequest` at all. **Disposition: forwarded as an extra field LiteLLM passes along.** There is no OpenAI-standard equivalent (it isn't part of the chat/completions surface), so this is carried through unchanged on `OAIRequest.TopK`. Whether the eventual provider honours it is a LiteLLM/provider concern; this translator must not be the one to silently drop it. 5. **Extended thinking** — no representation at all: no `thinking` request field, no `thinking`/`redacted_thinking` content blocks. A thinking-only message silently produced empty content (no case in `convertMessage`'s block switch matched, so both `parts` and `toolCalls` stayed empty). - `MessagesRequest.Thinking` (**disposition: carried through as-is** — identical field name and shape to Anthropic's own definition; LiteLLM's own Anthropic adapter interprets it on the way out, Hive has no local reasoning-budget concept to translate it into). - `ContentBlock.Thinking` / `.Signature` / `.Data` for `thinking` / `redacted_thinking` blocks in conversation history, surfaced on `OAIMessage.ThinkingBlocks` (**disposition: carried through via LiteLLM's own documented `thinking_blocks` convention** for round-tripping Anthropic extended-thinking content through an OpenAI-shaped assistant message — the signature must survive verbatim or Anthropic rejects the next turn, so this is a carry-through, not a reconstruction). Attached to every return path in `convertMessage`'s mixed-content section (alone, with tool_use, with text) so a thinking block can no longer produce a silently empty message in any combination. 6. **Pre-existing image drop in the tool-calls-plus-text branch** (preserved by #1152 to keep its regression guard exact) — **fixed here.** No existing test locked in the image-drop behaviour specifically (checked before touching it), so `partsNeedArrayForm` now also triggers on any non-text part, not just a cache breakpoint. An image mixed with a tool call no longer silently flattens away; the plain-text-only regression guard (#1152's actual concern) is untouched since it only ever exercised text parts. ## Structural fix Per the brief: fixing six instances of a recurring bug isn't fixing the bug. `TestToOAIRequest_MaximalRequest_NoFieldLost` (new file `translate_request_maximal_test.go`) builds one Anthropic request with every documented `MessagesRequest` field populated — including nested `ContentBlock`, `Tool`, and `ToolChoice` sub-fields — translates it once, and asserts each field survives with an explicit per-field check. `TestToOAIRequest_MaximalRequest_ToolChoiceNone_NotInverted` covers the one combination (`tool_choice: "none"`) that can't share the same request object as `disable_parallel_tool_use`. The next field added to `MessagesRequest` has a structural guard now, not just a hope someone remembers. ## Tests Table-driven, one test per fix plus the maximal round-trip: - `TestToOAIRequest_ToolChoice_None` - `TestToOAIRequest_Metadata_UserID`, `TestToOAIRequest_Metadata_Nil_UserOmitted` - `TestToOAIRequest_ToolChoice_DisableParallelToolUse`, `TestToOAIRequest_ToolChoice_ParallelToolUseAllowed_OmitsField` - `TestToOAIRequest_TopK` - `TestToOAIRequest_ThinkingConfig_Passthrough`, `TestToOAIRequest_ThinkingOnlyMessage_NotEmptyContent`, `TestToOAIRequest_RedactedThinkingBlock`, `TestToOAIRequest_ThinkingBlock_MixedWithToolUse` - `TestToOAIRequest_ImageBlock_MixedWithToolUse_NotDropped` - `TestToOAIRequest_MaximalRequest_NoFieldLost`, `TestToOAIRequest_MaximalRequest_ToolChoiceNone_NotInverted` **Mutation check (per the task brief):** temporarily reverted the `tool_choice: "none"` case in `convertToolChoice`, reran the suite — `TestToOAIRequest_ToolChoice_None` and `TestToOAIRequest_MaximalRequest_ToolChoiceNone_NotInverted` both failed with `want "none" got "auto"`, exactly the pre-fix inversion — then restored the fix and confirmed both pass again. ``` === RUN TestToOAIRequest_MaximalRequest_ToolChoiceNone_NotInverted translate_request_maximal_test.go:241: tool_choice: want "none" got "auto" --- FAIL: TestToOAIRequest_MaximalRequest_ToolChoiceNone_NotInverted (0.00s) === RUN TestToOAIRequest_ToolChoice_None translate_request_test.go:407: tool_choice json: want "none" got "auto" (an inversion back to "auto" means tool use is silently re-enabled) --- FAIL: TestToOAIRequest_ToolChoice_None (0.00s) FAIL github.com/sakibsadmanshajib/hive/apps/edge-api/internal/anthropic 0.011s ``` Restored, reran: ``` ok github.com/sakibsadmanshajib/hive/apps/edge-api/internal/anthropic 0.179s ``` Full `apps/edge-api` suite (all packages) green after the fix: ``` ok github.com/sakibsadmanshajib/hive/apps/edge-api/internal/anthropic 0.187s ok github.com/sakibsadmanshajib/hive/apps/edge-api/internal/inference 3.054s ... (all other apps/edge-api packages ok) ``` `go vet ./apps/edge-api/...` clean. `gofmt -l` clean on every touched file. ## Scope Strictly inside `apps/edge-api/internal/anthropic/`. Did not touch `.github/workflows/ci.yml`, any `go.mod`, `apps/control-plane/internal/payments/stripe/rail_test.go`, or `apps/edge-api/internal/inference/` (owned elsewhere). ## Buglog entry ```json {"error_message":"tool_choice:{\"type\":\"none\"} silently inverted to auto on /v1/messages, plus five other Anthropic request fields (metadata.user_id, disable_parallel_tool_use, top_k, thinking config/blocks, image-drop in tool-calls branch) silently lost by the same field-by-field rebuild","root_cause":"translate_request.go's convertToolChoice switch had no case for Anthropic's documented \"none\" tool_choice type and fell through to a default that returns \"auto\"; the same field-by-field rebuild pattern that caused the cache_control loss fixed in #1152 dropped five more fields with no representation on MessagesRequest/OAIRequest at all, and convertMessage's block switch had no case for thinking/redacted_thinking blocks so a thinking-only message produced an OAIMessage with empty content and no error","fix":"added an explicit \"none\" case to convertToolChoice mapping to OpenAI's own \"none\" sentinel; added MessagesRequest.TopK/Thinking, OAIRequest.TopK/Thinking/User/ParallelToolCalls, ToolChoice.DisableParallelToolUse, ContentBlock.Thinking/Signature/Data, and OAIMessage.ThinkingBlocks; wired Metadata.UserID to OAIRequest.User and disable_parallel_tool_use to the inverse parallel_tool_calls=false; widened partsNeedArrayForm to also trigger on any non-text part so an image mixed with a tool call keeps the block-array form instead of flattening away; added a maximal round-trip test asserting every documented MessagesRequest field survives translation as a structural guard against the next field going missing the same way","tags":["anthropic","translate_request","tool_choice","thinking","top_k","metadata","parallel_tool_calls","translator-field-loss"]} ``` Fixes #1153 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for extended thinking configuration and preservation of thinking content, including redacted blocks. * Added support for `top_k`, user metadata, and parallel tool-use controls. * Preserved images and other content when combined with tool calls. * **Bug Fixes** * Correctly handles `tool_choice: none`. * Rejects unsupported tool-choice values instead of silently changing behavior. * Prevents thinking-only messages and mixed content from being dropped. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…1177) ## Summary Live verification of every capability DEMO.md claims, against the actually deployed box (chat-hive, console-hive, api-hive, control-hive), run today after the 2026-08-25 Cloudflare regional-edge false alarm cleared. Full capability matrix and methodology in `docs/proof/demo-readiness-verify-2026-08-25/log.md`. **Confirmed fixed, DEMO.md corrected (was stale):** - Artifacts (#1110, fixed by PR #1141): `/artifacts` renders a real empty-state index today, not the "spins forever" DEMO.md described. No sidebar entry yet (tracked by #943 item 4, not new). - In-chat credits (#1063, fixed by PR #1119): a "You've used N credits today, N remaining" strip sits above the composer, matching the console Billing balance exactly. **Confirmed still broken, unchanged:** - Knowledge nav (#1109): clicking it still does nothing (URL unchanged). Direct `/knowledge` now answers an honest 404 instead of the originally-reported silent bounce home, a minor symptom shift, not a fix. **Verified today's merges, all landed after this session started:** - Cache-aware billing (#1157) and Anthropic `cache_control` passthrough (#1152): shipped and tested, but unexercised live. The catalog has no Anthropic model today, and a direct `usage_events` query shows zero cache-bearing requests since deploy. - Free pool failover (#1155) and the null-content coercion fix (#1169): no regressions in the trailing 24h of live traffic (zero error-status `usage_events` rows across 356 requests), though neither fix's specific trigger recurred live to re-test directly. - External uptime probe (#1166): confirmed running on its 15-minute schedule, all green. Added a T-1 checklist note pointing at it, since today's regional Cloudflare maintenance window is exactly the scenario it exists to catch. **Corrected a claim broader than the two named stale items:** the "not demoable: multi user isolation (#947, #948, #949 family)" line was itself stale. All three were fixed 2026-08-23 (PRs #960, #1067, #1091, #1096). One residual, #1056 (two Knowledge by-id/files routes still short-circuit on `role == admin`), is still open, so the line now says that precisely instead of citing three closed issues. ## New issues filed None. Every genuinely broken thing found already has an open tracking issue (#1109, #1056, #943). ## Verification - `node tools/lint-no-token-in-proof-captures.mjs` passes against the new proof log. - Live session obtained via the standard admin one-time-token mint (`docs/live-test-auth.md`), read-only against the demo fixture account, no password touched, no message sent, no key minted, no task submitted. - Screenshots posted separately to the PR via `scripts/post-pr-visual-proof.sh` (permanent GitHub Release, per `.wolf/decisions.md` D-042). ## Test plan - [x] `node tools/lint-no-token-in-proof-captures.mjs` - [x] Manual read of the rendered DEMO.md for internal consistency Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem
Hive's Anthropic-compatible
/v1/messagesendpoint (apps/edge-api/internal/anthropic/) had nocache_controlfield anywhere on its request or response types.translate_request.gorebuilds the Anthropic request field by field into an internal OpenAI-shapedOAIRequest, andcache_controlwas never one of the fields carried across, so it was silently dropped before the request ever reached LiteLLM. Every agent client that relies on Anthropic prompt caching (Claude Code, Cline, Cursor, Aider) got zero caching through Hive and paid full input price on every turn.What this PR does
Request side. Adds a
CacheControlstruct ({"type":"ephemeral","ttl":"5m"|"1h"}) and wires it through every documented placement:ContentBlock.CacheControlSystemFieldnow retains the originalBlocks(previously collapsed to one concatenated string, losing any block-level data)Tool.CacheControl(caching one tool in the array caches the whole array up to it; Anthropic has no separate "group" field)MessagesRequest.CacheControlAll four are mirrored onto the internal OAI-shaped types (
OAIContentPart,OAIMessage,OAIToolCall,OAITool,OAIRequest) with identical JSON shape, since the marshaledOAIRequestbody is forwarded essentially byte-for-byte to LiteLLM (litellm_client.go'srewriteModelonly ever touches the"model"key).The flattening bug. Two spots in
translate_request.gocollapsed a content-block array down to a plain string when it looked simple enough (a lone text block; a lone text block mixed with tool calls). A flat string cannot carry a per-blockcache_control, so a request that DID set a breakpoint on that block would have had it silently destroyed even with the field wired in. Both collapse sites now check for a cache breakpoint first and keep the block-array form when one is present, otherwise reproducing the exact prior output.TestToOAIRequest_CacheControl_SingleTextBlock_NotFlattenedToStringis a dedicated regression test for this; I confirmed it actually fails without the fix (reverted the guard locally, test failed with the flattened bare string, restored the fix, test passes).Response side — exclusive-shape echo contract. OpenRouter (Hive's only real dispatch path) reports usage in the OpenAI-compatible INCLUSIVE convention:
prompt_tokensalready contains the cached and written tokens. Anthropic-native clients (Claude Code's cost display included) expect the EXCLUSIVE convention:input_tokensas the fresh/uncached remainder only, pluscache_creation_input_tokensandcache_read_input_tokensreported alongside it.freshInputTokensintranslate_response.godoes that conversion (fresh = prompt - cached - written, clamped to 0, never negative) for both the non-streaming response and the streaming path (message_delta.usageinstream.go, since that relay only knows the terminal numbers once the upstream's usage-bearing chunk arrives —message_startfires before that and stays at its pre-existing zero, consistent with howoutput_tokensalready worked before this change).session_id. Not part of Anthropic's own
/v1/messagesschema, but OpenRouter uses it for sticky provider routing, which is what keeps a cache warm across turns. Added as passthrough onMessagesRequest/OAIRequest(truncated at 256 chars per OpenRouter's documented ceiling) so a client/proxy that sends it is no longer silently dropped by the same field-by-field rebuild.Merge coordination (feat/cache-aware-billing)
This PR's
OAIUsage.PromptTokensDetails({cached_tokens, cache_write_tokens}) is a capture-only struct in theanthropicpackage. The bytes it unmarshals come from the inference package's own re-marshal ofinference.UsageResponse(normalizeChatCompletioninchat_completions.go, and the typed chunk round-trip instream.go) — both of which do a typed unmarshal-then-remarshal, which silently drops any JSON field their own struct doesn't declare. Todayinference.UsageResponsehasPromptTokensDetails.CachedTokensbut noCacheWriteTokens, and OpenRouter'scache_creation_input_tokens/cache_read_input_tokensfields (if reported in Anthropic-native shape at all through the OpenRouter hop) have no matching field on that struct either.Net effect: the response-side cache token plumbing in this PR is real and tested against the
anthropicpackage's own types, but it cannot carry live data end-to-end untilfeat/cache-aware-billingadds a matchingCacheWriteTokens(and whatever else it needs) toinference.UsageResponsewith the same JSON field name (cache_write_tokens) I used here. I stayed out ofpricing.go,stream.go(inference package),orchestrator.goandtypes.goper the coordination boundary; this is the merge point to close by hand once both branches land — I'd suggest whichever branch merges second checks the other's final field names against this PR'sOAIPromptTokensDetails.Verified against the actual upstream path
chat_completions.go'shandleChatCompletionsforwards the original raw requestbodybytes downstream (not a re-marshal of its own typedChatCompletionRequest, whoseMessages/Toolsfields arejson.RawMessageanyway), andlitellm_client.go'srewriteModelonly replaces the"model"key viamap[string]json.RawMessage. So everycache_controlthis PR adds survives byte-for-byte from/v1/messagesthrough to LiteLLM's/chat/completions.deploy/litellm/config.yamlcurrently routes every chat alias toopenrouter/dots-studio/dots-3-note-preview:freeplus four paid non-Anthropic routes (deepseek-v4, doc-vlm, embedding fallback) — there is no Anthropic/Claude model in the live catalog today, socache_controlhas nowhere to take effect in production right now regardless of this fix. This is a routing/catalog fact, not something this PR's scope covers or should change. Recommend a follow-up live smoke test once/if a Claude route is added: send a >2k-token prompt with acache_controlbreakpoint to that alias and confirmcache_read_input_tokens/cache_creation_input_tokenscome back non-zero on the second turn.Other fields translate_request.go drops on the floor (audit, not fixed here)
Per the task brief's request to audit this file for every dropped field even if not all get fixed:
tool_choice: {"type":"none"}silently becomes"auto".convertToolChoice's switch only handlesauto/any/tool; anything else (including the documentednone, which means "forbid tool use") falls to thedefaultbranch and returnsauto. This is a behavior inversion, not just an omission — a caller that explicitly disabled tools would get tool use enabled. Worth its own follow-up issue; flagged here per audit but not fixed, out of this PR's cache_control scope.MessagesRequest.Metadata(metadata.user_id) is parsed butToOAIRequestnever reads it at all.ToolChoice.disable_parallel_tool_usehas no field at all on theToolChoicetype.top_ksampling parameter has no field onMessagesRequestat all (dropped at the type layer, not just the translator).thinkingrequest field, andthinking/redacted_thinkingcontent blocks, have no representation at all. A message consisting solely of a thinking block would fall throughconvertMessage's content-block switch with no matching case and produce an OAI message with empty content — silently, no error.convertMessagestill drops anyimageblock mixed with tool calls when neither has acache_control(documented in a code comment at the fix site rather than fixed, to keep the no-cache-control regression guard exact).Tests
Table-driven and mutation-checked (I deliberately broke the flattening guard locally and confirmed the dedicated test failed, then restored the fix and confirmed it passed again — see
cache_control_test.go):TestToOAIRequest_CacheControl_Placements— system block, message content block (+ 1h ttl variant), tool definition, request rootTestToOAIRequest_CacheControl_SingleTextBlock_NotFlattenedToString— the flattening regression guardTestToOAIRequest_NoCacheControl_Unchanged— regression guard: a request with no cache_control anywhere serializes with zerocache_controlkeys anywhere in the outputTestToOAIRequest_CacheControl_ToolResultBlock,TestToOAIRequest_CacheControl_ToolUseBlockTestToOAIRequest_SessionID_Passthrough,TestToOAIRequest_SessionID_TruncatedAt256TestFromOAIResponse_CacheTokens_NonStreaming,TestFromOAIResponse_CacheTokens_ClampedNeverNegative,TestFromOAIResponse_NoCacheDetails_InputTokensUnchangedTestSSETranslator_CacheTokensInMessageDeltaFull
apps/edge-apisuite (all packages, includinginference) passes on the rebased branch.Buglog entry
{"error_message":"cache_control silently dropped on /v1/messages, zero prompt caching for agent clients","root_cause":"translate_request.go rebuilds the Anthropic request field-by-field into OAIRequest; cache_control (content block, system block, tool, request root) was never one of the carried fields, and two collapse sites in convertMessage flattened a typed content-block array to a plain string, which cannot carry a per-block cache_control even once the field exists","fix":"added CacheControl to ContentBlock/Tool/MessagesRequest/SystemField and their OAI-shaped mirrors; guarded both flattening sites in translate_request.go to keep block-array form when a cache breakpoint is present; added the exclusive-shape cache_creation_input_tokens/cache_read_input_tokens echo to ResponseUsage and StreamUsage via freshInputTokens' inclusive-to-exclusive subtraction","tags":["anthropic","cache_control","prompt-caching","translate_request","billing-coordination"]}Test plan
go build ./apps/edge-api/...go vet ./apps/edge-api/...go test ./apps/edge-api/... -count=1(all packages green, rebased on origin/main)