Skip to content

Commit 8a96b7b

Browse files
nyxst4ckGWeale
authored andcommitted
fix(models): gate Gemini cache creation on cacheable prefix tokens
Merge google#6137 ### Link to Issue or Description of Change - Closes: google#5847 **Problem:** `GeminiContextCacheManager._create_new_cache_with_contents` decides whether a request is large enough to cache by comparing `llm_request.cacheable_contents_token_count` against Gemini's `_GEMINI_MIN_CACHE_TOKENS` (4096). But `cacheable_contents_token_count` is set (in `context_cache_processor.py`) to the **previous response's `prompt_token_count`** — i.e. the token count of the *entire* previous prompt (system instruction + tools + every content, including the trailing user turn). The cache, however, only stores the **prefix** `contents[:cache_contents_count]` (plus system instruction and tools — see `_create_gemini_cache`), where `cache_contents_count` excludes the last continuous batch of user contents. On a long conversation the full-prompt count can clear the 4096 gate while the actual cached prefix is far below it, so `caches.create()` is invoked with a sub-minimum payload and Gemini returns **`400 INVALID_ARGUMENT`**. The reporter confirmed this in production, and a maintainer confirmed the root cause in the issue thread. **Solution:** Gate cache creation on the size of the prefix that is *actually* cached rather than on the full previous prompt. Since the only accurate token count we have is for the whole prompt, the new `_estimate_cacheable_prefix_tokens` scales that accurate count by the prefix's estimated share of the request (`_estimate_request_tokens` is reused — now accepting an optional `cache_contents_count` — to estimate both the full request and the prefix). When the prefix already spans the whole request the scale factor is `1.0` and the accurate count is used unchanged, so existing behavior is preserved; when the prefix is a strict subset the estimate shrinks accordingly and a too-small prefix is correctly skipped before any API call. Scope note: this fixes the `400 INVALID_ARGUMENT` (the reported bug). The separate latency concern discussed later in the thread (synchronous `caches.create()` / cleanup on the request path, tracked in google#5889) is intentionally out of scope. The user-configured `min_tokens` gate is left on the full-prompt count, since the 4096 prefix gate is what prevents the invalid API call; happy to extend `min_tokens` to the prefix too if maintainers prefer. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Added `test_create_cache_gates_on_prefix_not_full_prompt`: a tiny cacheable prefix followed by a huge trailing user turn with `cacheable_contents_token_count = 75000`. It asserts no cache is created and `caches.create` is never called. This test is **red on `main`** (`caches.create` is called once — the 400-inducing path) and **green with this change**. Updated `test_fingerprint_only_metadata_transitions_to_active_cache` to use a realistically large cacheable prefix, since its previous setup relied on the buggy assumption that a tiny prefix would still be cached because the injected full-prompt count cleared the gate. ``` $ pytest tests/unittests/agents/test_gemini_context_cache_manager.py \ tests/unittests/agents/test_context_cache_config.py \ tests/unittests/flows/llm_flows/test_context_cache_processor.py -q 59 passed ``` **Manual End-to-End (E2E) Tests:** The fix is exercised entirely through the unit test above (it mocks `genai_client.aio.caches.create` and asserts it is not called for a below-minimum prefix), which mirrors the production failure mode reported in google#5847 (`400 INVALID_ARGUMENT` from `caches.create` on a long conversation). ### Checklist - [x] I have read the CONTRIBUTING.md document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective. - [x] New and existing unit tests pass locally with my changes. Co-authored-by: George Weale <gweale@google.com> COPYBARA_INTEGRATE_REVIEW=google#6137 from nyxst4ck:fix/gemini-cache-prefix-token-gate ade7a1e PiperOrigin-RevId: 938659723
1 parent 58f4067 commit 8a96b7b

2 files changed

Lines changed: 109 additions & 10 deletions

File tree

src/google/adk/models/gemini_context_cache_manager.py

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,21 @@ async def _create_new_cache_with_contents(
326326
)
327327
return None
328328

329-
# Check client-side to avoid unnecessary API round-trips.
330-
if llm_request.cacheable_contents_token_count < _GEMINI_MIN_CACHE_TOKENS:
329+
# `cacheable_contents_token_count` is the token count of the whole previous
330+
# prompt (system instruction + tools + every content). The cache, however,
331+
# only stores the prefix `contents[:cache_contents_count]` plus the system
332+
# instruction and tools (see `_create_gemini_cache`). On a long conversation
333+
# the full-prompt count can clear Gemini's minimum while the cached prefix
334+
# is far smaller, which makes `caches.create` fail with 400
335+
# INVALID_ARGUMENT.
336+
# Gate on the estimated prefix size so we never send a sub-minimum payload.
337+
cacheable_prefix_tokens = self._estimate_cacheable_prefix_tokens(
338+
llm_request, cache_contents_count
339+
)
340+
if cacheable_prefix_tokens < _GEMINI_MIN_CACHE_TOKENS:
331341
logger.info(
332-
"Request below Gemini minimum cache size (%d < %d tokens)",
333-
llm_request.cacheable_contents_token_count,
342+
"Cacheable prefix below Gemini minimum cache size (%d < %d tokens)",
343+
cacheable_prefix_tokens,
334344
_GEMINI_MIN_CACHE_TOKENS,
335345
)
336346
return None
@@ -342,13 +352,20 @@ async def _create_new_cache_with_contents(
342352
logger.warning("Failed to create cache: %s", e)
343353
return None
344354

345-
def _estimate_request_tokens(self, llm_request: LlmRequest) -> int:
346-
"""Estimate token count for the request.
355+
def _estimate_request_tokens(
356+
self,
357+
llm_request: LlmRequest,
358+
cache_contents_count: Optional[int] = None,
359+
) -> int:
360+
"""Estimate token count for the request (or its cacheable prefix).
347361
348362
This is a rough estimation based on content text length.
349363
350364
Args:
351365
llm_request: Request to estimate tokens for
366+
cache_contents_count: When provided, only the first
367+
``cache_contents_count`` contents are counted (the prefix that gets
368+
cached); the system instruction and tools are always included.
352369
353370
Returns:
354371
Estimated token count
@@ -366,15 +383,54 @@ def _estimate_request_tokens(self, llm_request: LlmRequest) -> int:
366383
tool_str = json.dumps(tool.model_dump())
367384
total_chars += len(tool_str)
368385

369-
# Contents
370-
for content in llm_request.contents:
386+
# Contents (optionally limited to the cacheable prefix)
387+
contents = llm_request.contents
388+
if cache_contents_count is not None:
389+
contents = contents[:cache_contents_count]
390+
for content in contents:
371391
for part in content.parts:
372392
if part.text:
373393
total_chars += len(part.text)
374394

375395
# Rough estimate: 4 characters per token
376396
return total_chars // 4
377397

398+
def _estimate_cacheable_prefix_tokens(
399+
self, llm_request: LlmRequest, cache_contents_count: int
400+
) -> int:
401+
"""Estimate the token count of the prefix that will actually be cached.
402+
403+
The only accurate token count available is
404+
``cacheable_contents_token_count``, which covers the entire previous prompt.
405+
Since the cache stores just the prefix ``contents[:cache_contents_count]``
406+
(plus system instruction and tools), we scale that accurate count by the
407+
prefix's estimated share of the request. When the prefix already spans the
408+
whole request the scale factor is 1 and the accurate count is returned
409+
unchanged.
410+
411+
Args:
412+
llm_request: Request to estimate the cacheable prefix tokens for
413+
cache_contents_count: Number of leading contents that get cached
414+
415+
Returns:
416+
Estimated token count of the cacheable prefix
417+
"""
418+
full_tokens = llm_request.cacheable_contents_token_count
419+
if not full_tokens:
420+
return 0
421+
422+
full_estimate = self._estimate_request_tokens(llm_request)
423+
if full_estimate <= 0:
424+
# No text to estimate from (e.g. non-text parts); fall back to the
425+
# accurate full count rather than incorrectly skipping the cache.
426+
return full_tokens
427+
428+
prefix_estimate = self._estimate_request_tokens(
429+
llm_request, cache_contents_count
430+
)
431+
ratio = min(1.0, prefix_estimate / full_estimate)
432+
return int(full_tokens * ratio)
433+
378434
async def _create_gemini_cache(
379435
self, llm_request: LlmRequest, cache_contents_count: int
380436
) -> CacheMetadata:

tests/unittests/agents/test_gemini_context_cache_manager.py

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,43 @@ async def test_handle_context_caching_invalid_cache_fingerprint_match(self):
202202
mock_cleanup.assert_called_once_with(existing_cache.cache_name)
203203
self.manager.genai_client.aio.caches.create.assert_called_once()
204204

205+
async def test_create_cache_gates_on_prefix_not_full_prompt(self):
206+
"""Cache creation is gated on the cacheable prefix, not the full prompt.
207+
208+
Regression test for https://github.com/google/adk-python/issues/5847.
209+
210+
On a long conversation the previous-prompt token count
211+
(``cacheable_contents_token_count``) can be well above Gemini's 4096-token
212+
minimum while the cached prefix ``contents[:cache_contents_count]`` is far
213+
below it. Creating a cache in that case makes ``caches.create`` fail with a
214+
400 INVALID_ARGUMENT. The manager must skip cache creation instead.
215+
"""
216+
self.manager.genai_client.aio.caches.create = AsyncMock()
217+
218+
# A tiny cacheable prefix followed by a huge trailing user turn.
219+
contents = [
220+
types.Content(role="user", parts=[types.Part(text="Short prefix.")]),
221+
types.Content(role="user", parts=[types.Part(text="word " * 100_000)]),
222+
]
223+
llm_request = LlmRequest(
224+
model="gemini-2.5-flash",
225+
contents=contents,
226+
config=types.GenerateContentConfig(
227+
system_instruction="You are a helpful assistant.",
228+
),
229+
cache_config=self.cache_config,
230+
)
231+
# Full previous prompt is large (clears the old, buggy gate)...
232+
llm_request.cacheable_contents_token_count = 75000
233+
234+
# ...but only the tiny first content is cacheable.
235+
result = await self.manager._create_new_cache_with_contents(
236+
llm_request, cache_contents_count=1
237+
)
238+
239+
assert result is None
240+
self.manager.genai_client.aio.caches.create.assert_not_called()
241+
205242
async def test_handle_context_caching_invalid_cache_fingerprint_mismatch(
206243
self,
207244
):
@@ -916,7 +953,10 @@ async def test_fingerprint_only_metadata_transitions_to_active_cache(
916953
llm_request_2 = self.create_llm_request(
917954
cache_metadata=result_1, contents_count=5
918955
)
919-
llm_request_2.cacheable_contents_token_count = 4096
956+
# contents_count is 0 (all-user conversation), so the cached prefix is the
957+
# system instruction + tools; use a large previous-prompt count so the
958+
# estimated prefix clears Gemini's 4096-token minimum.
959+
llm_request_2.cacheable_contents_token_count = 30000
920960

921961
# Verify prefix fingerprint matches (real implementation).
922962
# The fingerprint-only metadata is "invalid" (no cache_name),
@@ -997,7 +1037,10 @@ async def test_dynamic_instruction_does_not_break_initial_cache_fingerprint(
9971037
dynamic_instruction,
9981038
tool_response,
9991039
]
1000-
request_2.cacheable_contents_token_count = 4096
1040+
# contents_count is 0, so the cached prefix is the system instruction +
1041+
# tools; use a large previous-prompt count so the estimated prefix clears
1042+
# Gemini's 4096-token minimum.
1043+
request_2.cacheable_contents_token_count = 30000
10011044

10021045
mock_cached_content = AsyncMock()
10031046
mock_cached_content.name = (

0 commit comments

Comments
 (0)