/simplify follow-ups:
* Replace the two-``pop`` reach into ``cache_dict``/``ttl_dict`` with
the existing public ``InMemoryCache.delete_cache(key)`` — the same
idiom used elsewhere in the proxy. Bonus: ``delete_cache`` calls
``_remove_key`` which also handles ``expiration_heap`` consistency
the direct pops were silently leaking.
* JSON-encode the sorted scope list for the cache key instead of
``"|".join``. ``user_id`` / ``team_id`` / ``org_id`` / ``api_key``
are free-form strings and could contain a literal ``|`` — JSON
quoting escapes any in-string separator unambiguously.
* Extract ``_allowed_container_ids_cache_key()`` so the read and
invalidation sites compute the key the same way.
* Fix a placeholder-then-overwrite test construction: the
``__module__.split(".")[0] and "proxy_admin"`` line evaluated to a
literal string that was immediately overwritten with the real enum
value. Hoist the import and construct directly.
Address Greptile P2 follow-ups from the prior round:
* Cache ``_get_allowed_container_ids`` (60s LRU/TTL keyed by sorted
owner-scope tuple) so ``GET /v1/containers`` doesn't issue a fresh
``find_many`` against ``litellm_managedobjecttable`` on every list
call. Invalidate the caller's own cache entry when they record a
new owner so the just-created container shows up on their next list.
* Tighten the admin early-return in ``record_container_owner`` to skip
ONLY when there's literally no container ID to stamp. An admin with
identity (the master-key path populates ``user_id`` + ``api_key``)
flows through the normal record path so admin-created containers are
tracked like any other caller's. The truly-identity-less admin case
still falls through to the 403 below — correct fail-secure default.
Skill-cache invalidation gap (also flagged by Greptile) is moot: there
is no skill update endpoint exposed; ownership-affecting mutations are
only delete (already invalidates) and create (new ID, no cache entry
to update).
Substantial reduction (~765 LOC) without changing the security
boundary:
* Drop ContainerOwnershipStore and LiteLLMSkillsStore — both were
one-method-per-Prisma-call wrappers. Inline the calls instead,
matching the established pattern in vector_store_endpoints,
agent_endpoints, and mcp_server/db.py.
* Drop the prisma_client is None in-memory fallback. Production
deploys always have Prisma; running ownership-critical paths on a
process-local dict is a security footgun in the dev-mode case it
was meant to support, and complicates every code path with a
branch. Fail-secure: skip recording if Prisma is unavailable, and
treat reads as "not found" (admin-only).
* Drop the hand-rolled module-level cache. Replace with the existing
litellm.caching.in_memory_cache.InMemoryCache, which already has
TTL + max-size + eviction tested in its own module. Sentinel string
for negative caching since InMemoryCache can't disambiguate "miss"
from "cached as None".
* Tests: drop coverage for removed code paths (in-memory fallback,
hand-rolled cache internals). Keep tests for actual behavior (cache
hit-rate, negative caching, owner check, list filtering,
identity-less reject, admin bypass).
Two cleanups:
* ``LiteLLMSkillsHandler.create_skill`` raised ``HTTPException`` for
identity-less callers, importing FastAPI from a ``litellm/llms/``
module — that violates the project rule that FastAPI lives only
under ``proxy/``. Switch to ``ValueError`` (the same shape the rest
of the handler uses for not-found/forbidden) and update the test.
* The proxy-auth body bouncer derived its observability ban list from
``_supported_callback_params`` only, missing
``_request_blocked_callback_params`` (where ``gcs_bucket_name`` and
``gcs_path_service_account`` live). Two recently-merged sibling PRs
(#27019 added the deny list, #27081 added the test asserting these
are rejected at the request body root) crossed without folding them
together. Union the GCS deny list into the bouncer's derivation so
the single source of truth covers both code paths.
UNSCOPED_RESOURCE_OWNER_SCOPE collapsed every caller without an
identity field (no user_id / team_id / org_id / api_key / token) into
a single shared owner — a cross-tenant access primitive: any two such
callers could see and delete each other's containers and skills.
Drop the sentinel. ``get_primary_resource_owner_scope`` returns
``None`` and ``get_resource_owner_scopes`` returns ``[]`` for
identity-less callers. ``record_container_owner`` and
``LiteLLMSkillsHandler.create_skill`` now reject creates from
identity-less callers with a 403 instead of stamping the placeholder.
Read paths already deny ``owner is None`` correctly so legacy rows
(if any) are admin-only.
LITELLM_ALLOW_UNTRACKED_CONTAINER_ACCESS and
LITELLM_ALLOW_UNOWNED_SKILL_ACCESS were operator-toggleable opt-outs
for the cross-tenant access primitive this PR closes — flipping either
on re-enabled exactly the VERIA-20 read path. Default-secure with no
escape hatch matches sibling fixes (vector-store cred isolation, semantic
cache key isolation, user_config strip): all rejected the
opt-out-of-security pattern.
Untracked containers and unowned skills (rows that pre-date this
enforcement) are admin-only. Non-admin owners need to either re-create
via the now-tracked flow or have an admin assign ``created_by`` on the
existing row. Update tests to assert the strict-only behaviour.
Two cleanups from the /simplify pass:
* ``_CONTAINER_OWNER_CACHE`` and ``_SKILL_CACHE`` now LRU-evict via
``OrderedDict.popitem(last=False)`` instead of full ``clear()`` at
capacity. Full clears converted a steady-state cached workload into a
periodic full-DB-load oscillation as the cache repopulated from zero
and cleared again. Reads now ``move_to_end`` so the just-touched
entry survives the next eviction. Mirrors the pre-existing LRU
pattern in ``_remember_container_owner``.
* ``LiteLLM_ManagedObjectTable.file_purpose`` Literal now includes
``"container"`` so Pydantic validation accepts rows written by the
ownership store.
The flag was an opt-in escape hatch for the cross-tenant leak the rest
of the patch closes — flipping it on (env var or constructor param)
re-enables exactly the VERIA-54 primitive on either backend. There is
no operational need that the secure path doesn't already meet:
- Qdrant: legacy points without ``litellm_cache_key`` payload are
excluded by the must-clause filter and treated as misses; new sets
populate the cache key, so cold-start lasts only as long as the
natural cache rebuild.
- Redis: existing unscoped index can't carry the new schema; the init
path falls back to ``{name}_isolated`` (and recreates it on stale
schema), leaving the legacy index untouched.
Drop the constructor param, env-var fallback, ``_using_legacy_unscoped_index``
flag, the legacy-reuse branch in ``_init_semantic_cache``, and the
matching guards in set/get paths. Update tests to drop the legacy-mode
cases and assert the secure-only behaviour.
The hooks gated on ``call_type == "completion"`` but the proxy ingress
passes ``route_type`` straight through as ``call_type`` —
``"acompletion"`` for /v1/chat/completions and ``"aresponses"`` for
/v1/responses. Tests passed because they used the literal sync
``"completion"`` value, masking the gap.
Switch both hooks to ``is_text_content_call_type`` (matches the
canonical runtime values: completion / acompletion / aresponses) and
update existing tests to assert against runtime values, plus parametrize
a regression test that pins the gate.
Strip out the explanatory and historical comments that don't carry
business-logic justification. Comments that simply narrate what code
does — or that explain prior behavior, what was changed, or which PR
introduced a fix — are removed. Docstrings are reduced to a one-line
summary where the long form repeated information already evident from
the code or test data.
No code-behavior changes. All 643 affected unit tests still pass.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
`test_aaamodel_prices_and_context_window_json_is_valid` validates the
model-map JSON against an explicit schema with `additionalProperties`,
so the new `supports_adaptive_thinking` flag added in
98ced0ae43 needs a matching schema entry.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
xAI's chat completions API accounts reasoning_tokens separately from
completion_tokens, but rolls them into total_tokens. This breaks the
OpenAI invariant total_tokens == prompt_tokens + completion_tokens
that downstream consumers (including litellm's own _usage_format_tests
in tests/llm_translation/base_llm_unit_tests.py:58) rely on.
Live capture (grok-3-mini-beta, 2026-05-04):
prompt=14, completion=10, total=336, reasoning=312
14 + 10 = 24, NOT 336.
OpenAI's o1/o3 reasoning models include reasoning_tokens in
completion_tokens, leaving the prompt+completion=total invariant
intact. xAI deviates. This patch aligns xAI to OpenAI semantics by
folding reasoning_tokens into completion_tokens after the parent
OpenAI parser runs.
The fold is idempotent and defensive:
- Only fires when total_tokens == prompt_tokens + completion_tokens
+ reasoning_tokens (the documented xAI shape). Refuses to fold if
the gap doesn't match, guarding against silent corruption when xAI
changes accounting.
- Skips if completion_tokens already covers the gap (already
normalised — e.g. cost calc replays a previously-folded Usage).
xai.cost_calculator.cost_per_token already added reasoning_tokens to
the visible completion count for billing. Post-fold the Usage block
now satisfies that invariant directly, so the cost calc would
double-bill. Updated cost_per_token to detect the OpenAI-normalised
shape (total == prompt + completion) and skip the reasoning add-on
in that case, falling through to the legacy raw-shape behaviour for
callers that bypass the transformation (e.g. proxy log replay).
Tests:
- Adds TestXAIReasoningTokenFolding covering: gap-explained-fold,
idempotent-no-double-fold, no-reasoning-skip, gap-mismatch-skip.
- Adds test_already_normalised_usage_does_not_double_count_reasoning
to lock the cost-calc idempotency.
- Updates 7 pre-existing cost-calc tests whose total_tokens was
internally inconsistent (used the OpenAI-normalised total but kept
reasoning_tokens external) to use the documented xAI raw shape
total = prompt + visible completion + reasoning. Pre-existing
values masked the missing-fold by accident.
Verified end-to-end against the live xAI API:
LITELLM_LOCAL_MODEL_COST_MAP=False (CI default) +
XAI_API_KEY set +
pytest tests/llm_translation/test_xai.py::TestXAIChat::test_prompt_caching
-> PASSED in 18.81s (was: AssertionError on
usage.total_tokens == usage.prompt_tokens + usage.completion_tokens)
20/20 tests in tests/test_litellm/llms/xai/test_xai_cost_calculator.py
and 8/8 in tests/test_litellm/llms/xai/test_xai_chat_transformation.py
pass.
CI runs without LITELLM_LOCAL_MODEL_COST_MAP=True, so litellm.model_cost
is loaded from main-branch JSON (default model_cost_map_url) instead of
the PR's checked-out model_prices_and_context_window.json. Tests that
assert per-model flags added in this PR (supports_max_reasoning_effort,
supports_xhigh_reasoning_effort) therefore pass locally but fail in CI
with 'AssertionError: assert False is True' on 5 cases:
- test_anthropic_model_supports_effort_param_recognizes_supporting_models
[anthropic.claude-mythos-preview, bedrock/.../mythos-preview,
claude-opus-4-5-20251101]
- test_supports_effort_level_handles_provider_prefixes
[bedrock/invoke/us.anthropic.claude-sonnet-4-6-max-True,
claude-sonnet-4-6-max-True]
Add an autouse fixture at tests/test_litellm/llms/anthropic/chat/conftest.py
that monkey-patches litellm.model_cost to the PR-local JSON for every test
in this directory. The parent conftest already snapshots+restores
litellm.model_cost per-function, so the mutation is contained.
This is a scoped workaround. The proper fix is to set the env var
globally in the test workflow once the ~10 inline self-set test files
are audited; tracking that as a follow-up issue.
The chat completion path (`_apply_output_config`) and the /v1/messages
pass-through (`AnthropicMessagesConfig._translate_reasoning_effort_to_anthropic`)
both gate `max` / `xhigh` per model. The two sites had diverged from
near-identical copies into separately maintained blocks, creating a real
drift risk when a new model tier (e.g. Claude 4.8) lands -- a contributor
could update one site and miss the other.
Centralise the gating in `AnthropicConfig._validate_effort_for_model`,
which returns an error message string or `None`. Each call site keeps
its own provider-appropriate exception type (`BadRequestError` for the
chat path, `AnthropicError` for the /v1/messages pass-through) but the
gating decision now comes from one place. Net -11 LOC.
Adds a parametrised unit test exercising the helper directly across
4.5 / 4.6 / 4.7 model families and `max` / `xhigh` / lower-effort
inputs. Existing tests at both call sites continue to pass unchanged.
Addresses Greptile finding on PR #27074.
`azure_ai` is registered in `litellm.openai_compatible_providers`, so
`add_provider_specific_params_to_optional_params` (litellm/utils.py)
auto-stuffs any non-OpenAI kwarg (e.g. `output_config={"effort": "..."}`)
into `optional_params["extra_body"]`. `AzureAnthropicConfig.transform_request`
then strips `extra_body` entirely on the way out, silently dropping the
param — and `AnthropicConfig._apply_output_config` never sees it, so
`effort="invalid"` / `effort="xhigh"` on a non-supporting model
quietly reaches the model with default behavior instead of returning a
clean 400 (as the native `anthropic` provider does).
Promote the keys back to top-level `optional_params` (using `setdefault`
so explicit top-level values win) before delegating to the parent
`AnthropicConfig`. Apply in both `validate_environment` and
`transform_request` so flag detection (`is_mcp_server_used`, etc.) and
output-config validation both run.
Surfaced by the QA matrix expansion on PR #27074: 20 cells where Azure
returned 200 while `anthropic` returned 400 — all `output_config` mode
across haiku_4_5, sonnet_4_5, opus_4_5, sonnet_4_6, opus_4_6, opus_4_7
families with `effort` in {invalid, xhigh, max, low, medium, high}.
Tests:
* `test_output_config_promoted_from_extra_body`: valid effort reaches data
* `test_invalid_output_config_effort_raises_via_extra_body`: 400 on bad effort
* `test_unsupported_effort_xhigh_raises_via_extra_body`: 400 on xhigh-on-Sonnet-4.6
* `test_extra_body_promotion_does_not_clobber_top_level`: setdefault semantics
- Image generation tests patch HTTPHandler.post / get_async_httpx_client so
make_*_azure_httpx_request runs and wire json is asserted on call kwargs.
- Azure image edit: strip model in finalize_image_edit_multipart_data using the
same URL string the handler passes to POST (no second get_complete_url in
transform). BaseImageEditConfig default finalize is a no-op.
Co-authored-by: Cursor <cursoragent@cursor.com>
