fix(bedrock): preserve cache_control ttl on message-level cache points - #32538
Conversation
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a bug where message-level Bedrock cache breakpoints silently dropped the
Confidence Score: 5/5Safe to merge — the change is a targeted one-argument fix at 8 symmetric call sites with direct regression test coverage on both the sync and async paths. The fix is minimal and mechanical: passing No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/prompt_templates/factory.py | Adds model=model to all 8 message-level _get_cache_point_block call sites across sync and async paths; no logic changes, just passes the missing argument through. |
| tests/test_litellm/litellm_core_utils/prompt_templates/test_litellm_core_utils_prompt_templates_factory.py | Adds new parametrized sync and async regression tests verifying that message-level cache points retain ttl="1h"; tests are pure unit tests with no real network calls. |
Reviews (1): Last reviewed commit: "fix(bedrock): preserve cache_control ttl..." | Re-trigger Greptile
Merging this PR will improve performance by 24.36%
Performance Changes
Tip Curious why this is faster? Comment Comparing |
BerriAI#32538) Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Relevant issues
Fixes #32154
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Two proxies running the same config (
bedrock/eu.anthropic.claude-opus-4-8, eu-central-1), before on port 4001 atbfff5e8d86, after on port 4000 at7195c2dfa2. Payload putscache_control: {"type": "ephemeral", "ttl": "1h"}on both the system prompt and a large conversation-history user message. Turn 2 is sent 7 minutes after turn 1 so anything that only got the 5m default has expired while 1h caches surviveReal Bedrock calls, no mocks:
Type
🐛 Bug Fix
Changes
AmazonConverseConfig._get_cache_point_blockonly preserves thettlfromcache_controlwhen it receives amodelthat supports extended caching; withmodel=Nonethe ttl is silently stripped and Bedrock falls back to the 5m default. The system-prompt and tools paths already passmodel, but none of the 8 message-level call sites in_bedrock_converse_messages_pt/_bedrock_converse_messages_pt_asyncdid, so message-level cache breakpoints (the ones that cache conversation history) always lost their requestedttl: "1h"This produced mixed TTLs on models that support 1h caching (Opus 4.8 etc after the JSON-driven gate landed in #31929): the system prompt got 1h while conversation-history breakpoints silently got 5m, so multi-turn agent workloads re-created the history prefix every turn;
cache_read_input_tokensfroze at the system-prefix size whilecache_creation_input_tokensgrew, driving large cost increasesThe fix passes
modelthrough at all 8 message-level call sites. Regression tests assert the message-level cachePoint keepsttl: "1h"on both the sync and async paths; they fail on the base commit and pass with this changeLink to Devin session: https://app.devin.ai/sessions/2cd3570c8b8a4986bec41d701aa44a87
Requested by: @ishaan-berri