fix(bedrock): honor cache_control ttl on message-level cachePoint blocks - #32155
fix(bedrock): honor cache_control ttl on message-level cachePoint blocks#32155arashne wants to merge 1 commit into
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).
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5Safe to merge. The change is a targeted, additive fix that threads an existing parameter to call sites that were already present; it does not alter any API surface, add new branching logic, or touch any auth/security paths. The refactoring is minimal and correct: the tool-result boolean flag is replaced by capturing the actual dict (no behavioral change when no TTL is set, correct behavior when TTL is set), and the model argument is simply plumbed through to existing helper calls. The is_claude_4_5_on_bedrock gate is driven by model_prices_and_context_window.json, so newly-supported models will pick it up automatically without code changes. The new tests cover all three placement types and both sync/async paths, and make no real network calls. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/prompt_templates/factory.py | All eight _get_cache_point_block call sites in the message and tool-call paths now receive model=model; tool-result path now captures the actual cache_control dict instead of a boolean, preserving the TTL value correctly. |
| tests/test_litellm/llms/bedrock/chat/test_converse_transformation.py | New parametrized tests for supported/unsupported model TTL behavior across all three placement types (user, tool_call, tool-result); pure transformation tests with no real network calls. |
Reviews (1): Last reviewed commit: "fix(bedrock): honor cache_control ttl on..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a silent TTL downgrade in Bedrock Converse prompt caching: message-level and per-tool-call
Confidence Score: 5/5Safe to merge — the change is a mechanical parameter thread-through with no new logic, and the allow-list gate already uses model_prices_and_context_window.json rather than hardcoded patterns. All eight call sites are updated consistently across both sync and async paths. The tool-result boolean-to-dict refactor preserves existing behavior for non-TTL cache_control while correctly forwarding TTL when present. The new tests verify both the happy path and the guard, and confirm sync/async parity. No existing tests are weakened or removed. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/prompt_templates/factory.py | Threads model parameter through all 8 _get_cache_point_block call sites in the Bedrock Converse message transformation paths (sync + async), and captures the actual cache_control dict (instead of a boolean sentinel) for tool-result blocks so TTL survives into the cachePoint block |
| tests/test_litellm/llms/bedrock/chat/test_converse_transformation.py | Adds parameterized tests (user / tool_call / tool placements × supported / unsupported models) covering the sync and async transformation paths; pure in-memory tests with no network calls |
Reviews (2): Last reviewed commit: "fix(bedrock): honor cache_control ttl on..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Gentle nudge on this one, and some fresh precedent for it. Since I opened this, #31929 ("honor ttl for tool_config cache injection points") merged and shipped in 1.90.3 / 1.91.0. That threads I re-checked against v1.91.0: all eight message-path call sites still omit CI is green (Codecov reports full coverage on the changed lines), Greptile scored it 5/5, and the CLA is signed. @mateo-berri, since you merged the sibling tool_config fix, would you be up for reviewing this one? |
|
Landed upstream as #32551 ( |
Relevant issues
Fixes #32154. Completes the ttl support added for system messages in #20338 (#19848, #20326).
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer review (received 5/5: "Safe to merge")Screenshots / Proof of Fix
Repro before the fix (v1.90.0 and main): a message-level
cache_controlwithttl: "1h"on a supported model emitscachePoint: {"type": "default"}— the ttl is silently dropped (see #32154 for the snippet). After the fix, the same input emits{"type": "default", "ttl": "1h"}for user content, tool-result, and per-tool-call placements; unsupported models keep emitting the plain default block.E2e proof with real Bedrock calls (cacheWriteInputTokens billed at the 1h rate for
global.anthropic.claude-opus-4-7) to follow as a comment — we run this patch in production via a sitecustomize overlay and will attach the live usage output.Adjacent suites (
test_converse_transformation.py,test_litellm_core_utils_prompt_templates_factory.py,test_anthropic_cache_control_hook.py) have byte-identical failure sets on base and patched (pre-existing env failures only): 241 passed → 247 passed, no new failures.Type
🐛 Bug Fix
Changes
Bedrock Converse supports
cachePoint.ttland_get_cache_point_blockmapscache_control.ttl→cachePoint.ttl, but themodelargument its allow-list gate requires was only threaded through the system-message path — every message-level path silently degraded a requested 1h to the 5-minute default, exactly on the conversation-tail breakpoint that long-running agents set (cache_control_injection_pointswithindex: -1) to survive tool calls longer than 5 minutes.factory.py: passmodel=at the 8_get_cache_point_blockcall sites in_bedrock_converse_messages_pt/_pt_async.cache_controldict (previously reduced to a boolean) and route through_get_cache_point_blockso ttl survives._convert_to_bedrock_tool_call_invoke: accept optionalmodeland route per-tool-callcache_controlthrough_get_cache_point_block; both callers passmodel=model.opus-4-7) and unsupported (claude-3-5-sonnet) models, sync and async paths asserted equal.Out of scope (pre-existing, orthogonal): message-level
cache_controlon a content-less assistant message emits no cachePoint at all; per-tool-call placement covers that message.