diff --git a/skyvern/forge/sdk/api/llm/api_handler_factory.py b/skyvern/forge/sdk/api/llm/api_handler_factory.py index 2e84f3df3..d06ff6051 100644 --- a/skyvern/forge/sdk/api/llm/api_handler_factory.py +++ b/skyvern/forge/sdk/api/llm/api_handler_factory.py @@ -34,6 +34,7 @@ from skyvern.forge.sdk.api.llm.exceptions import ( from skyvern.forge.sdk.api.llm.litellm_transport import configure_litellm_transport from skyvern.forge.sdk.api.llm.ui_tars_response import UITarsResponse from skyvern.forge.sdk.api.llm.utils import ( + is_content_filtered_response, is_image_message, is_truncated_response, llm_messages_builder, @@ -1378,6 +1379,43 @@ class LLMAPIHandlerFactory: ), ) + # A content_filter response with empty content is a valid ModelResponse, + # so litellm's router fallback (which only fires on exceptions) never + # recovers it. Gemini's non-configurable safety filters block PII-heavy + # prompts that safety_settings=BLOCK_NONE cannot recover, so retrying on + # another Gemini tier hits the same block — jump straight to the first + # non-Gemini fallback group. Fires for any Gemini model that produced the + # block, including a standard tier litellm already fell back to (SKY-11766). + non_gemini_fallback = next( + (group for group in fallback_groups if "gemini" not in group.lower()), None + ) + if ( + is_content_filtered_response(response) + and non_gemini_fallback is not None + and "gemini" in (model_used or "").lower() + ): + fallback_model = non_gemini_fallback + LOG.warning( + "LLM response blocked by content filter on Gemini, retrying with non-Gemini fallback", + llm_key=llm_key, + prompt_name=prompt_name, + filtered_model=model_used, + fallback_model=fallback_model, + ) + _llm_call_start = time.perf_counter() + try: + # No timeout= kwarg here either; see SKY-10200 note on + # the primary call site above. + response = await router.acompletion( + model=fallback_model, + messages=messages, + drop_params=True, + **parameters, + ) + finally: + llm_duration_seconds += time.perf_counter() - _llm_call_start + model_used = response.model or fallback_model + # Error paths only set status=error, not token/cost attrs via # _enrich_llm_span — no response object exists so there's nothing to report. except litellm.exceptions.APIError as e: diff --git a/skyvern/forge/sdk/api/llm/utils.py b/skyvern/forge/sdk/api/llm/utils.py index 0178e710f..ec4170eba 100644 --- a/skyvern/forge/sdk/api/llm/utils.py +++ b/skyvern/forge/sdk/api/llm/utils.py @@ -190,6 +190,20 @@ def is_truncated_response(response: litellm.ModelResponse) -> bool: ) +def is_content_filtered_response(response: litellm.ModelResponse) -> bool: + # Gemini's non-configurable safety filters (prohibited-content / SPII) block PII-heavy + # prompts at the input stage, returning finish_reason=content_filter with empty content. + # This is a valid ModelResponse, so litellm's router fallback never fires — the caller + # must detect it and retry on a non-Gemini fallback explicitly (SKY-11766). + if not response.choices: + return False + choice = response.choices[0] + return ( + getattr(choice, "finish_reason", None) == "content_filter" + and getattr(getattr(choice, "message", None), "content", None) is None + ) + + def parse_api_response( response: litellm.ModelResponse, add_assistant_prefix: bool = False, force_dict: bool = True ) -> dict[str, Any] | Any: diff --git a/tests/unit/helpers.py b/tests/unit/helpers.py index 27c5edbdd..e3d1b79cd 100644 --- a/tests/unit/helpers.py +++ b/tests/unit/helpers.py @@ -22,14 +22,15 @@ from skyvern.forge.sdk.schemas.tasks import Task, TaskStatus class FakeLLMResponse: - def __init__(self, model: str) -> None: + def __init__(self, model: str, content: str | None = '{"actions": []}', finish_reason: str | None = None) -> None: self.model = model - self._content = '{"actions": []}' + self._content = content self.choices = [ SimpleNamespace( + finish_reason=finish_reason, message=SimpleNamespace( - content=self._content, - ) + content=content, + ), ) ] self.usage = SimpleNamespace( diff --git a/tests/unit/test_api_handler_factory.py b/tests/unit/test_api_handler_factory.py index c9385f7f8..d123ab594 100644 --- a/tests/unit/test_api_handler_factory.py +++ b/tests/unit/test_api_handler_factory.py @@ -770,6 +770,106 @@ async def test_router_acompletion_does_not_pass_timeout_kwarg(monkeypatch: pytes ) +@pytest.mark.asyncio +async def test_router_retries_content_filter_on_first_non_gemini_fallback(monkeypatch: pytest.MonkeyPatch) -> None: + """Gemini's non-configurable content filter blocks a PII-heavy prompt, returning a *valid* + empty ModelResponse that litellm's exception-driven router fallback never recovers. Retrying + on another Gemini tier hits the same block, so the handler must skip the Gemini + standard-fallback and jump to the first NON-Gemini fallback group (SKY-11766).""" + + calls: list[str] = [] + fallback_kwargs: dict[str, Any] = {} + + class _FilterThenSucceedRouter: + def __init__(self, **kwargs: Any) -> None: + pass + + async def acompletion(self, *, model: str, messages: Any, **kwargs: Any) -> FakeLLMResponse: + calls.append(model) + if len(calls) == 1: + return FakeLLMResponse("gemini-3.1-flash-lite-preview", content=None, finish_reason="content_filter") + fallback_kwargs.update(kwargs) + return FakeLLMResponse("gpt-5-fallback", content='{"actions": []}') + + monkeypatch.setattr(api_handler_factory.litellm, "Router", _FilterThenSucceedRouter) + + config = _make_three_tier_router_config(fallback_groups=["vertex-gemini-standard-fallback", "gpt-5-fallback"]) + _stub_for_router_test(monkeypatch, llm_key="TEST_CONTENT_FILTER_FALLBACK", config=config) + + handler = LLMAPIHandlerFactory.get_llm_api_handler_with_router("TEST_CONTENT_FILTER_FALLBACK") + result = await handler(prompt='{"actions": []}', prompt_name="extract-actions") + + assert calls == ["primary-group", "gpt-5-fallback"], ( + "handler must skip the Gemini standard-fallback tier and retry the first non-Gemini " + f"fallback after a content_filter response; got calls={calls}" + ) + assert result == {"actions": []} + # The non-Gemini fallback call must not carry Gemini's safety_settings param — Azure 400s on + # it and the fallback dies. get_api_parameters keeps it off router configs (per-deployment + # injection instead), so **parameters stays clean here. Regression guard for incident #646. + assert "safety_settings" not in fallback_kwargs, ( + f"non-Gemini fallback call must not carry safety_settings; got {fallback_kwargs}" + ) + + +@pytest.mark.asyncio +async def test_router_does_not_retry_content_filter_without_non_gemini_fallback( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When every fallback group is also Gemini there is no filter-free tier to escape to — the + content_filter must surface as a parse failure, not loop or retry another Gemini (SKY-11766).""" + from skyvern.forge.sdk.api.llm.exceptions import EmptyLLMResponseError, InvalidLLMResponseFormat + + calls: list[str] = [] + + class _AlwaysFilterRouter: + def __init__(self, **kwargs: Any) -> None: + pass + + async def acompletion(self, *, model: str, messages: Any, **kwargs: Any) -> FakeLLMResponse: + calls.append(model) + return FakeLLMResponse("gemini-3.1-flash-lite-preview", content=None, finish_reason="content_filter") + + monkeypatch.setattr(api_handler_factory.litellm, "Router", _AlwaysFilterRouter) + + config = _make_three_tier_router_config(fallback_groups=["vertex-gemini-standard-fallback"]) + _stub_for_router_test(monkeypatch, llm_key="TEST_CONTENT_FILTER_NO_NON_GEMINI", config=config) + + handler = LLMAPIHandlerFactory.get_llm_api_handler_with_router("TEST_CONTENT_FILTER_NO_NON_GEMINI") + with pytest.raises((EmptyLLMResponseError, InvalidLLMResponseFormat)): + await handler(prompt='{"actions": []}', prompt_name="extract-actions") + + assert calls == ["primary-group"], f"must not retry when there is no non-Gemini fallback; got calls={calls}" + + +@pytest.mark.asyncio +async def test_router_does_not_retry_content_filter_for_non_gemini_model(monkeypatch: pytest.MonkeyPatch) -> None: + """The escape hatch is scoped to Gemini's non-configurable filter. A content_filter from a + non-Gemini model must not trigger the Gemini-specific fallback retry (SKY-11766).""" + from skyvern.forge.sdk.api.llm.exceptions import EmptyLLMResponseError, InvalidLLMResponseFormat + + calls: list[str] = [] + + class _FilterRouter: + def __init__(self, **kwargs: Any) -> None: + pass + + async def acompletion(self, *, model: str, messages: Any, **kwargs: Any) -> FakeLLMResponse: + calls.append(model) + return FakeLLMResponse("gpt-5", content=None, finish_reason="content_filter") + + monkeypatch.setattr(api_handler_factory.litellm, "Router", _FilterRouter) + + config = _make_three_tier_router_config(fallback_groups=["gpt-5-mini-fallback"]) + _stub_for_router_test(monkeypatch, llm_key="TEST_CONTENT_FILTER_NON_GEMINI_MODEL", config=config) + + handler = LLMAPIHandlerFactory.get_llm_api_handler_with_router("TEST_CONTENT_FILTER_NON_GEMINI_MODEL") + with pytest.raises((EmptyLLMResponseError, InvalidLLMResponseFormat)): + await handler(prompt='{"actions": []}', prompt_name="extract-actions") + + assert calls == ["primary-group"], f"non-Gemini content_filter must not trigger Gemini fallback; got calls={calls}" + + def test_router_fallback_chain_no_duplicate_keys_or_overlapping_chains(monkeypatch: pytest.MonkeyPatch) -> None: """Per-hop fallback expansion is constructed as strict suffixes — each non-terminal hop appears as a key at most once and each entry's chain diff --git a/tests/unit/test_llm_output_truncation.py b/tests/unit/test_llm_output_truncation.py index 38ea6f291..86bf0702c 100644 --- a/tests/unit/test_llm_output_truncation.py +++ b/tests/unit/test_llm_output_truncation.py @@ -3,7 +3,11 @@ from unittest.mock import MagicMock import pytest from skyvern.forge.sdk.api.llm.exceptions import LLMOutputTruncatedError -from skyvern.forge.sdk.api.llm.utils import is_truncated_response, parse_api_response +from skyvern.forge.sdk.api.llm.utils import ( + is_content_filtered_response, + is_truncated_response, + parse_api_response, +) def _make_response(finish_reason: str, content: str | None, model: str = "gemini-3-flash-preview") -> MagicMock: @@ -47,6 +51,29 @@ class TestIsTruncatedResponse: assert is_truncated_response(resp) is False +class TestIsContentFilteredResponse: + def test_content_filter_finish_with_none_content(self) -> None: + resp = _make_response(finish_reason="content_filter", content=None) + assert is_content_filtered_response(resp) is True + + def test_content_filter_finish_with_content(self) -> None: + resp = _make_response(finish_reason="content_filter", content='{"ok": true}') + assert is_content_filtered_response(resp) is False + + def test_stop_finish_with_none_content(self) -> None: + resp = _make_response(finish_reason="stop", content=None) + assert is_content_filtered_response(resp) is False + + def test_length_finish_with_none_content(self) -> None: + resp = _make_response(finish_reason="length", content=None) + assert is_content_filtered_response(resp) is False + + def test_empty_choices(self) -> None: + resp = MagicMock() + resp.choices = [] + assert is_content_filtered_response(resp) is False + + class TestParseApiResponseTruncation: def test_truncated_response_raises_llm_output_truncated_error(self) -> None: resp = _make_response(finish_reason="length", content=None)