feat(llm): retry without prompt caching when cached content is below provider minimum - #3480
Conversation
Vertex AI (Gemini) requires a minimum number of tokens (currently 4096) to create a context cache. When the cached content is below this threshold, the API returns a 400 BadRequestError. Previously this would bubble up as a hard failure. This change introduces a 'prompt cache too small' classifier and adds retry-without-caching logic to all four LLM entrypoints (sync/async completion + responses). When the error is detected and caching is active, the LLM transparently retries with caching_prompt=False, preserving the original caller kwargs. The new is_prompt_cache_too_small classifier matches the canonical Vertex AI error string 'minimum token count to start caching'. Also threads the caller's kwargs through the existing fallback path so behavior matches between the no-cache retry and provider-level fallback. Tests: - tests/sdk/llm/test_exception_classifier.py: classifier coverage - tests/sdk/llm/test_llm_completion.py: sync/async retry behavior - tests/sdk/llm/test_responses_parsing_and_kwargs.py: responses path Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED Behavioral default changes detectedThese public
|
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Coverage Report •
|
||||||||||||||||||||||||||||||
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
⚠️ QA Report: PASS WITH ISSUES
The prompt-cache-too-small retry behavior works in the SDK paths I exercised, but CI was not clean at review time.
Does this PR achieve its stated goal?
Yes. On the base branch, the Vertex-style BadRequestError bubbled as LLMBadRequestError after one LLM.completion() attempt; on the PR commit, the same SDK call retried once, removed prompt cache markers, preserved caller kwargs, and returned the successful response. I also exercised the async completion path and both sync/async Responses API paths; all retried successfully and preserved caller kwargs. A live Vertex AI call could not be completed in this environment because Google ADC credentials are not configured, so the provider interaction was verified with a provider-equivalent LiteLLM exception injected into the real SDK call path.
| Phase | Result |
|---|---|
| Environment Setup | ✅ make build completed and installed the uv workspace dependencies. |
| CI Status | sdk-tests), 8 pending, 1 skipped. |
| Functional Verification | ✅ SDK retry behavior verified for completion, acompletion, responses, and aresponses; unrelated bad requests still did not retry. |
Functional Verification
Test 1: Live Vertex-style SDK call attempt
Step 1 — Attempt real execution first:
Ran a real LLM(model="vertex_ai/gemini-2.5-flash", caching_prompt=True).completion(...) call.
Observed excerpt:
Failed to load vertex credentials...
google.auth.exceptions.DefaultCredentialsError: Your default credentials were not found.
LLMServiceUnavailableError
This shows the environment cannot complete a live Vertex AI request because Application Default Credentials are missing.
Test 2: Before/after completion retry behavior
Step 1 — Establish baseline without the fix:
Checked out origin/main and ran the same SDK LLM.completion() scenario with LiteLLM raising the canonical Vertex error string.
Observed excerpt:
raised_type= LLMBadRequestError
raised_message= litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096."}}
call_count= 1
This confirms the prior behavior: the first cache-too-small error ended the user call without retrying.
Step 2 — Apply the PR's changes:
Checked out PR commit c46b9c9a9cee9ce598db265a89474e6443881cc2.
Step 3 — Re-run with the fix in place:
Ran the same SDK scenario.
Observed excerpt:
Prompt cache content too small for provider minimum, retrying without prompt caching
completion_text= Retry succeeded
completion_call_count= 2
completion_first_cache= True
completion_second_cache= False
completion_second_metadata= {'trace': 'qa-completion'}
This shows the PR delivers the intended behavior for completion(): the first call used cache markers, the retry removed them, caller kwargs were preserved, and the user received the successful response.
Test 3: Responses API retry behavior
Ran the same provider-equivalent cache-too-small scenario through LLM.responses().
Observed excerpt:
Prompt cache content too small for provider minimum, retrying without prompt caching
responses_raw_id= resp-qa
responses_call_count= 2
responses_second_store= False
responses_second_metadata= {'trace': 'qa-responses'}
This shows the Responses API path also retries once and preserves both named and arbitrary caller kwargs.
Test 4: Async retry behavior
Ran equivalent scenarios through LLM.acompletion() and LLM.aresponses().
Observed excerpt:
acompletion_text= Async retry succeeded
acompletion_call_count= 2
acompletion_first_cache= True
acompletion_second_cache= False
acompletion_second_metadata= {'trace': 'qa-async-completion'}
aresponses_raw_id= aresp-qa
aresponses_call_count= 2
aresponses_second_store= False
aresponses_second_metadata= {'trace': 'qa-async-responses'}
This verifies the async paths behave consistently with the sync paths.
Test 5: Related error behavior remains unchanged
Ran LLM.completion() with an unrelated context-window BadRequestError.
Observed excerpt:
raised_type= LLMContextWindowExceedError
call_count= 1
This confirms the new retry did not broadly retry unrelated provider bad requests.
Unable to Verify
A live Vertex AI cache-too-small request could not be verified because this QA environment lacks Google Application Default Credentials. Future QA runs would benefit from AGENTS.md guidance for a non-production Vertex project/model and credential setup that can safely trigger a sub-4096-token context-cache request.
Issues Found
- 🟠 Issue: CI was not clean at review time:
sdk-testswas failing and 8 checks were still pending. I did not rerun tests per QA instructions.
This review was created by an AI agent (OpenHands) on behalf of the user.
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Review
Clean, well-motivated fix. The four-method consistency, documented one-shot recursion safety, and thorough test coverage all check out. A few observations below.
✅ What works well
- The classifier follows the existing
PATTERN → isinstance-guard → string matchstyle exactly, making it easy to find and update. - Capturing
_caller_kwargs = kwargs.copy()before any kwargs mutation guarantees the original caller intent is preserved on retry and in the fallback path. - The one-shot recursion safety argument is correct:
no_cache_llm.is_caching_prompt_active()returnsFalsebecausecaching_prompt=False, so the retry cannot re-enter this branch. - Test coverage is solid: classification positives/negatives, context-window-vs-cache discrimination, and all four API methods (sync/async × completion/responses) including kwargs-forwarding probes.
⚠️ Points to consider
_caller_kwargs in _handle_error fallback (scope creep): Threading **_caller_kwargs into the _handle_error lambda is a useful bonus fix, but it silently changes the behavior of the existing fallback-LLM path for every call — not just the cache-retry path. If a configured fallback LLM (from a different provider) does not accept a kwarg that the primary LLM does, this could turn a previously-working fallback into a failure. Consider either (a) adding an explicit test that exercises the fallback path with extra kwargs, or (b) calling out this behavioral change in the PR description so reviewers know it is intentional.
Pattern brittleness (already noted in code, restating for visibility): The single phrase "minimum token count to start caching" covers Vertex AI today, but other providers (e.g., Anthropic, AWS Bedrock) may express the same constraint with different wording. If cross-provider coverage is a future goal, the list will need additions per provider. A regex like "minimum.*token.*cach" in a helper function would be more resilient to minor phrasing variation, at the cost of slightly harder debuggability.
🔍 Nit
In is_prompt_cache_too_small: isinstance(exception, (BadRequestError, OpenAIError)) is redundant because BadRequestError is already a subclass of OpenAIError in LiteLLM — checking OpenAIError alone suffices. Listing both is consistent with the project style elsewhere and makes the expected error type explicit, so this is purely cosmetic. No change necessary.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
|
@OpenHands please resolve reviewers concerns |
|
I'm on it! juanmichelini can track my progress at all-hands.dev |
Adds a regression test verifying that arbitrary caller kwargs (e.g. ``metadata``) passed to ``LLM.completion()`` reach the fallback LLM call when the primary fails with a transient error. Addresses review feedback on the scope of the ``_caller_kwargs`` threading through ``_handle_error``. Co-authored-by: openhands <openhands@all-hands.dev>
|
@juanmichelini I've addressed the reviewer concerns:
All three review threads are now resolved with reply context. Pushed as commit This comment was created by an AI agent (OpenHands) on behalf of @juanmichelini. |
Final SummaryI addressed the reviewer concerns on PR #3480 ("feat(llm): retry without prompt caching when cached content is below provider minimum"). The PR had three open review threads from Checklist of instructions followed
Conciseness of changesThe only file changed is
No extraneous changes; no production code modified. The PR description edit is documentation-only and directly tied to the reviewer's request to "call out this behavioral change in the PR description so reviewers know it is intentional." PR: #3480 |
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ PR Review: Approved
This is a well-designed feature that addresses a real operational problem with Vertex AI Gemini models and prompt caching. The implementation is clean, consistent across all four LLM method variants (completion/acompletion/responses/aresponses), and well-covered by tests.
Strengths
- Clean error classification:
is_prompt_cache_too_small()follows the existing classifier pattern and is properly exported from the exceptions package. - One-shot retry safety: Using
model_copy(update={"caching_prompt": False})is elegant—it prevents recursion becauseis_caching_prompt_active()will return False afterward. - Consistent fallback improvement: Threading
**_caller_kwargsthrough_handle_erroris a good secondary fix that prevents silent kwargs drops in the fallback path. - Comprehensive tests: The test coverage spans the classifier, all four method paths, and the fallback kwarg forwarding.
Minor Observations (no blocking issues)
-
Classifier pattern fragility: The pattern
"minimum token count to start caching"is specific to Vertex AI's current error message. If the provider changes the wording, this classifier would silently stop working. Consider adding a comment inPROMPT_CACHE_TOO_SMALL_PATTERNSnoting this, or adding a test that explicitly documents the expected error format. -
Test assertions: The test files appear to have partial assertions at the end (e.g.,
assert response.raw_response == mock_in test_llm_completion.py). This may be truncated output, but worth verifying the assertions are complete. -
CI Status: CI checks are currently pending. Ensure they pass before merging.
Existing Review Comments
The existing comments from all-hands-bot and juanmichelini have been appropriately addressed. The PR is ready for merge pending CI.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 QA Report: PARTIAL
SDK retry behavior works for the canonical Vertex prompt-cache-too-small error across sync/async chat and responses entrypoints; live Vertex AI verification was not possible because no Google/Vertex credentials were present.
Does this PR achieve its stated goal?
Yes for the SDK behavior this PR implements. On origin/main, the same simulated Vertex BadRequestError bubbled up from LLM.completion() as LLMBadRequestError; on the PR commit, the SDK retried once without prompt caching and returned successful results for completion, acompletion, responses, and aresponses, while preserving caller kwargs. The provider fallback path also preserved metadata when falling through to a profile-store fallback LLM.
| Phase | Result |
|---|---|
| Environment Setup | ✅ make build completed and installed the uv environment. |
| CI Status | qa-changes was in progress and unresolved-review-threads was failing at the time checked. |
| Functional Verification | ✅ SDK behavior verified with public LLM APIs and a canonical Vertex error; live Vertex API call not verified due missing credentials. |
Functional Verification
Test 1: Prompt-cache-too-small retry behavior
Step 1 — Establish baseline without the fix:
Checked out origin/main and ran uv run python /tmp/qa_prompt_cache_retry.py against the SDK public LLM.completion() API with a simulated LiteLLM BadRequestError containing the canonical Vertex message:
litellm.exceptions.BadRequestError: ... The minimum token count to start caching is 4096.
...
openhands.sdk.llm.exceptions.types.LLMBadRequestError: litellm.BadRequestError: ... The minimum token count to start caching is 4096.
This confirms the old behavior: the cache-too-small provider error bubbled up as a hard LLM failure instead of retrying without prompt caching.
Step 2 — Apply the PR's changes:
Checked out PR commit 8aa5c53687914140bfca428e62fdc43ad1764f03.
Step 3 — Re-run with the fix in place:
Ran the same command, uv run python /tmp/qa_prompt_cache_retry.py:
Prompt cache content too small for provider minimum, retrying without prompt caching
completion_result= completion retry succeeded
completion_calls= 2
completion_first_has_cache= True
completion_second_has_cache= False
completion_second_metadata= {'trace': 'sync-completion'}
acompletion_result= acompletion retry succeeded
acompletion_calls= 2
acompletion_first_has_cache= True
acompletion_second_has_cache= False
acompletion_second_metadata= {'trace': 'async-completion'}
responses_output= responses retry succeeded
responses_calls= 2
responses_second_store= False
responses_second_metadata= {'trace': 'sync-responses'}
aresponses_output= aresponses retry succeeded
aresponses_calls= 2
aresponses_second_store= False
aresponses_second_metadata= {'trace': 'async-responses'}
This shows the PR behavior: the first call receives the provider cache error, the SDK retries exactly once, the retry succeeds with prompt caching disabled where markers are visible, and caller kwargs (metadata, store) survive the retry path.
Test 2: Fallback path preserves caller kwargs
Using the same PR checkout, the script configured a real temporary LLMProfileStore fallback profile and called LLM.completion(..., metadata={"trace": "fallback"}) with the primary transport raising APIConnectionError:
[Fallback Strategy]Primary LLM (gpt-4o) failed with APIConnectionError, trying fallback 1/1 (fallback-model)
[Fallback Strategy] Fallback LLM (fallback-model) succeeded
fallback_result= fallback succeeded
fallback_calls= 2
fallback_second_model= fallback-model
fallback_second_metadata= {'trace': 'fallback'}
This verifies the PR's stated secondary behavior: fallback LLM calls now receive caller kwargs instead of silently dropping them.
Unable to Verify
A live Vertex AI/Gemini request was not executed. I checked the environment without printing values and found GOOGLE_APPLICATION_CREDENTIALS, GOOGLE_CLOUD_PROJECT, VERTEXAI_PROJECT, and VERTEXAI_LOCATION were all unset, so there was no usable Vertex project/auth context for an end-to-end provider call. Future QA runs could add AGENTS.md guidance for a safe Vertex test project/model and required env vars if live-provider validation is expected.
Issues Found
None.
This review was created by an AI agent (OpenHands) on behalf of the user.
Verdict: PARTIAL
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 QA Report: PARTIAL
Verified the SDK retry behavior through real public LLM entry points; live Vertex AI could not be exercised because no Google/Vertex credentials are configured in this environment.
Does this PR achieve its stated goal?
Yes for the SDK behavior this PR implements. On the base branch, the same LLM.completion, LLM.acompletion, LLM.responses, and LLM.aresponses calls surfaced the provider-shaped cache-too-small BadRequestError after a single transport call; on the PR branch, each call retried once and returned success while preserving caller kwargs such as metadata/store. I also verified the related fallback path now forwards caller kwargs to the fallback LLM. Live Vertex AI integration remains unverified due to absent GOOGLE_/VERTEX_/GCP_ credential environment variables.
| Phase | Result |
|---|---|
| Environment Setup | ✅ make build completed and installed the workspace packages. |
| CI Status | 🟡 Snapshot at QA time: 20 successful checks, 7 in progress, 1 skipped. |
| Functional Verification | ✅ Public SDK calls show the claimed before/after behavior; live Vertex call not available. |
Functional Verification
Test 1: Prompt-cache-too-small retry across LLM entry points
Step 1 — Reproduce / establish baseline without the fix:
Checked out the base branch and ran a temporary SDK script that calls the public LLM methods with caching_prompt=True; the script simulates the provider returning the canonical Vertex error from the LiteLLM transport, then returns typed success responses if the SDK retries:
git fetch origin main && git checkout --detach origin/main && git rev-parse --short HEAD
uv run python /tmp/qa_prompt_cache_retry.pyObserved excerpt:
c950fdb0
completion: ERROR LLMBadRequestError litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}
completion_calls: 1
completion_call_1: cache=True metadata={'qa': 'completion'}
acompletion: ERROR LLMBadRequestError litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}
acompletion_calls: 1
responses: ERROR LLMBadRequestError litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}
responses_calls: 1
aresponses: ERROR LLMBadRequestError litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}
aresponses_calls: 1
This confirms the old behavior: the provider cache-size error bubbles as a hard LLMBadRequestError and no retry occurs.
Step 2 — Apply the PR's changes:
git checkout openhands/llm-prompt-cache-too-small-retryStep 3 — Re-run with the fix in place:
git rev-parse --short HEAD && uv run python /tmp/qa_prompt_cache_retry.pyObserved excerpt:
73b71665
completion: SUCCESS completion retry succeeded
completion_calls: 2
completion_call_1: cache=True metadata={'qa': 'completion'}
completion_call_2: cache=False metadata={'qa': 'completion'}
acompletion: SUCCESS acompletion retry succeeded
acompletion_calls: 2
acompletion_call_1: cache=True metadata={'qa': 'acompletion'}
acompletion_call_2: cache=False metadata={'qa': 'acompletion'}
responses: SUCCESS responses retry succeeded
responses_calls: 2
responses_call_1: cache=False store=False metadata={'qa': 'responses'}
responses_call_2: cache=False store=False metadata={'qa': 'responses'}
aresponses: SUCCESS aresponses retry succeeded
aresponses_calls: 2
aresponses_call_1: cache=False store=False metadata={'qa': 'aresponses'}
aresponses_call_2: cache=False store=False metadata={'qa': 'aresponses'}
This shows the PR catches the provider-shaped cache-too-small error, performs a one-shot retry, and returns the successful SDK result. For completion paths, the first transport call includes cache markers and the retry removes them; for responses paths, the public method still retries and preserves store/metadata on the second call.
Test 2: Fallback path preserves caller kwargs
The same script also used a real FallbackStrategy with a temporary LLMProfileStore: the primary transport raised APIConnectionError, then fallback succeeded.
Base branch excerpt:
fallback: SUCCESS fallback preserved kwargs
fallback_calls: 2
fallback_call_1: model=gpt-4o metadata={'qa': 'fallback'}
fallback_call_2: model=fallback-model metadata=None
PR branch excerpt:
fallback: SUCCESS fallback preserved kwargs
fallback_calls: 2
fallback_call_1: model=gpt-4o metadata={'qa': 'fallback'}
fallback_call_2: model=fallback-model metadata={'qa': 'fallback'}
This confirms the PR's stated fallback-path behavior change: caller kwargs now reach the fallback LLM call instead of being silently dropped.
Unable to Verify
I did not make a live Vertex AI/Gemini API call because the environment has no relevant credential variables configured:
python - <<'EOF'
import os
names = [k for k in os.environ if k.startswith(('GOOGLE_', 'VERTEX_', 'GCP_')) or k in {'GOOGLE_APPLICATION_CREDENTIALS', 'GCLOUD_PROJECT'}]
print('vertex_related_env_names:', sorted(names))
EOFObserved:
vertex_related_env_names: []
Future QA would be stronger if AGENTS.md documented a safe, low-cost Vertex/Gemini smoke-test profile for SDK LLM changes that require provider credentials.
Issues Found
None.
This QA review was created by an AI agent (OpenHands) on behalf of the user.
Verdict: PARTIAL
…provider minimum (OpenHands#3480) Co-authored-by: openhands <openhands@all-hands.dev>
3 participants
Why
Vertex AI (Gemini) requires a minimum number of tokens (currently 4096) to create a context cache. When the cached content is below that threshold, the API returns a 400
BadRequestErrorlike:Previously this would bubble up as a hard failure on the very first agent step (where the system prompt + small initial context is naturally well under 4 KB). This makes any Vertex Gemini model with
caching_prompt=Trueeffectively unusable until the conversation grows past the threshold.Summary
is_prompt_cache_too_small(exc)classifier inopenhands/sdk/llm/exceptions/classifier.py, matching the canonical Vertex AI error string"minimum token count to start caching"onBadRequestError/OpenAIError. Exported from theopenhands.sdk.llm.exceptionspackage.LLM.completion,LLM.acompletion,LLM.responses, andLLM.aresponses: catch this exception, log a warning, build amodel_copy(update={"caching_prompt": False})of the current LLM, and retry the call with the original callerkwargspreserved.kwargsthrough the existing_handle_errorfallback path so the no-cache retry and provider-level fallback have the same behavior.Behavioral note on the fallback path: Forwarding
**_caller_kwargsinto_handle_erroris an intentional change for all callers ofcompletion/responses(and their async variants), not only the cache-retry path. Previously the fallback LLM was called without the caller's kwargs, which silently dropped things likemetadata. The new behavior is covered bytests/sdk/llm/test_llm_fallback.py::test_fallback_forwards_caller_kwargs. If a configured fallback LLM rejects an unknown kwarg accepted by the primary, that would surface as a new failure, but the previous silent-drop behavior is the more likely source of bugs in practice.This is a standalone reliability improvement that benefits all Vertex AI Gemini models, not just
gemini-3.5-flash.Issue Number
N/A — extracted from #3315.
How to Test
Local result: classifier + retry + fallback-kwargs tests pass.
Type
Notes
This PR is one of three that replace #3315 (which is being closed):
ADDINGMODEL.mdguidance update (separate PR)gemini-3.5-flashmodel addition (separate PR)The retry path is intentionally a one-shot opt-out (no recursion guard needed) because after
model_copy(update={"caching_prompt": False})the next attempt cannot triggeris_caching_prompt_active().This PR was created by an AI agent (OpenHands) on behalf of @juanmichelini.
@juanmichelini can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:1ec0757-pythonRun
All tags pushed for this build
About Multi-Architecture Support
1ec0757-python) is a multi-arch manifest supporting both amd64 and arm641ec0757-python-amd64) are also available if needed