From cc2b3351b8d2ebac35c7f258fc2320ac29d2d08c Mon Sep 17 00:00:00 2001 From: seanturner83 Date: Thu, 16 Jul 2026 14:22:25 +0100 Subject: [PATCH] fix(llm): don't inject prompt-cache marker for unmapped Bedrock Claude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache breakpoints are gated on _is_claude_model (name contains "claude"), but LiteLLM's AnthropicCacheControlHook only *consumes* cache_control_injection_points for models it recognises as cache-capable via its statically bundled model map. On a Bedrock route whose model isn't in that map, the marker passes straight through and Bedrock's Converse API rejects it outright: ValidationException: cache_control_injection_points: Extra inputs are not permitted — which fails the whole scan at the first LLM call. This bites any Bedrock Claude model LiteLLM hasn't mapped yet (a just-released model), and is made worse when LiteLLM can't refresh its remote model map (e.g. behind a TLS-intercepting corporate proxy) and falls back to a stale local copy. Observed live on bedrock/global.anthropic.claude-sonnet-5. Fix: withhold the marker only for a Bedrock route LiteLLM can't confirm supports prompt caching. Scope is deliberately narrow — Anthropic-native, Vertex, and OpenRouter Claude tolerate/ignore the marker (or LiteLLM maps them under keys we don't resolve), so gating those on confirmed support would DISABLE caching for capable models — the opposite of this PR's intent. Only Bedrock hard-rejects, so only Bedrock is guarded. Co-Authored-By: Claude Opus 4.8 (1M context) --- strix/core/inputs.py | 69 +++++++++++++++++++++++++++++++++++++++++++- tests/test_inputs.py | 41 ++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/strix/core/inputs.py b/strix/core/inputs.py index f78bb9e4..3c6592c5 100644 --- a/strix/core/inputs.py +++ b/strix/core/inputs.py @@ -141,7 +141,7 @@ def make_model_settings( ) if force_required_tool_choice and _accepts_required_tool_choice(model_name): model_settings = model_settings.resolve(ModelSettings(tool_choice="required")) - if _is_claude_model(model_name): + if _is_claude_model(model_name) and not _bedrock_route_without_cache_support(model_name): # Merge into any existing extra_args rather than relying on resolve()'s # dict-merge semantics — makes it obvious at the call site that unrelated # LiteLLM options are preserved (make_model_settings currently builds @@ -161,6 +161,73 @@ def _is_claude_model(model_name: str) -> bool: return "claude" in (model_name or "").strip().lower() +def _litellm_name_candidates(model_name: str) -> list[str]: + """Candidate LiteLLM model-map keys for ``model_name``, most→least specific. + + LiteLLM keys the same model under several names (``bedrock/global.anthropic. + claude-opus-4-1``, ``anthropic.claude-opus-4-1``, ``claude-opus-4-1``) and not + every provider/region-prefixed variant is present for every model. Strip the + LiteLLM route prefix, then leading dotted segments (region, then provider) so + a prefixed name still resolves to a bare key. + """ + name = (model_name or "").strip().lower() + for prefix in ("litellm/", "bedrock/"): + if name.startswith(prefix): + name = name[len(prefix) :] + break + candidates = [name] + for cand in list(candidates): + rest = cand + while "." in rest: + rest = rest.split(".", 1)[1] + candidates.append(rest) + return candidates + + +def _bedrock_route_without_cache_support(model_name: str) -> bool: + """True for a BEDROCK Claude route that LiteLLM can't confirm supports prompt + caching — the one case where injecting the cache marker HARD-CRASHES the run. + + Bedrock's Converse API rejects unknown request fields outright + (``ValidationException: cache_control_injection_points: Extra inputs are not + permitted``). LiteLLM's ``AnthropicCacheControlHook`` strips + ``cache_control_injection_points`` from the outgoing call only for models it + recognises as cache-capable via its (statically bundled) model map; for a + model missing from that map the marker passes straight through and Bedrock + 500s the first call, failing the whole scan. This bites any Bedrock Claude + model LiteLLM hasn't mapped yet — a just-released model, or ANY model when + LiteLLM can't refresh its remote model map (e.g. behind a TLS-intercepting + corporate proxy) and falls back to a stale local copy. + + Scope is deliberately narrow — ONLY Bedrock routes. Anthropic-native, + Vertex, and OpenRouter Claude tolerate/ignore the marker (or LiteLLM maps + them under keys we don't resolve), so gating those on confirmed support + would DISABLE caching for genuinely-capable models — a caching regression, + the opposite of this change's intent. So elsewhere we keep injecting by + model family and only withhold on the provider that actually rejects. + """ + name = (model_name or "").strip().lower() + if not name.startswith("bedrock/") and "anthropic." not in name: + # Not a Bedrock route (bedrock/... or a bare bedrock model id like + # global.anthropic.claude-...); other providers don't hard-reject. + return False + + import litellm + + checker = getattr(getattr(litellm, "utils", None), "supports_prompt_caching", None) + for cand in _litellm_name_candidates(model_name): + if checker is not None: + try: + if checker(cand): + return False # confirmed cache-capable → safe to inject + except Exception: # noqa: BLE001 — unknown model raises; keep checking + pass + entry = litellm.model_cost.get(cand) + if entry and entry.get("supports_prompt_caching"): + return False + return True # Bedrock route, support unconfirmed → withhold to avoid the 500 + + def _claude_prompt_cache_extra_args() -> dict[str, Any]: """Enable Anthropic/Bedrock prompt caching for Claude models via LiteLLM. diff --git a/tests/test_inputs.py b/tests/test_inputs.py index 52d2bcb6..3f9c4e70 100644 --- a/tests/test_inputs.py +++ b/tests/test_inputs.py @@ -84,6 +84,47 @@ def test_make_model_settings_no_prompt_cache_for_non_claude(model_name: str) -> assert make_model_settings(None, model_name=model_name).extra_args is None +def test_no_prompt_cache_for_unmapped_bedrock_claude_model(monkeypatch: Any) -> None: + """A BEDROCK Claude route LiteLLM has NOT mapped (a new release, or any model + when LiteLLM can't refresh its model map and falls back to a stale local + copy) must run UNCACHED, not crash. Bedrock's Converse API rejects the + unknown field outright (ValidationException 'cache_control_injection_points: + Extra inputs are not permitted'); LiteLLM only strips the marker for models + it recognises as cache-capable, so an unmapped model would 500 the first + call and fail the whole run.""" + import litellm + + unmapped = "bedrock/global.anthropic.claude-brand-new-9" + # Simulate a model LiteLLM doesn't know: no cost-map entry, checker says no. + monkeypatch.setattr(litellm, "model_cost", {}, raising=False) + if getattr(getattr(litellm, "utils", None), "supports_prompt_caching", None): + monkeypatch.setattr(litellm.utils, "supports_prompt_caching", lambda *_a, **_k: False) + + # Bedrock Claude by name, but unmapped → no injection points, no crash. + assert make_model_settings(None, model_name=unmapped).extra_args is None + + +def test_prompt_cache_kept_for_non_bedrock_claude_even_if_unmapped(monkeypatch: Any) -> None: + """Non-Bedrock Claude routes must KEEP caching-by-family even when LiteLLM + can't confirm support — those providers tolerate/ignore the marker (or + LiteLLM maps them under keys we don't resolve, e.g. OpenRouter), so gating + them on confirmed support would DISABLE caching for capable models — a + regression. Only Bedrock hard-rejects, so only Bedrock is guarded.""" + import litellm + + monkeypatch.setattr(litellm, "model_cost", {}, raising=False) + if getattr(getattr(litellm, "utils", None), "supports_prompt_caching", None): + monkeypatch.setattr(litellm.utils, "supports_prompt_caching", lambda *_a, **_k: False) + + # Even with LiteLLM knowing nothing, an Anthropic-native / OpenRouter Claude + # still gets the injection points. + for model in ("anthropic/claude-brand-new-9", "openrouter/anthropic/claude-brand-new"): + assert _cache_points(model) == [ + {"location": "message", "role": "system"}, + {"location": "tool_config"}, + ] + + def test_build_root_task_empty_config() -> None: assert build_root_task({}) == ""