mirror of
https://github.com/usestrix/strix.git
synced 2026-09-05 08:06:08 +00:00
fix(llm): only attach prompt-cache points on routes LiteLLM serves (#1186)
Co-authored-by: Ahmed Allam <ahmed39652003@gmail.com>
This commit is contained in:
parent
717ffc8f4c
commit
583af23d9a
4 changed files with 73 additions and 1 deletions
|
|
@ -864,6 +864,22 @@ def is_claude_model(model_name: str) -> bool:
|
|||
return "claude" in (model_name or "").strip().lower()
|
||||
|
||||
|
||||
def routes_through_litellm(model_name: str | None) -> bool:
|
||||
"""Whether :class:`StrixProvider` sends this model through LiteLLM.
|
||||
|
||||
Bare names and the ``openai/``/``any-llm/`` prefixes are served by the SDK's
|
||||
own clients, which raise ``TypeError`` on request fields they do not know,
|
||||
so LiteLLM-only fields must not be attached there. A bare ``claude-...``
|
||||
name is exactly that case: an ``LLM_API_BASE`` pointing at an
|
||||
OpenAI-compatible gateway in front of Claude.
|
||||
"""
|
||||
name = (model_name or "").strip()
|
||||
if not name or codex.subscription_model(name):
|
||||
return False
|
||||
prefix, _, rest = name.partition("/")
|
||||
return bool(rest) and prefix.lower() not in {"openai", "any-llm"}
|
||||
|
||||
|
||||
def is_bedrock_route(model_name: str) -> bool:
|
||||
name = (model_name or "").strip().lower()
|
||||
return name.startswith("bedrock/") or "anthropic." in name
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ from strix.config.models import (
|
|||
is_openrouter_model,
|
||||
model_supports_reasoning,
|
||||
request_timeout_extra_args,
|
||||
routes_through_litellm,
|
||||
)
|
||||
from strix.core.sessions import scrub_images_from_items
|
||||
|
||||
|
|
@ -317,8 +318,13 @@ def _prompt_cache_extra_args(model_name: str) -> dict[str, Any] | None:
|
|||
it — elsewhere it leaks onto the wire and native Anthropic 400s). Unmapped
|
||||
Bedrock models get no points at all: Bedrock rejects the passed-through
|
||||
field outright.
|
||||
|
||||
The field is LiteLLM's own, consumed by its transform, so it only goes to
|
||||
routes LiteLLM serves. A bare ``claude-...`` name is served by the SDK's
|
||||
OpenAI client instead (a gateway in front of Claude), and that client raises
|
||||
``TypeError`` on request kwargs it does not know.
|
||||
"""
|
||||
if not is_claude_model(model_name):
|
||||
if not is_claude_model(model_name) or not routes_through_litellm(model_name):
|
||||
return None
|
||||
if is_bedrock_route(model_name) and not bedrock_route_supports_prompt_caching(model_name):
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -90,6 +90,16 @@ def test_make_model_settings_enables_prompt_cache_for_non_bedrock_claude(model_n
|
|||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model_name",
|
||||
["claude-sonnet-4-5", "openai/claude-sonnet-4-5", "any-llm/anthropic/claude-sonnet-4-5"],
|
||||
)
|
||||
def test_no_prompt_cache_for_claude_off_the_litellm_route(model_name: str) -> None:
|
||||
# These names are served by SDK clients that raise TypeError on LiteLLM-only
|
||||
# request kwargs — e.g. a gateway in front of Claude reached with a bare name.
|
||||
assert _cache_points(model_name) is None
|
||||
|
||||
|
||||
def test_tool_config_point_not_leaked_to_non_bedrock_claude() -> None:
|
||||
# LiteLLM only consumes tool_config on Bedrock; elsewhere it leaks onto the
|
||||
# wire and native Anthropic 400s.
|
||||
|
|
|
|||
|
|
@ -3,12 +3,17 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from agents.extensions.models.litellm_model import LitellmModel
|
||||
from agents.model_settings import ModelSettings
|
||||
|
||||
from strix.config.models import (
|
||||
RECOMMENDED_MODEL_NAMES,
|
||||
StrixProvider,
|
||||
_NonStreamingModel,
|
||||
_TurnGuardModel,
|
||||
is_recommended_or_frontier_model,
|
||||
request_timeout_extra_args,
|
||||
routes_through_litellm,
|
||||
supports_strict_tool_schemas,
|
||||
)
|
||||
|
||||
|
|
@ -112,3 +117,38 @@ def test_claude_routes_reject_strict_tool_schemas(model_name: str) -> None:
|
|||
)
|
||||
def test_other_routes_keep_strict_tool_schemas(model_name: str) -> None:
|
||||
assert supports_strict_tool_schemas(model_name)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("model_name", "litellm"),
|
||||
[
|
||||
("claude-sonnet-4-5", False),
|
||||
("openai/claude-sonnet-4-5", False),
|
||||
("any-llm/anthropic/claude-sonnet-4-5", False),
|
||||
("anthropic/claude-sonnet-4-5", True),
|
||||
("litellm/anthropic/claude-sonnet-4-5", True),
|
||||
("bedrock/anthropic.claude-sonnet-4-5-20250929-v1:0", True),
|
||||
("ollama/llama3", True),
|
||||
],
|
||||
)
|
||||
def test_routes_through_litellm_matches_the_provider(
|
||||
monkeypatch: pytest.MonkeyPatch, model_name: str, litellm: bool
|
||||
) -> None:
|
||||
"""The helper must agree with what StrixProvider actually builds.
|
||||
|
||||
Callers use it to decide whether a LiteLLM-only request field is safe to
|
||||
attach; on the SDK's own clients such a field raises TypeError mid-turn, so
|
||||
drift here breaks every request on that route.
|
||||
"""
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "test-key")
|
||||
assert routes_through_litellm(model_name) is litellm
|
||||
try:
|
||||
model = StrixProvider().get_model(model_name)
|
||||
except ImportError:
|
||||
# any-llm's client is an optional dependency; reaching it at all already
|
||||
# proves the route is not LiteLLM's.
|
||||
assert not litellm
|
||||
return
|
||||
while isinstance(model, _NonStreamingModel | _TurnGuardModel):
|
||||
model = model._inner
|
||||
assert isinstance(model, LitellmModel) is litellm
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue