mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(llm): fall back to non-Gemini model on Gemini content_filter (SKY-11766) (#7155)
Some checks are pending
Run tests and pre-commit / Run tests and pre-commit hooks (push) Waiting to run
Run tests and pre-commit / Frontend Lint and Build (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.11) (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.13) (push) Waiting to run
Publish Fern Docs / run (push) Waiting to run
Some checks are pending
Run tests and pre-commit / Run tests and pre-commit hooks (push) Waiting to run
Run tests and pre-commit / Frontend Lint and Build (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.11) (push) Waiting to run
Run tests and pre-commit / pip Package Smoke Tests (3.13) (push) Waiting to run
Publish Fern Docs / run (push) Waiting to run
This commit is contained in:
parent
c14b6cb95f
commit
90e29f9fc9
5 changed files with 185 additions and 5 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue