From 39c5a1ba883eb08c24befd2e9793b8b1ed3e71b4 Mon Sep 17 00:00:00 2001 From: enyst Date: Thu, 25 Jun 2026 12:43:15 +0000 Subject: [PATCH] fix(llm): remove obsolete Vertex cache retry Gemini no longer uses explicit cache_control prompt caching, so the Vertex cached-content minimum-token recovery path is dead code. Co-authored-by: openhands --- .../openhands/sdk/llm/exceptions/__init__.py | 2 - .../sdk/llm/exceptions/classifier.py | 24 --- openhands-sdk/openhands/sdk/llm/llm.py | 69 -------- tests/sdk/llm/test_exception_classifier.py | 33 ---- tests/sdk/llm/test_llm_completion.py | 147 +--------------- tests/sdk/llm/test_llm_fallback.py | 5 +- .../llm/test_responses_parsing_and_kwargs.py | 157 +----------------- 7 files changed, 4 insertions(+), 433 deletions(-) diff --git a/openhands-sdk/openhands/sdk/llm/exceptions/__init__.py b/openhands-sdk/openhands/sdk/llm/exceptions/__init__.py index 383e6580f6..41e668dc6f 100644 --- a/openhands-sdk/openhands/sdk/llm/exceptions/__init__.py +++ b/openhands-sdk/openhands/sdk/llm/exceptions/__init__.py @@ -1,6 +1,5 @@ from .classifier import ( is_context_window_exceeded, - is_prompt_cache_too_small, looks_like_auth_error, looks_like_malformed_conversation_history_error, ) @@ -49,7 +48,6 @@ "OperationCancelled", # Helpers "is_context_window_exceeded", - "is_prompt_cache_too_small", "looks_like_auth_error", "looks_like_malformed_conversation_history_error", "map_provider_exception", diff --git a/openhands-sdk/openhands/sdk/llm/exceptions/classifier.py b/openhands-sdk/openhands/sdk/llm/exceptions/classifier.py index 470c303e6d..5e1515169f 100644 --- a/openhands-sdk/openhands/sdk/llm/exceptions/classifier.py +++ b/openhands-sdk/openhands/sdk/llm/exceptions/classifier.py @@ -86,16 +86,6 @@ def looks_like_malformed_conversation_history_error(exception: Exception) -> boo return any(p in s for p in MALFORMED_HISTORY_PATTERNS) -# Vertex AI (Gemini) rejects context-caching requests when the cached content -# is below the provider's minimum token threshold (currently 4096 tokens). -# Example error: "The cached content is of 1171 tokens. The minimum token -# count to start caching is 4096." — the `.lower()` comparison handles case -# variation across providers but won't match reworded messages; update this -# pattern if the API phrasing changes. -PROMPT_CACHE_TOO_SMALL_PATTERNS: list[str] = [ - "minimum token count to start caching", -] - AUTH_PATTERNS: list[str] = [ "invalid api key", "unauthorized", @@ -105,20 +95,6 @@ def looks_like_malformed_conversation_history_error(exception: Exception) -> boo ] -def is_prompt_cache_too_small(exception: Exception) -> bool: - """Return True if the error indicates the prompt cache content is too small. - - 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 error. The SDK should detect this and retry without - prompt caching markers. - """ - if not isinstance(exception, (BadRequestError, OpenAIError)): - return False - s = str(exception).lower() - return any(p in s for p in PROMPT_CACHE_TOO_SMALL_PATTERNS) - - def looks_like_auth_error(exception: Exception) -> bool: # Trust the typed exception when the provider/LiteLLM raised an explicit # 401/403 — its message text may not contain the heuristic patterns below. diff --git a/openhands-sdk/openhands/sdk/llm/llm.py b/openhands-sdk/openhands/sdk/llm/llm.py index fba9003422..fdc5fed648 100644 --- a/openhands-sdk/openhands/sdk/llm/llm.py +++ b/openhands-sdk/openhands/sdk/llm/llm.py @@ -88,7 +88,6 @@ from openhands.sdk.llm.exceptions import ( LLMContextWindowTooSmallError, LLMNoResponseError, - is_prompt_cache_too_small, map_provider_exception, ) @@ -1388,22 +1387,6 @@ def _one_attempt(**retry_kwargs: Any) -> ModelResponse: try: return self._build_completion_result(_one_attempt()) except Exception as e: - # If the prompt cache content is too small for the provider's - # minimum token threshold (e.g., Vertex AI requires ≥4096 tokens), - # retry without prompt caching markers. - if is_prompt_cache_too_small(e) and self.is_caching_prompt_active(): - logger.warning( - "Prompt cache content too small for provider minimum, " - "retrying without prompt caching" - ) - no_cache_llm = self.model_copy(update={"caching_prompt": False}) - return no_cache_llm.completion( - messages, - tools, - add_security_risk_prediction=add_security_risk_prediction, - on_token=on_token, - **_caller_kwargs, - ) return self._handle_error( e, lambda fb: fb.completion( @@ -1471,22 +1454,6 @@ async def _one_attempt(**retry_kwargs: Any) -> ModelResponse: try: return self._build_completion_result(await _one_attempt()) except Exception as e: - # If the prompt cache content is too small for the provider's - # minimum token threshold (e.g., Vertex AI requires ≥4096 tokens), - # retry without prompt caching markers. - if is_prompt_cache_too_small(e) and self.is_caching_prompt_active(): - logger.warning( - "Prompt cache content too small for provider minimum, " - "retrying without prompt caching" - ) - no_cache_llm = self.model_copy(update={"caching_prompt": False}) - return await no_cache_llm.acompletion( - messages, - tools, - add_security_risk_prediction=add_security_risk_prediction, - on_token=on_token, - **_caller_kwargs, - ) # Fallback is synchronous; cast the token callback since the # fallback LLM's sync path accepts TokenCallbackType. _fb_token = cast("TokenCallbackType | None", on_token) @@ -1610,24 +1577,6 @@ def _one_attempt(**retry_kwargs: Any) -> ResponsesAPIResponse: try: return self._build_responses_result(_one_attempt()) except Exception as e: - # If the prompt cache content is too small for the provider's - # minimum token threshold (e.g., Vertex AI requires ≥4096 tokens), - # retry without prompt caching markers. - if is_prompt_cache_too_small(e) and self.is_caching_prompt_active(): - logger.warning( - "Prompt cache content too small for provider minimum, " - "retrying without prompt caching" - ) - no_cache_llm = self.model_copy(update={"caching_prompt": False}) - return no_cache_llm.responses( - messages, - tools, - include, - store, - add_security_risk_prediction=add_security_risk_prediction, - on_token=on_token, - **_caller_kwargs, - ) return self._handle_error( e, lambda fb: fb.responses( @@ -1746,24 +1695,6 @@ async def _one_attempt( try: return self._build_responses_result(await _one_attempt()) except Exception as e: - # If the prompt cache content is too small for the provider's - # minimum token threshold (e.g., Vertex AI requires ≥4096 tokens), - # retry without prompt caching markers. - if is_prompt_cache_too_small(e) and self.is_caching_prompt_active(): - logger.warning( - "Prompt cache content too small for provider minimum, " - "retrying without prompt caching" - ) - no_cache_llm = self.model_copy(update={"caching_prompt": False}) - return await no_cache_llm.aresponses( - messages, - tools, - include, - store, - add_security_risk_prediction=add_security_risk_prediction, - on_token=on_token, - **_caller_kwargs, - ) _fb_token = cast("TokenCallbackType | None", on_token) return await self._ahandle_error( e, diff --git a/tests/sdk/llm/test_exception_classifier.py b/tests/sdk/llm/test_exception_classifier.py index be6a4bb08b..fc1d0d3644 100644 --- a/tests/sdk/llm/test_exception_classifier.py +++ b/tests/sdk/llm/test_exception_classifier.py @@ -7,7 +7,6 @@ from openhands.sdk.llm.exceptions import ( is_context_window_exceeded, - is_prompt_cache_too_small, looks_like_auth_error, looks_like_malformed_conversation_history_error, ) @@ -135,35 +134,3 @@ def test_looks_like_auth_error_negative(): looks_like_auth_error(BadRequestError("Something else", MODEL, PROVIDER)) is False ) - - -def test_is_prompt_cache_too_small_positive(): - """Vertex AI rejects caching when cached content is below minimum token count.""" - vertex_error = 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"}}' - ), - MODEL, - PROVIDER, - ) - assert is_prompt_cache_too_small(vertex_error) is True - - -def test_is_prompt_cache_too_small_negative(): - assert ( - is_prompt_cache_too_small(BadRequestError("irrelevant", MODEL, PROVIDER)) - is False - ) - - -def test_is_prompt_cache_too_small_context_window_not_cache_too_small(): - """Context window exceeded is a different error from cache too small.""" - ctx_error = BadRequestError( - "The request exceeds the available context size", MODEL, PROVIDER - ) - assert is_prompt_cache_too_small(ctx_error) is False - assert is_context_window_exceeded(ctx_error) is True diff --git a/tests/sdk/llm/test_llm_completion.py b/tests/sdk/llm/test_llm_completion.py index 4203fb569e..4587e832ba 100644 --- a/tests/sdk/llm/test_llm_completion.py +++ b/tests/sdk/llm/test_llm_completion.py @@ -3,7 +3,7 @@ import threading from collections.abc import Sequence from typing import Any, ClassVar -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import MagicMock, patch import pytest from litellm import ChatCompletionMessageToolCall, CustomStreamWrapper @@ -825,148 +825,3 @@ def test_llm_streaming_preserves_cache_read_tokens(mock_completion): assert actual_stream_options == {"include_usage": True}, ( f"Expected stream_options={{include_usage: True}}, got {actual_stream_options}" ) - - -@patch("openhands.sdk.llm.llm.litellm_completion") -def test_completion_retries_without_caching_on_prompt_cache_too_small( - mock_completion, -): - """When Vertex AI rejects caching due to small content, retry without cache.""" - from litellm.exceptions import BadRequestError - - # First call raises the "cache too small" error, second succeeds - cache_error = 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"}}' - ), - model="gemini-3.5-flash", - llm_provider="vertex_ai", - ) - mock_response = create_mock_response("Retry succeeded") - mock_completion.side_effect = [cache_error, mock_response] - - llm = LLM( - model="claude-sonnet-4-20250514", - api_key=SecretStr("test_key"), - usage_id="test-llm", - caching_prompt=True, - num_retries=2, - retry_min_wait=1, - retry_max_wait=2, - ) - - messages = [Message(role="user", content=[TextContent(text="Hello")])] - # Pass a kwarg via **kwargs to verify _caller_kwargs preservation on retry. - response = llm.completion(messages=messages, metadata={"trace": "sync"}) - - # Should succeed after retry without caching - assert response.raw_response == mock_response - # Two calls: first with cache (fails), second without cache (succeeds) - assert mock_completion.call_count == 2 - - # The first call SHOULD have cache_control markers - first_call_kwargs = mock_completion.call_args_list[0].kwargs - first_messages = first_call_kwargs.get("messages", []) - first_has_cache = any( - "cache_control" in str(block) - for msg in first_messages - for block in ( - msg.get("content", []) if isinstance(msg.get("content"), list) else [] - ) - ) - assert first_has_cache, "First call should include cache_control markers" - - # The second call should NOT have cache_control markers - second_call_kwargs = mock_completion.call_args_list[1].kwargs - second_messages = second_call_kwargs.get("messages", []) - second_has_cache = any( - "cache_control" in str(block) - for msg in second_messages - for block in ( - msg.get("content", []) if isinstance(msg.get("content"), list) else [] - ) - ) - assert not second_has_cache, "Retry should not include cache_control markers" - - # Caller kwargs preserved on the retry — without _caller_kwargs the retry - # would silently drop them. - assert second_call_kwargs.get("metadata") == {"trace": "sync"} - - -@pytest.mark.asyncio -@patch("openhands.sdk.llm.llm.litellm_acompletion", new_callable=AsyncMock) -async def test_acompletion_retries_without_caching_on_prompt_cache_too_small( - mock_acompletion, -): - """Async version of the sync prompt-cache-too-small retry test. - - When Vertex AI rejects caching due to small content, acompletion() should - retry without prompt caching while preserving caller kwargs. Mirrors - test_completion_retries_without_caching_on_prompt_cache_too_small. - """ - from litellm.exceptions import BadRequestError - - cache_error = 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"}}' - ), - model="gemini-3.5-flash", - llm_provider="vertex_ai", - ) - mock_response = create_mock_response("Retry succeeded") - mock_acompletion.side_effect = [cache_error, mock_response] - - llm = LLM( - model="claude-sonnet-4-20250514", - api_key=SecretStr("test_key"), - usage_id="test-llm", - caching_prompt=True, - num_retries=2, - retry_min_wait=1, - retry_max_wait=2, - ) - - messages = [Message(role="user", content=[TextContent(text="Hello")])] - # Pass a kwarg via **kwargs to verify _caller_kwargs preservation on retry. - response = await llm.acompletion(messages=messages, metadata={"trace": "abc"}) - - # Should succeed after retry without caching - assert response.raw_response == mock_response - # Two calls: first with cache (fails), second without cache (succeeds) - assert mock_acompletion.call_count == 2 - - # The first call SHOULD have cache_control markers - first_call_kwargs = mock_acompletion.call_args_list[0].kwargs - first_messages = first_call_kwargs.get("messages", []) - first_has_cache = any( - "cache_control" in str(block) - for msg in first_messages - for block in ( - msg.get("content", []) if isinstance(msg.get("content"), list) else [] - ) - ) - assert first_has_cache, "First call should include cache_control markers" - - # The second call should NOT have cache_control markers - second_call_kwargs = mock_acompletion.call_args_list[1].kwargs - second_messages = second_call_kwargs.get("messages", []) - second_has_cache = any( - "cache_control" in str(block) - for msg in second_messages - for block in ( - msg.get("content", []) if isinstance(msg.get("content"), list) else [] - ) - ) - assert not second_has_cache, "Retry should not include cache_control markers" - - # Caller kwargs preserved on the retry — without _caller_kwargs the retry - # would silently drop them. - assert second_call_kwargs.get("metadata") == {"trace": "abc"} diff --git a/tests/sdk/llm/test_llm_fallback.py b/tests/sdk/llm/test_llm_fallback.py index 44ce5ddd72..d46ac23dd5 100644 --- a/tests/sdk/llm/test_llm_fallback.py +++ b/tests/sdk/llm/test_llm_fallback.py @@ -441,9 +441,8 @@ async def test_aresponses_maps_connection_error(mock_aresp): def test_fallback_forwards_caller_kwargs(mock_comp): """Caller kwargs (e.g. ``metadata``) must reach the fallback LLM call. - Regression guard for the ``_caller_kwargs`` forwarding added alongside - the prompt-cache-too-small retry: the fallback path now receives the - same kwargs the caller passed to the primary's ``completion()``. + Regression guard that the fallback path receives the same kwargs the caller + passed to the primary's ``completion()``. """ primary_error = APIConnectionError( message="connection reset", llm_provider="openai", model="gpt-4o" diff --git a/tests/sdk/llm/test_responses_parsing_and_kwargs.py b/tests/sdk/llm/test_responses_parsing_and_kwargs.py index 92ec7d9f32..4f7985f763 100644 --- a/tests/sdk/llm/test_responses_parsing_and_kwargs.py +++ b/tests/sdk/llm/test_responses_parsing_and_kwargs.py @@ -1,4 +1,4 @@ -from unittest.mock import AsyncMock, patch +from unittest.mock import patch import pytest from litellm.types.llms.openai import ( @@ -12,7 +12,6 @@ ResponseReasoningItem, Summary, ) -from pydantic import SecretStr from openhands.sdk.llm import LLM from openhands.sdk.llm.message import Message, ReasoningItemModel, TextContent @@ -293,157 +292,3 @@ def test_responses_options_omits_prompt_cache_key_when_unset(): assert "prompt_cache_key" not in select_responses_options( llm, {}, include=None, store=None ) - - -@patch("openhands.sdk.llm.llm.litellm_responses") -def test_responses_retries_without_caching_on_prompt_cache_too_small(mock_responses): - """When Vertex AI rejects caching due to small content, responses() should - retry without prompt caching while preserving caller kwargs. - - Mirrors test_completion_retries_without_caching_on_prompt_cache_too_small in - test_llm_completion.py, but exercises the Responses API path. The two - methods differ in signature (``include``, ``store`` positional args) and in - how ``stream`` is resolved, so they need independent coverage. - """ - from litellm.exceptions import BadRequestError - - cache_error = 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"}}' - ), - model="gemini-3-flash", - llm_provider="vertex_ai", - ) - - # Build a typed ResponsesAPIResponse for the successful retry - msg = build_responses_message_output(["Retry succeeded"]) - usage = ResponseAPIUsage(input_tokens=0, output_tokens=0, total_tokens=0) - success_resp = ResponsesAPIResponse( - id="r1", - created_at=0, - output=[msg], - parallel_tool_calls=False, - tool_choice="auto", - top_p=None, - tools=[], - usage=usage, - instructions="", - status="completed", - ) - mock_responses.side_effect = [cache_error, success_resp] - - # Pick a model that supports prompt caching so is_caching_prompt_active() - # is True and the retry branch is reachable on the responses() path. - # (Gemini no longer uses explicit caching, so use an Anthropic model here.) - llm = LLM( - model="claude-sonnet-4-20250514", - api_key=SecretStr("test_key"), - usage_id="test-llm", - caching_prompt=True, - num_retries=2, - retry_min_wait=1, - retry_max_wait=2, - ) - - messages = [ - Message(role="system", content=[TextContent(text="sys")]), - Message(role="user", content=[TextContent(text="Hello")]), - ] - - # Pass caller kwargs that must survive the retry. ``metadata`` flows through - # ``**kwargs`` (not a named param), so it's the cleanest probe for the - # ``_caller_kwargs`` forwarding fix; ``store`` exercises the positional-arg - # path on the retry call signature. - response = llm.responses( - messages, - store=False, - metadata={"trace_id": "abc-123"}, - ) - - # Two calls: first with caching active (fails), second without (succeeds). - assert mock_responses.call_count == 2 - assert response.raw_response is success_resp - - # Caller kwargs preserved on the retry — without ``_caller_kwargs`` the - # retry would silently drop them. - second_kwargs = mock_responses.call_args_list[1].kwargs - assert second_kwargs.get("store") is False - assert second_kwargs.get("metadata") == {"trace_id": "abc-123"} - - -@pytest.mark.asyncio -@patch("openhands.sdk.llm.llm.litellm_aresponses", new_callable=AsyncMock) -async def test_aresponses_retries_without_caching_on_prompt_cache_too_small( - mock_aresponses, -): - """Async version of the sync responses prompt-cache-too-small retry test. - - Ensures aresponses() also retries without prompt caching when Vertex AI - rejects the request due to cache content below the minimum token - threshold, and preserves caller kwargs (positional ``store`` and - ``**kwargs`` metadata). Mirrors - test_responses_retries_without_caching_on_prompt_cache_too_small. - """ - from litellm.exceptions import BadRequestError - - cache_error = 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"}}' - ), - model="gemini-3-flash", - llm_provider="vertex_ai", - ) - - msg = build_responses_message_output(["Retry succeeded"]) - usage = ResponseAPIUsage(input_tokens=0, output_tokens=0, total_tokens=0) - success_resp = ResponsesAPIResponse( - id="r1", - created_at=0, - output=[msg], - parallel_tool_calls=False, - tool_choice="auto", - top_p=None, - tools=[], - usage=usage, - instructions="", - status="completed", - ) - mock_aresponses.side_effect = [cache_error, success_resp] - - # Anthropic model so is_caching_prompt_active() is True (Gemini no longer - # uses explicit caching); mirrors the sync test above. - llm = LLM( - model="claude-sonnet-4-20250514", - api_key=SecretStr("test_key"), - usage_id="test-llm", - caching_prompt=True, - num_retries=2, - retry_min_wait=1, - retry_max_wait=2, - ) - - messages = [ - Message(role="system", content=[TextContent(text="sys")]), - Message(role="user", content=[TextContent(text="Hello")]), - ] - - response = await llm.aresponses( - messages, - store=False, - metadata={"trace_id": "abc-123"}, - ) - - assert mock_aresponses.call_count == 2 - assert response.raw_response is success_resp - - second_kwargs = mock_aresponses.call_args_list[1].kwargs - assert second_kwargs.get("store") is False - assert second_kwargs.get("metadata") == {"trace_id": "abc-123"}