feat(caching): add provider cache writes toggle - #2407
Conversation
Adds a project-level toggle that suppresses the gateway's automatic injection of cache_control / cachePoint markers into Anthropic and AWS Bedrock Claude requests. Caller-supplied markers still pass through; only the gateway's length-based heuristic auto-injection is gated. Lets users on sparse prompt patterns avoid paying the 1.25x/2x cache-write premium when their gap between requests exceeds the cache TTL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds ChangesProvider cache control feature
Sequence DiagramsequenceDiagram
participant chat as apps/gateway/src/chat/chat.ts
participant cacheHelper as packages/db/src/cache-helpers.isCachingEnabled()
participant prepare as packages/actions/src/prepare-request-body.ts
participant transform as packages/actions/src/transform-anthropic-messages.ts
chat->>cacheHelper: isCachingEnabled(project.id)
cacheHelper-->>chat: { enabled, duration, providerCacheControlEnabled }
chat->>prepare: prepareRequestBody(..., providerCacheControlEnabled)
prepare->>prepare: gate or strip cache markers based on providerCacheControlEnabled
alt provider == "anthropic"
prepare->>transform: transformAnthropicMessages(messages, ..., providerCacheControlEnabled)
transform->>transform: auto-inject cache_control only if providerCacheControlEnabled
else provider == "bedrock"
prepare->>prepare: insert cachePoint only if providerCacheControlEnabled
end
prepare-->>chat: final request body
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3921bd7d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const shouldCache = | ||
| providerCacheControlEnabled && | ||
| msg.content.length >= bedrockMinCacheableChars && | ||
| bedrockCacheControlCount < bedrockMaxCacheControlBlocks; |
There was a problem hiding this comment.
The new providerCacheControlEnabled check is applied to heuristic cache-point insertion here, but later in the Bedrock branch the turn-boundary logic still adds a cachePoint whenever bedrockMessages.length >= 3 and the block limit allows it. In multi-turn Bedrock conversations with the toggle turned off, the gateway still auto-injects a cache marker and can incur cache-write charges, so the new setting does not fully disable automatic provider cache writes. The turn-boundary insertion path should be gated by the same flag.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/gateway/src/chat/chat.ts (1)
4298-4326:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread
providerCacheControlEnabledinto the retry/fallback provider context too.The initial request body passes
providerCacheControlEnabledintoprepareRequestBody(...), but the retry context rebuilds provider context viaresolveProviderContext(...)andprepareRequestBody(...)there doesn’t clearly show forwarding this flag (andProviderContextOptions/options passed on retry don’t appear to include it). If retries fall back with the default, cache-control markers can be auto-injected when the toggle is off.🤖 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 `@apps/gateway/src/chat/chat.ts` around lines 4298 - 4326, The retry/fallback path is not receiving the providerCacheControlEnabled flag so rebuilt contexts default and may inject cache-control markers; update resolveProviderContext and the options passed into it (e.g., ProviderContextOptions used in the retry/fallback logic) to accept providerCacheControlEnabled, and then pass that same flag into prepareRequestBody when building the retry requestBody; ensure the retry call sites that construct ProviderContextOptions and call prepareRequestBody include providerCacheControlEnabled so the retry/fallback provider honors the original cache-control toggle.packages/actions/src/prepare-request-body.ts (1)
2031-2061:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate Bedrock turn-boundary cachePoint injection behind the toggle.
Line 2031 still auto-adds a
cachePointeven whenproviderCacheControlEnabledisfalse. That keeps automatic cache writes active (and billable) after opting out.Suggested fix
- if (bedrockMessages.length >= 3) { + if (providerCacheControlEnabled && bedrockMessages.length >= 3) {🤖 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 `@packages/actions/src/prepare-request-body.ts` around lines 2031 - 2061, The turn-boundary cachePoint injection currently runs unconditionally; wrap the logic that finds lastUserIdx and potentially pushes createBedrockCachePoint() behind a check of providerCacheControlEnabled so no cachePoint is added when the toggle is false. Specifically, inside the code handling bedrockMessages (referencing bedrockMessages, lastUserIdx, boundaryIdx, boundaryMsg, lastBlock, createBedrockCachePoint, bedrockCacheControlCount, bedrockMaxCacheControlBlocks), add or combine an if (providerCacheControlEnabled) guard before computing boundaryIdx and pushing the cache point (or skip the push when providerCacheControlEnabled is false).
🧹 Nitpick comments (1)
apps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/caching-settings-rsc.tsx (1)
52-52: 💤 Low valueConsider adding a fallback for consistency.
The form component (line 63 of
caching-settings.tsx) uses a?? truefallback when reading this same field frominitialData, but this server component passesproject.providerCacheControlEnableddirectly. For defensive consistency, consider:- providerCacheControlEnabled: project.providerCacheControlEnabled, + providerCacheControlEnabled: project.providerCacheControlEnabled ?? true,This ensures the default aligns with both the database schema default and the form's fallback behavior, even if the Project type allows undefined.
🤖 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 `@apps/ui/src/app/dashboard/`[orgId]/[projectId]/settings/preferences/_components/caching-settings-rsc.tsx at line 52, The server component currently passes providerCacheControlEnabled directly from project; change it to use a defensive fallback (project.providerCacheControlEnabled ?? true) so it matches the form's `?? true` behavior and the DB default; update the object where providerCacheControlEnabled is set in this file (referencing the project object and the exported value used by the form) to use that nullish-coalescing fallback so undefined values behave consistently.
🤖 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.
Outside diff comments:
In `@apps/gateway/src/chat/chat.ts`:
- Around line 4298-4326: The retry/fallback path is not receiving the
providerCacheControlEnabled flag so rebuilt contexts default and may inject
cache-control markers; update resolveProviderContext and the options passed into
it (e.g., ProviderContextOptions used in the retry/fallback logic) to accept
providerCacheControlEnabled, and then pass that same flag into
prepareRequestBody when building the retry requestBody; ensure the retry call
sites that construct ProviderContextOptions and call prepareRequestBody include
providerCacheControlEnabled so the retry/fallback provider honors the original
cache-control toggle.
In `@packages/actions/src/prepare-request-body.ts`:
- Around line 2031-2061: The turn-boundary cachePoint injection currently runs
unconditionally; wrap the logic that finds lastUserIdx and potentially pushes
createBedrockCachePoint() behind a check of providerCacheControlEnabled so no
cachePoint is added when the toggle is false. Specifically, inside the code
handling bedrockMessages (referencing bedrockMessages, lastUserIdx, boundaryIdx,
boundaryMsg, lastBlock, createBedrockCachePoint, bedrockCacheControlCount,
bedrockMaxCacheControlBlocks), add or combine an if
(providerCacheControlEnabled) guard before computing boundaryIdx and pushing the
cache point (or skip the push when providerCacheControlEnabled is false).
---
Nitpick comments:
In
`@apps/ui/src/app/dashboard/`[orgId]/[projectId]/settings/preferences/_components/caching-settings-rsc.tsx:
- Line 52: The server component currently passes providerCacheControlEnabled
directly from project; change it to use a defensive fallback
(project.providerCacheControlEnabled ?? true) so it matches the form's `?? true`
behavior and the DB default; update the object where providerCacheControlEnabled
is set in this file (referencing the project object and the exported value used
by the form) to use that nullish-coalescing fallback so undefined values behave
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2cdd528a-9f12-46d1-b4f4-e48cc75ff01b
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (15)
apps/api/src/routes/organization.tsapps/api/src/routes/projects.tsapps/api/src/routes/v1-master.tsapps/docs/content/features/caching/provider-cache-control.mdxapps/gateway/src/chat/chat.tsapps/ui/src/app/dashboard/[orgId]/[projectId]/settings/preferences/_components/caching-settings-rsc.tsxapps/ui/src/components/settings/caching-settings.tsxapps/ui/src/types/settings.tspackages/actions/src/prepare-request-body.tspackages/actions/src/transform-anthropic-messages.tspackages/db/migrations/1779728096_needy_robbie_robertson.sqlpackages/db/migrations/meta/1779728096_snapshot.jsonpackages/db/migrations/meta/_journal.jsonpackages/db/src/cache-helpers.tspackages/db/src/schema.ts
Extends the provider cache writes toggle to also strip cache_control markers that callers send themselves, not just suppress the gateway's auto-injection heuristic. Without this, coding agents (Claude Code, Cursor, Cline) would still trigger cache writes on every request because they emit explicit markers regardless of the user's request cadence — defeating the toggle for the actual reported use case. Adds a regression test covering both system and user message stripping, and updates UI/docs copy to reflect the broader semantics. Verified end-to-end against api.anthropic.com: a 2509-token prompt with cache_control returns cache_creation_input_tokens=2509 (write premium); the same prompt with markers stripped returns 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/actions/src/prepare-request-body.spec.ts (1)
232-300: ⚡ Quick winAdd an AWS Bedrock counterpart for this opt-out stripping test.
This validates Anthropic well, but the same toggle contract applies to Bedrock in
prepareRequestBody; adding one Bedrock case here will prevent regressions in the second provider path.🤖 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 `@packages/actions/src/prepare-request-body.spec.ts` around lines 232 - 300, Add a parallel test that mirrors the Anthropic "strips caller-supplied cache_control when providerCacheControlEnabled is false" case but uses the Bedrock branch of prepareRequestBody: call prepareRequestBody with provider "bedrock" (or the Bedrock model id used elsewhere in tests), include system/user content blocks with caller-supplied cache_control entries, pass providerCacheControlEnabled = false, and assert that the returned request body (e.g., requestBody.prompts or the Bedrock-specific fields) has no cache_control on any content blocks; reference prepareRequestBody and the existing Anthropic test to copy payload shape and assertions so the Bedrock code path is exercised the same way.
🤖 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.
Inline comments:
In `@packages/actions/src/prepare-request-body.spec.ts`:
- Around line 286-297: The tests use force-casts `(block as any).cache_control`
in the cache-control assertions; replace these with a small typed helper to
narrow the shape safely (e.g., create a helper function like
`getCacheControl(value: unknown): unknown` or `assertNoCacheControl(value:
unknown): void`) and use it in the loops over `requestBody.system` and
`requestBody.messages` to access `cache_control` without `as any`; update the
assertions to call that helper (or narrow with a type guard) when inspecting
`block.cache_control` so the code remains type-safe while keeping the same
expectation logic for `requestBody`, `block`, and `msg` checks.
---
Nitpick comments:
In `@packages/actions/src/prepare-request-body.spec.ts`:
- Around line 232-300: Add a parallel test that mirrors the Anthropic "strips
caller-supplied cache_control when providerCacheControlEnabled is false" case
but uses the Bedrock branch of prepareRequestBody: call prepareRequestBody with
provider "bedrock" (or the Bedrock model id used elsewhere in tests), include
system/user content blocks with caller-supplied cache_control entries, pass
providerCacheControlEnabled = false, and assert that the returned request body
(e.g., requestBody.prompts or the Bedrock-specific fields) has no cache_control
on any content blocks; reference prepareRequestBody and the existing Anthropic
test to copy payload shape and assertions so the Bedrock code path is exercised
the same way.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5343ebde-9fb2-4287-a822-812e1abc64dc
📒 Files selected for processing (4)
apps/docs/content/features/caching/provider-cache-control.mdxapps/ui/src/components/settings/caching-settings.tsxpackages/actions/src/prepare-request-body.spec.tspackages/actions/src/prepare-request-body.ts
✅ Files skipped from review due to trivial changes (1)
- apps/docs/content/features/caching/provider-cache-control.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/settings/caching-settings.tsx
Three fixes uncovered while testing the provider cache toggle: - Bedrock turn-boundary cachePoint injection wasn't gated by providerCacheControlEnabled, so multi-turn conversations leaked one cache write per request even with the toggle off. - transformAnthropicMessages now counts caller-supplied cache_control markers toward the 4-block cap; otherwise Claude Code's 4 markers plus our turn-boundary injection produced "Found 5" 400 errors. - Retry path now threads providerCacheControlEnabled through resolveProviderContext so fallback attempts honor the toggle too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve conflict in apps/gateway/src/chat/chat.ts: keep providerCacheControlEnabled from isCachingEnabled (this branch) while applying upstream's devPlan gate that disables gateway-side response caching for dev plan organizations.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ache-toggle # Conflicts: # apps/gateway/src/chat/chat.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d85e137d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { | ||
| enabled: projectCachingEnabled, | ||
| duration: cacheDuration, | ||
| providerCacheControlEnabled, | ||
| } = await isCachingEnabled(project.id); |
There was a problem hiding this comment.
For dev-plan organizations this new unconditional isCachingEnabled(project.id) call adds a second project DB dependency even though cachingEnabled is forced to false on the next line. If Postgres is unavailable and the existing swr:project:<id> mirror used by findProjectById is populated but the newly required swr:project:cachingEnabled:<id> mirror is not, dev-plan chat requests that previously proceeded will now fail before reaching the provider because isCachingEnabled rethrows on a fallback miss. The provider cache flag is already available on the fetched project, so the dev-plan branch can avoid this lookup or default from that row.
Useful? React with 👍 / 👎.
2 participants
Summary
Adds a project-level "Provider Cache Writes" toggle (default on) that suppresses the gateway's automatic injection of
cache_control/cachePointmarkers into Anthropic and AWS Bedrock Claude requests, and strips caller-supplied markers when off. This lets users on sparse prompt patterns avoid paying the 1.25× (5m) / 2× (1h) cache-write premium when their gap between requests exceeds the cache TTL, which was the reported pain point (see Discord thread on Opus 4.7 sparse usage).This also covers coding agents like Claude Code, Cursor, and Cline that always emit
cache_controlmarkers regardless of the user's usage pattern — without the strip, those markers would flow through unchanged and the toggle would have no effect.Verified empirically via direct curl to api.anthropic.com that omitting the markers cleanly disables cache writes (
usage.cache_creation_input_tokens=0), per Anthropic docs and AWS Bedrock docs.Anthropic vs Bedrock cache payloads
The two providers use different wire formats for prompt caching, and the strip / auto-inject logic has to handle both:
cache_control: {"type": "ephemeral", "ttl"?: "5m"|"1h"}field on atextcontent block. The field sits inside the same block as the text.cachePoint: {"type": "default", "ttl"?: "5m"|"1h"}content block inserted after the text block it should cache. The format is unrelated to Anthropic'scache_control;{"type": "ephemeral"}is not a valid Bedrock cachePoint type and is silently ignored.cache_controlformat, but the gateway routes Bedrock Claude through Converse, so that path is not exercised here.When the toggle is off:
cache_controlfrom all message content parts inprepareRequestBodyso caller markers don't bleed into either provider.providerCacheControlEnabledfor both the Anthropiccache_controlpath and the BedrockcachePointpath.cachePointper request.cache_controlmarkers toward Anthropic's 4-block cap, so requests from agents that already include their own 4 markers don't trigger the "Found 5 blocks with cache_control" 400 after our turn-boundary injection.Test plan
usage.cache_creation_input_tokens=0in the responsecache_write_tokens=0in the activity logcache_creation_input_tokens>0(write) thencache_read_input_tokens>0on a follow-up callcache_write_tokens=0on every request with the toggle offSummary by CodeRabbit
New Features
Behavior Changes
Documentation
Tests & Migrations