fix(bedrock): honor cache_control ttl on message-level cachePoint blocks - #32551
Conversation
Bedrock Converse supports cachePoint ttl (1h GA for Claude 4.5+), and _get_cache_point_block maps cache_control.ttl -> cachePoint.ttl, but the model parameter its allow-list gate requires was only threaded through the system-message path. Every message-level path either called _get_cache_point_block without model= (8 call sites in _bedrock_converse_messages_pt / _pt_async) or hardcoded CachePointBlock(type="default") (tool-result blocks and _convert_to_bedrock_tool_call_invoke), so a requested 1h ttl silently degraded to the 5-minute default - exactly on the conversation-tail breakpoint that long-running agents need to survive tool calls longer than 5 minutes. - pass model= at the 8 _get_cache_point_block call sites - tool-result blocks: capture the cache_control dict (was a boolean) and route through _get_cache_point_block so ttl survives - _convert_to_bedrock_tool_call_invoke: accept optional model and route per-tool-call cache_control through _get_cache_point_block Completes the ttl support added for system messages (#19848, #20326): message-level cache_control now behaves identically. Note: message-level cache_control on a content-less assistant message emits no cachePoint at all today; that pre-existing gap is orthogonal to ttl and left out of scope (per-tool-call placement covers it).
|
Generated by Claude Code |
Greptile SummaryThis PR fixes the Bedrock prompt-caching
Confidence Score: 5/5The change is a focused, backward-compatible bug fix with symmetric treatment of sync and async paths and comprehensive regression tests. No network calls, no new dependencies, no structural changes. Both affected code paths (tool-result blocks and per-tool-call cache_control) are fixed symmetrically and tested. The model-capability check reuses the existing is_claude_4_5_on_bedrock helper backed by model_prices_and_context_window.json, so no model names are hardcoded. The test model global.anthropic.claude-opus-4-7 correctly has cache_creation_input_token_cost_above_1hr in the pricing table, confirming the parametrized assertions will behave as expected. No correctness or compatibility issues were found. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/prompt_templates/factory.py | Adds optional model parameter to _convert_to_bedrock_tool_call_invoke and threads it through to _get_cache_point_block at all four affected call sites; replaces the hard-coded CachePointBlock(type="default") pattern with the shared helper so ttl survives for extended-caching models. Sync and async tool-result paths receive symmetric treatment. |
| tests/test_litellm/llms/bedrock/chat/test_converse_transformation.py | Adds two parametrized regression tests covering user-level, per-tool-call, and tool-result cache_control placements for both a supported model (global.anthropic.claude-opus-4-7, which has cache_creation_input_token_cost_above_1hr in the pricing table) and an unsupported model. The async test also asserts sync/async parity. No real network calls are made. |
Reviews (1): Last reviewed commit: "fix(bedrock): honor cache_control ttl on..." | Re-trigger Greptile
|
bugbot run Generated by Claude Code |
Greptile SummaryThis PR fixes the silent TTL downgrade that occurred when
Confidence Score: 5/5The change is narrowly scoped to four previously-broken cache-point emission paths; unsupported models continue to emit the plain default block, and supported models now correctly forward the ttl value. All six affected call sites are updated symmetrically in both the sync and async paths. The wrapping pattern used to call _get_cache_point_block is consistent with the dozen pre-existing call sites in the same file. The deduplication step cannot accidentally strip the new cache-point blocks because it only deduplicates on toolUseId. The two new tests cover all three placement targets and assert sync/async parity. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/prompt_templates/factory.py | Fixes four call sites that hardcoded CachePointBlock(type="default") to instead route through _get_cache_point_block so ttl survives; model is now threaded into _convert_to_bedrock_tool_call_invoke and the tool-result blocks in both sync and async paths. |
| tests/test_litellm/llms/bedrock/chat/test_converse_transformation.py | Adds two new parametrized tests covering all three placement targets (user, tool_call, tool) on a supported model (ttl=1h must survive) and an unsupported model (plain default block must be emitted); sync and async outputs are asserted equal. |
Reviews (2): Last reviewed commit: "fix(bedrock): honor cache_control ttl on..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit adb6e5a. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 11.45%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
|
…cks (BerriAI#32551) Bedrock Converse supports cachePoint ttl (1h GA for Claude 4.5+), and _get_cache_point_block maps cache_control.ttl -> cachePoint.ttl, but the model parameter its allow-list gate requires was only threaded through the system-message path. Every message-level path either called _get_cache_point_block without model= (8 call sites in _bedrock_converse_messages_pt / _pt_async) or hardcoded CachePointBlock(type="default") (tool-result blocks and _convert_to_bedrock_tool_call_invoke), so a requested 1h ttl silently degraded to the 5-minute default - exactly on the conversation-tail breakpoint that long-running agents need to survive tool calls longer than 5 minutes. - pass model= at the 8 _get_cache_point_block call sites - tool-result blocks: capture the cache_control dict (was a boolean) and route through _get_cache_point_block so ttl survives - _convert_to_bedrock_tool_call_invoke: accept optional model and route per-tool-call cache_control through _get_cache_point_block Completes the ttl support added for system messages (BerriAI#19848, BerriAI#20326): message-level cache_control now behaves identically. Note: message-level cache_control on a content-less assistant message emits no cachePoint at all today; that pre-existing gap is orthogonal to ttl and left out of scope (per-tool-call placement covers it). Co-authored-by: Arash <arashne@glia-ai.com>
3 participants
Relevant issues
Completes the fix for #32154. Copy of #32155 by @arashne, cherry-picked onto litellm_internal_staging so CircleCI can run on it; the code and tests are their work and the commit keeps the original authorship
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)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
See #32155 for the original repro (a message-level
cache_controlwithttl: "1h"silently emittingcachePoint: {"type": "default"}) and #32538 for live Bedrock usage output captured atbfff5e8d86(before) and7195c2dfa2(after), showing the full prefix served from the 1h cache on turn 2 after a 7 minute wait oncemodelreaches the cachePoint builder. The paths completed here (tool-result blocks and per-tool-callcache_control) fail the same way on current staging: they hardcodeCachePointBlock(type="default")without ever consulting_get_cache_point_block, so a requested ttl is dropped. After this change they emit{"type": "default", "ttl": "1h"}on supported models and keep emitting the plain default block on unsupported onesType
🐛 Bug Fix
Changes
Copy of #32155, authored by @arashne, so CircleCI can run on it
Since the original PR was opened, #32538 landed on litellm_internal_staging and covered part of the same ground: it passes
model=at the 8 existing_get_cache_point_blockcall sites in_bedrock_converse_messages_pt/_bedrock_converse_messages_pt_async. This PR carries the remaining paths from #32155, the ones that never reached_get_cache_point_blockat all because they hardcodedCachePointBlock(type="default"): tool-result blocks now capture thecache_controldict (previously reduced to a boolean) and route it through_get_cache_point_blockso ttl survives, and_convert_to_bedrock_tool_call_invokenow accepts an optionalmodeland routes per-tool-callcache_controlthe same way, with both callers passingmodel=modelThe regression tests from the original PR are parameterized over user, tool-call, and tool-result placements crossed with a supported and an unsupported model, and assert the sync and async paths emit identical blocks
Note
Medium Risk
Changes Bedrock prompt shaping for cached agentic/tool loops; wrong TTL handling could affect cache behavior and API acceptance, but scope is limited to cache-point emission with new tests.
Overview
Bedrock Converse message conversion no longer hardcodes
cachePoint: {"type": "default"}for tool results and assistant tool-callcache_control; those paths now build cache points viaAmazonConverseConfig()._get_cache_point_block(..., model=model)so a requestedttl(e.g.1h) is preserved on models that support extended caching and omitted on unsupported ones._convert_to_bedrock_tool_call_invokegains an optionalmodelargument and both sync/async Converse processors pass it through when converting tool calls. Tool-message handling now keeps the actualcache_controldict (message- or content-level) instead of a boolean before emitting the cache block.Regression tests cover user, per-tool-call, and tool-result placements for supported vs unsupported models, including sync/async parity.
Reviewed by Cursor Bugbot for commit adb6e5a. Bugbot is set up for automated code reviews on this repo. Configure here.