feat(cache): optimize provider cache economics - #624
Open
SantiagoDePolonia wants to merge 4 commits into
Open
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughThe change adds exact-cache JSON canonicalization and concurrent miss coalescing. It adds provider-native prompt-cache planning for chat and Responses requests, routing integration, Bedrock markers, Gemini cached-content reuse, and cache and compression documentation. ChangesCache behaviors
Pro compression documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant CachePlanner
participant Provider
Client->>Router: send chat or Responses request
Router->>CachePlanner: plan eligible request
CachePlanner-->>Router: return cloned request with cache metadata
Router->>Provider: forward planned request
Provider-->>Client: return response or stream
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
Confidence Score: 3/5Not safe to merge until T-Rex findings are addressed. An executed request flow reproduced a functional defect in a primary prompt-cache scenario: the router plans a Gemini cache entry, but the provider sends an uncached generation request. The result makes the newly added cache optimization ineffective for large system prompts with a single current user turn. T-Rex reproduced 2 failing behaviors at runtime in internal/providers/gemini/gemini.go; the change needs fixes before it is safe to merge. Files Needing Attention:
 T-Rex LogsWhat T-Rex did
Comments Outside Diff (1)
Reviews (2): Last reviewed commit: "feat(cache): optimize provider cache eco..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
from
July 31, 2026 14:41
e413b30 to
bfb4801
Compare
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
from
August 1, 2026 11:23
bfb4801 to
93d990c
Compare
Contributor
Author
|
Rebased onto the updated #622 and added the provider cache-planner kill switch in 93d990c. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/providers/bedrock/chat.go (1)
156-182: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftHandle failed Bedrock cache checkpoints before retrying.
Bedrock prompt caching has model-specific support and minimum prefix thresholds, such as 1,536 tokens for Nova profiles. The planner uses a fixed 1,024-token minimum and
(len(json)+3)/4, and the Bedrock request path insertscachePointunconditionally. If Bedrock rejects the checkpoint, the request fails without fallback; detect the error and retry with cache points stripped, or scope cache points to models with a higher-confidence minimum.🤖 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 `@internal/providers/bedrock/chat.go` around lines 156 - 182, Update the Bedrock request flow around cache-point insertion and error handling to recover from rejected cache checkpoints: when a request fails because of a cache-point validation or support error, remove all generated cache-point blocks and retry once without them. Preserve the existing request behavior for successful cache-enabled calls and return the original failure for unrelated errors; use the visible cache-point handling in the message conversion path and its enclosing request method as the implementation anchors.
🤖 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 `@docs/features/cache.mdx`:
- Around line 109-114: Update the Gemini cached-content documentation bullet to
state that the object has a 300-second TTL and is reused only when the native
Gemini API is enabled via USE_GOOGLE_GEMINI_NATIVE_API. Clearly distinguish the
default behavior from the override condition while keeping the explanation
concise and user-focused.
In `@internal/providers/bedrock/chat.go`:
- Around line 193-195: Update isGatewayCachePoint to decode the raw value from
fields.Lookup("_gomodel_cache_point") as a JSON boolean and return the decoded
result, accepting valid boolean representations regardless of raw formatting.
Preserve false behavior when the field is missing or cannot be decoded.
In `@internal/providers/cache_planner_test.go`:
- Around line 12-91: Add a table-driven test covering planResponses and
markOpenAIResponsesBreakpoint for string, []core.ContentPart, and []any content
shapes. Assert the OpenAI prompt_cache_key and explicit breakpoint, the
Anthropic cache directive, skipping plans below the provider minimum, and that
the original request remains unchanged after planning.
In `@internal/providers/cache_planner.go`:
- Around line 16-19: The cache-point marker is duplicated across packages,
creating an unsynchronized contract. In internal/providers/cache_planner.go
lines 16-19, define the marker as an exported constant in internal/core and
reference it from the cache planner; in internal/providers/bedrock/chat.go lines
193-195, replace the inline marker literal with the same shared constant.
- Around line 42-58: Update planChat to estimate the prefix size from message
text lengths before constructing prefixBody, returning the original request when
the estimate is below providerCacheMinimum; only marshal the tools and prior
messages after the threshold passes, preserving the existing cacheAffinityKey
input and cloneChatRequest flow.
- Around line 146-153: Extend hasCacheDirective and its caller to inspect the
prefix’s message ExtraFields and each content part’s ExtraFields, not only
request-level fields. Treat cache_control and prompt_cache_breakpoint, along
with the existing cache directive keys, as indicators and return true when any
are present so the planner does not add a duplicate directive.
- Around line 71-74: Update planChat so the Anthropic cache plan produced by
mergeCacheExtras is applied before hasCacheDirective rejects existing
cache_control, while preserving rejection of client-supplied directives. Ensure
the resulting planner-owned cache data is forwarded through forwardChatRequest
and remains consumable by adaptAnthropicCacheControl.
In `@internal/providers/gemini/gemini_test.go`:
- Around line 40-47: Update the httptest handler in the Gemini test to avoid
calling t.Fatalf from its server goroutine. Record an unexpected r.URL.Path for
assertion after the requests complete, or use t.Errorf followed by a plain
return, while preserving the existing path validation and response behavior.
- Around line 38-74: Expand TestPrepareCachedContentCreatesAndReusesObject into
table-driven cases covering creation errors, empty response names, entries
expiring within 15 seconds, and re-creation after expiry. Assert best-effort
failures preserve the original Contents, SystemInstruction, and Tools, while
expired entries trigger a new cache creation; retain the existing successful
creation-and-reuse assertions.
In `@internal/providers/gemini/gemini.go`:
- Around line 591-596: Bound growth of p.cacheObjects in the cache insertion
path around cachedContentObject: before or during inserting a new prefix, sweep
entries whose expiresAt is in the past when the map reaches a defined size
threshold. Preserve current lookup and expiry behavior, and ensure the cleanup
runs under p.cacheMu without leaving stale entries indefinitely.
- Around line 599-604: Update the cache-creation error path surrounding the
visible err check in the Gemini flow to log failures at debug or warn level,
including the model identifier and error reason. Preserve the existing
successful cached-prefix behavior, and do not include the cache key contents or
credentials in the log.
- Around line 565-598: Update prepareCachedContent and the cache lookup/storage
paths around cacheFlight, cachedContentObject, and cacheObjects so
cached-content entries are keyed by the active Gemini API key/project identity
in addition to the prompt cache key. Ensure both creation and reuse use this
scoped key, preventing requests under one credential context from referencing
resources created under another; alternatively disable cached-content reuse when
multiple rotating keys are configured.
In `@internal/providers/router.go`:
- Around line 581-598: In plannedChatRequest and plannedResponsesRequest, retain
the cachePlanner nil checks but remove the duplicated message-count, input type,
and length eligibility guards. After forwarding the request, delegate directly
to cachePlanner.planChat or planResponses so the planner owns eligibility and
planning order.
In `@internal/responsecache/exact_cache_test.go`:
- Around line 189-197: Update the concurrent test around driveHandleRequest and
wg.Go to avoid calling testing.T.Fatalf from worker goroutines: capture each
goroutine’s error/result and assert or fail from the test goroutine after
wg.Wait(). Replace the fixed time.Sleep synchronization with an atomic counter
or equivalent barrier that keeps the leader blocked until all follower requests
have entered the handler, ensuring the calls == 1 assertion is deterministic.
In `@internal/responsecache/simple.go`:
- Around line 111-124: Replace the blocking m.misses.Do call in the cache-miss
flow with m.misses.DoChan, then select between its result channel and
ex.Context().Done(). Return the context cancellation error immediately when the
follower disconnects or its deadline expires, while allowing the leader
operation to continue for other waiters and preserving the existing result/error
handling when the singleflight call completes.
---
Outside diff comments:
In `@internal/providers/bedrock/chat.go`:
- Around line 156-182: Update the Bedrock request flow around cache-point
insertion and error handling to recover from rejected cache checkpoints: when a
request fails because of a cache-point validation or support error, remove all
generated cache-point blocks and retry once without them. Preserve the existing
request behavior for successful cache-enabled calls and return the original
failure for unrelated errors; use the visible cache-point handling in the
message conversion path and its enclosing request method as the implementation
anchors.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9350a263-a7da-42f8-a190-9695c8189cbd
📒 Files selected for processing (13)
.env.templatedocs/features/cache.mdxdocs/pro.mdxinternal/core/types.gointernal/providers/bedrock/chat.gointernal/providers/cache_planner.gointernal/providers/cache_planner_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/router.gointernal/responsecache/exact_cache_test.gointernal/responsecache/simple.gointernal/responsecache/stream_cache.go
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
from
August 1, 2026 12:17
93d990c to
2547be9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@docs/features/cache.mdx`:
- Around line 116-122: Update the provider prompt-cache planning documentation
near the PROVIDER_PROMPT_CACHE_PLANNER_ENABLED example to state that invalid
values retain the default enabled behavior. Clearly distinguish this from the
explicit value false, while keeping the existing concise default and override
guidance.
In `@internal/providers/bedrock/chat.go`:
- Around line 175-183: Update the assistant-message handling around
convertAssistantMessage and isGatewayCachePoint so the cache-point block is
appended only when blocks already contains at least one content block. Preserve
the existing empty-message behavior by allowing empty assistant messages to
reach appendOrMerge without adding a cache point.
In `@internal/providers/cache_planner_test.go`:
- Around line 57-82: Update TestNewCachePlanner_EnvironmentKillSwitch to unset
providerPromptCachePlannerEnabledEnv for the default-on case and call t.Setenv
only when a test value is explicitly declared, so the unset branch is exercised.
In TestCachePlannerHonorsMinimumAndClientDirective, set
providerPromptCachePlannerEnabledEnv to "true" before constructing the planner,
ensuring the assertions exercise the minimum threshold and client directive
logic rather than an ambient disabled configuration.
In `@internal/providers/cache_planner.go`:
- Around line 23-40: Update newCachePlanner to always return a non-nil
*cachePlanner containing an enabled bool that reflects the parsed environment
flag, including the disabled case. Modify planChat and planResponses to return
the request unchanged when the planner is disabled, while preserving existing
planning behavior when enabled.
- Around line 75-76: The Bedrock planner can mark a tool-result message that the
adapter ignores. Update the "bedrock" case in the cache planning flow and
markLastChatPrefix usage to restrict selection to roles handled by Bedrock's
convertMessages, while preserving the existing text/tool-call eligibility and
marker behavior.
In `@internal/providers/gemini/gemini_test.go`:
- Around line 62-68: Extend the cache test around prepareCachedContent to
exercise cacheFlight with concurrent calls using the same key: start N
goroutines against a creation handler that blocks until all callers arrive,
release them together, and assert that exactly one creation occurs. Ensure the
test synchronizes goroutine completion and is race-safe, while preserving the
existing sequential cache-hit coverage.
In `@internal/providers/gemini/gemini.go`:
- Around line 565-604: Update prepareCachedContent to add a short negative cache
entry when cached-content creation fails, and consult that entry to skip
repeated attempts for the same key. Use p.backend to restrict cached-content
creation to the implemented Gemini resource path, skipping unsupported backends
such as Vertex rather than posting to /cachedContents; preserve the existing
success caching behavior for supported Gemini requests.
In `@internal/responsecache/exact_cache_test.go`:
- Around line 216-223: The test
TestHashRequest_CanonicalizesJSONFormattingAndKeyOrder should become
table-driven and cover canonical-equivalent formatting/key order plus malformed
JSON, multiple JSON values, and distinct numeric spellings. Assert the
documented raw-byte fallback for invalid or multiple-value input and the
expected json.Number-preserving behavior for valid numeric variants, while
retaining the existing equivalent-object assertion.
In `@internal/responsecache/simple.go`:
- Around line 111-124: Update the miss handling around m.misses.Do and
ex.Capture so leader execution errors are stored in missResult rather than
returned directly through singleflight. Return the stored error only when the
result belongs to owner; when a follower receives a missResult containing an
error, invoke next() to execute independently.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4434329e-0b8e-421c-9238-2d2baacd88e0
📒 Files selected for processing (13)
.env.templatedocs/features/cache.mdxdocs/pro.mdxinternal/core/types.gointernal/providers/bedrock/chat.gointernal/providers/cache_planner.gointernal/providers/cache_planner_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/router.gointernal/responsecache/exact_cache_test.gointernal/responsecache/simple.gointernal/responsecache/stream_cache.go
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
2 times, most recently
from
August 1, 2026 16:12
6dc9466 to
6adc6a2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@internal/providers/gemini/gemini.go`:
- Around line 553-555: The guard in the Gemini caching conversion must not
reject requests solely because body.Contents has fewer than two items when
SystemInstruction is present. Update the early-return condition in the relevant
caching function to allow a non-empty SystemInstruction with one remaining
content item, while preserving the existing nil-body and already-cached checks.
- Line 565: Update the cache creation closure in the cacheFlight.Do call to
derive a detached context with its own timeout instead of using the requesting
caller’s ctx, then pass that createCtx to p.nativeClient.Do. Ensure the timeout
context is canceled after creation and preserves the shared result for all
waiters.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11c40eed-61bd-49ec-8cd7-87bc51b02d19
📒 Files selected for processing (13)
.env.templatedocs/features/cache.mdxdocs/pro.mdxinternal/core/types.gointernal/providers/bedrock/chat.gointernal/providers/cache_planner.gointernal/providers/cache_planner_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/router.gointernal/responsecache/exact_cache_test.gointernal/responsecache/simple.gointernal/responsecache/stream_cache.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/providers/cache_planner.go`:
- Around line 122-144: The cloneChatRequest function currently loses
non-serialized fields during the JSON round trip. After unmarshalling the cloned
request, explicitly copy fields such as AnthropicMessagesRequest and
PromptCachePlan from req onto clone, while preserving the existing serialization
behavior for other fields.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 14d00085-ae06-4250-8ae9-73c33b4252fb
📒 Files selected for processing (13)
.env.templatedocs/features/cache.mdxdocs/pro.mdxinternal/core/types.gointernal/providers/bedrock/chat.gointernal/providers/cache_planner.gointernal/providers/cache_planner_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/router.gointernal/responsecache/exact_cache_test.gointernal/responsecache/simple.gointernal/responsecache/stream_cache.go
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
from
August 1, 2026 17:00
6adc6a2 to
89b23f8
Compare
SantiagoDePolonia
force-pushed
the
feat/cache-economic-optimization
branch
from
August 1, 2026 17:36
89b23f8 to
0a2a57b
Compare
SantiagoDePolonia
changed the base branch from
feat/prompt-cache-affinity
to
main
August 1, 2026 17:36
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/responsecache/simple.go (1)
104-146: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
finishMissagainst a panic inex.Capture/next().In the leader branch,
m.finishMiss(key, call, data, err)runs only afterex.Capturereturns normally. Ifnext()panics,finishMissnever runs. The map entry forkeyinm.missesstays forever, andcall.donenever closes.Every later request that hashes to the same
keybecomes a follower. Each follower waits oncall.doneuntil its own context times out, then returns a context error instead of retrying independently. Thecall.err != nilretry path at Line 132 only fires afterclose(call.done), which never happens for a poisoned key.
golang.org/x/sync/singleflight, which this code replaces, guards against exactly this case: on panic, it recovers, propagates the panic to all waiters, and still removes the call from its internal map. The current implementation drops that guarantee.Wrap the leader branch so
finishMissalways executes, then re-panic to preserve the original failure signal.🔧 Proposed fix for panic safety in the leader branch
call, leader := m.joinMiss(key) if leader { - data, ok, err := ex.Capture("response cache: failed to capture cacheable response body", next) - if err == nil && ok { - m.enqueueWrite(cacheWriteJob{key: key, data: data}) - } else { - data = nil - } - m.finishMiss(key, call, data, err) - return err + var data []byte + var err error + finished := false + defer func() { + if finished { + return + } + r := recover() + m.finishMiss(key, call, nil, err) + if r != nil { + panic(r) + } + }() + var ok bool + data, ok, err = ex.Capture("response cache: failed to capture cacheable response body", next) + if err == nil && ok { + m.enqueueWrite(cacheWriteJob{key: key, data: data}) + } else { + data = nil + } + m.finishMiss(key, call, data, err) + finished = true + return err }🤖 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 `@internal/responsecache/simple.go` around lines 104 - 146, Update the leader branch of simpleCacheMiddleware.StoreAfter so finishMiss always executes when ex.Capture or next panics, ensuring the miss entry is removed and call.done is closed. Add panic-safe cleanup around the capture/write flow, then re-panic with the original value after finishMiss so callers and followers observe the existing failure signal.internal/providers/cache_control.go (1)
47-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the re-marshal for batch items without Anthropic cache control.
adaptAnthropicBatchCacheControlcallsjson.Marshal(adaptAnthropicCacheControl(chat, providerType))for every item once the provider doesn't accept Anthropic cache control, even when that specific item carries nocache_controldirective at all (in which caseadaptAnthropicCacheControlreturns the request unchanged). For large batches, this means decoding and re-encoding every item purely to reproduce the same bytes.Check
hasAnthropicCacheControl(chat)before re-marshaling, and reuseitem.Bodywhen it returns false.♻️ Proposed refactor
chat, ok := decoded.Request.(*core.ChatRequest) if !ok { return nil, core.NewInvalidRequestError( fmt.Sprintf("requests[%d]: Anthropic Message Batch item is not a chat completion", i), nil, ) } - body, err := json.Marshal(adaptAnthropicCacheControl(chat, providerType)) - if err != nil { - return nil, core.NewInvalidRequestError(fmt.Sprintf("requests[%d]: failed to encode adapted chat request", i), err) + if !hasAnthropicCacheControl(chat) { + continue + } + body, err := json.Marshal(adaptAnthropicCacheControl(chat, providerType)) + if err != nil { + return nil, core.NewInvalidRequestError(fmt.Sprintf("requests[%d]: failed to encode adapted chat request", i), err) } adapted.Requests[i].Body = body🤖 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 `@internal/providers/cache_control.go` around lines 47 - 73, Update adaptAnthropicBatchCacheControl to check hasAnthropicCacheControl(chat) after decoding each batch item; when false, preserve item.Body directly in adapted.Requests[i].Body and skip adaptAnthropicCacheControl and json.Marshal. Continue adapting and marshaling only items that contain Anthropic cache control, while preserving the existing error handling.
🤖 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 `@docs/pro.mdx`:
- Around line 57-66: Align the documented compression settings with the
implementation: either remove unsupported variables from docs/pro.mdx or wire
each named PRO_COMPRESSION_* setting through the configuration parser and
request rewriter, including defaults, units, normalization, scope, and bounded
frozen-epoch behavior. Add parser/config coverage to verify these settings match
the rewriter’s thresholds and per-epoch compaction semantics.
In `@internal/providers/cache_planner.go`:
- Around line 456-471: Update promptCacheProfileFor to recognize the normalized
provider type "bedrock-mantle" and return the same Bedrock cache profile,
preserving the cached OpenAI responses behavior and
supportsExplicitOpenAICache("gpt-5.6") semantics.
- Around line 428-454: Update the Anthropic branch of providerCacheMinimum to
match current model-generation requirements using exact platform model-ID
substring patterns: return 4096 for Haiku 4.5 and Opus 4.5/4.6, preserve the
appropriate 2048 minimum for Haiku generations that require it, and retain
existing 1024 behavior for older supported models. Ensure the conditions do not
classify Haiku 3 or 3.5 as 4096.
In `@internal/providers/gemini/gemini.go`:
- Around line 593-599: Update the cached-content request construction around
geminiCreateCachedContentRequest to derive the upstream TTL from geminiCacheTTL
instead of hardcoding "300s", converting the configured duration to the
provider’s seconds-formatted string with strconv as needed. Keep the upstream
TTL aligned with the local expiry fallback used later in the flow.
- Around line 679-684: Update planChat’s PromptCachePlan.Key derivation to
include a digest of the final content item in the cached prefix, matching the
boundary selected by useGeminiCachedPrefix. Preserve the existing Messages,
Tools, and credential components while ensuring requests with different boundary
content produce distinct cache keys.
- Around line 585-620: Re-read the clock immediately after the /cachedContents
call in the cached-content creation flow, before handling the result. Use this
post-call timestamp for the failure retryAfter values, the expiration freshness
check, and the stored successful geminiCacheObject; retain the initial timestamp
only for request setup and pre-call behavior.
In `@internal/responsecache/exact_cache_test.go`:
- Around line 38-43: The blockingMissExchange.Context method relies on
StoreAfter invoking ex.Context twice before the follower wait point. Add a short
comment immediately above Context documenting this coupling and noting that
changes to StoreAfter’s call count require updating the helper.
---
Outside diff comments:
In `@internal/providers/cache_control.go`:
- Around line 47-73: Update adaptAnthropicBatchCacheControl to check
hasAnthropicCacheControl(chat) after decoding each batch item; when false,
preserve item.Body directly in adapted.Requests[i].Body and skip
adaptAnthropicCacheControl and json.Marshal. Continue adapting and marshaling
only items that contain Anthropic cache control, while preserving the existing
error handling.
In `@internal/responsecache/simple.go`:
- Around line 104-146: Update the leader branch of
simpleCacheMiddleware.StoreAfter so finishMiss always executes when ex.Capture
or next panics, ensuring the miss entry is removed and call.done is closed. Add
panic-safe cleanup around the capture/write flow, then re-panic with the
original value after finishMiss so callers and followers observe the existing
failure signal.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c143831-3e04-4646-b1d8-ce71f269dd6e
📒 Files selected for processing (19)
.env.templatedocs/features/cache.mdxdocs/pro.mdxdocs/providers/gemini.mdxinternal/core/types.gointernal/providers/bedrock/bedrock_test.gointernal/providers/bedrock/chat.gointernal/providers/bedrock/chat_stream.gointernal/providers/cache_control.gointernal/providers/cache_planner.gointernal/providers/cache_planner_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/keyring.gointernal/providers/keyring_test.gointernal/providers/router.gointernal/responsecache/exact_cache_test.gointernal/responsecache/simple.gointernal/responsecache/stream_cache.go
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
go test ./...go test -race ./internal/responsecache ./internal/providers/gemini ./internal/providers/bedrockgo test ./tests/perf -run TestHotPathPerfGuard -count=1Stack
Depends on #622.
Summary by CodeRabbit
New Features
Improvements