Repository navigation
fix(caching): skip the cache past max_messages and keep tool_result text in semantic prompts - #43878
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…rompts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…in complexity budget Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
bugbot run |
|
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…nswers Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
… text cannot forge result tags Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…rompt parity case Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…ache prompt helper Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…he proxy Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…che prompts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…er under complexity limit Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…eep walker under complexity limit" This reverts commit 0665296.
…f splitting it Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
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 580a970. Configure here.
…without the message cap (#45350) * test(integration): run the response cache tool-call tests on a proxy without the message cap Since #43878 the response cache skips any request past 4 messages by default, so the three providers-lane tests that post a seven-message tool conversation twice never saw a cache hit. They now boot one owned proxy from the lane config with max_messages set to null and keep their assertions; the shared proxy and the caching-lane cap test are unchanged * test(integration): give the uncapped-proxy cache tests the owned-proxy timeout budget The providers lane runs pytest with a 90 second per-test timeout that includes fixture setup, and the first of the three tests to run pays the owned proxy boot before the spend-log test can poll for up to 70 seconds. The sibling owned-proxy tests already carry pytest.mark.timeout(240), so these three take the same budget --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
TLDR
Problem this solves:
tool_resultblock text was dropped from the embedded prompt, so turn 2 embedded byte-identical to turn 1function_call_outputwhoseoutputis a list ofinput_textblocks contributed nothing to the embedded prompt, only the string form didHow it solves it:
cache_params.max_messages(default 4): a request with moremessages, or more Responses APIinputitems, than this is neither looked up nor stored.Cache.should_use_cachereturns False, which is the shared gate for every backend, exact and semantic, Python and the Rust native cacheget_semantic_cache_prompt_from_messagesincommon_utils.pyisget_str_from_messagesplus the text inside Messages APItool_resultblocks. Redis, Valkey and Qdrant semantic caches embed that, and the Rust semantic cache mirrors itredis_semantic_cache.pyrecurses into a list-formoutputthe same way it does intocontent, so block-form tool outputs embed like string ones. The Rust walker mirrors itconvert_content_list_to_strand the provider-facing helpers are unchangedA message cap was chosen over encoding tool calls into the embedded text: it is one shared check, it also stops long agent histories from filling the store, and it holds for every backend
User Flow
Before: an agent loop with semantic caching on gets the same cached answer for every tool turn
tool_useforwrite_filetool_useand atool_resultsayingPermissionError(3 messages)tool_useback from the cache, same tool id, same cost header, so the agent writes the file againtool_useback, foreverAfter: only short conversations are cached, and a tool turn no longer embeds like the turn before it
tool_useforwrite_file; sending the same request again is a cache hittool_useand atool_resultsayingPermissionError(3 messages, under the cap, so the cache is checked)The same cap applies to /v1/chat/completions (each
tool_callsmessage and eachrole: "tool"message counts) and to /v1/responses (eachinputitem counts). Exact caching stops caching past 4 messages tooLinear ticket
Resolves LIT-8373
Pre-Submission checklist
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Tests:
tests/integration/caching/test_cache_max_messages.pyruns against the shared integration proxy (cache: redis) and its scripted provider: for each of /v1/messages, /v1/chat/completions and /v1/responses it sends turn 1 of an agent session twice (1 or 2 messages, the repeat is served from the cache) and turn 3 twice (5 or 6 messages, both reach the provider), so the provider sees exactly 3 of the 4 requests. Removing the cap fails it on the past-cap assertion.tests/unit/caching/test_caching.pycoversshould_use_cacheat 4, 5, Responsesinputcounting, embedding string inputs, a custommax_messages, andNonedisabling the cap.test_litellm_core_utils_prompt_templates_common_utils.pyandtests/unit/caching/test_redis_semantic_cache.pypin the embedded text for string and blocktool_result,tests/unit/caching/test_redis_semantic_cache.pyalso pins a block-formfunction_call_output, andlitellm-rust/crates/cache/tests/semantic.rspins the Rust side of both. Mutations checked: changing>to>=inshould_use_cache, dropping thetool_resultbranch, dropping the list-formoutputbranch, and countinginputstrings each fail a testScreenshots / Proof of Fix
Setup: proxy with
cache: true,type: redis-semanticon Redis Stack,similarity_threshold: 0.95,text-embedding-3-small, real Anthropic (claude-haiku-4-5), Redis flushed before each leg. Five requests to /v1/messages: turn 1 (1 message), turn 1 again, turn 2 (3 messages:tool_useplustool_resultsayingPermissionError), turn 3 (5 messages: a secondtool_useplus atool_resultsaying the write succeeded), turn 3 again. The responseidand thetool_useid show whether Anthropic answered or the cache didBefore (merge base d7e7f40): one Anthropic call, then the cache answers everything, including the 3 and 5 message tool turns
After (394abff): the repeat of turn 1 is the only cache hit; the tool turns reach Anthropic, and the 5 message turn is never stored, so its repeat reaches Anthropic too
Type
🐛 Bug Fix
✅ Test
Caveats
A 3 or 4 message tool turn is still under the cap, so it is still looked up. Its embedded text now differs from turn 1 by the tool result text, and in the live run above it missed at
similarity_threshold: 0.95, but a very short tool output with a lower threshold could still score above it and replay the turn 1 answer once. Turns with 5 or more messages cannotTool call names and arguments are not part of the embedded text, as on main. Two short turns that differ only in the command they ran, with the same tool output, embed the same and can share an answer. Encoding tool calls was considered and set aside for the cap
Exact caching changes too: a plain chat with 5 or more messages that hit the exact cache before no longer does. Set
cache_params.max_messageshigher, or tonullto turn the cap offExisting semantic entries for /v1/messages tool turns were embedded without the
tool_resulttext, so after upgrade those turns miss once and are re-stored. Requests withouttool_resultblocks embed exactly as beforeFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/ed7571b353934c67b185cf3b2b06ec77
Open in Devin Desktop: https://app.devin.ai/desktop/session/ed7571b353934c67b185cf3b2b06ec77?variant=devin
Requested by: @kerry-berri
Note
Medium Risk
Changes default caching behavior for conversations with more than four messages and alters semantic-cache keys for tool turns, which can cause one-time misses and different hit rates after upgrade.
Overview
Fixes agent loops getting stale semantic cache hits by capping which conversations are cached and by embedding tool-turn text that was previously dropped.
max_messages(default 4) onCacheturns off lookup and storage when a request has more chatmessagesor Responses APIinputitems (mapping/model objects only; embedding string lists are not counted).should_use_cacheenforces this before mode/opt-in, so exact and semantic backends all skip long histories. Setmax_messages=Noneto disable the cap.Semantic embedding text now uses
get_semantic_cache_prompt_from_messages(Python) and matching Rust logic: Messages APItool_resultcontent (string or nested text blocks) is included so a tool turn does not embed like the prior turn. Responses prompt collection also recurses into list-shapedoutputon items such asfunction_call_output, so block-form tool outputs contribute text like stringoutputdid.Redis, Valkey, and Qdrant semantic caches call the new helper;
get_str_from_messagesis unchanged for non-cache paths.Reviewed by Cursor Bugbot for commit 580a970. Bugbot is set up for automated code reviews on this repo. Configure here.