chore(release): backport pending 1.84.x/1.85.x fixes into stable/1.86.x and cut 1.86.3 - #29541
Merged
Conversation
* Add day 0 support for gemini 3.5 flash * Fix pricing * Fix greptile review * Fix failing test * Fix tests * Fix: revert tool removing logic * fix greptile and test --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> (cherry picked from commit 3c3d131) (cherry picked from commit cbf9ffe)
…d double-seed (#27854) * fix(spend_counter): seed Redis counter via SET NX to prevent cross-pod double-seed Symptom ------- Customers on multi-pod deployments see team `spend` jump to ~2x (or N x the pod count) shortly after a Redis cache miss / TTL expiry, triggering spurious "Budget Crossed" alerts and blocked requests until the value is manually reset. Root cause ---------- `SpendCounterReseed.coalesced` warmed the primary spend counter by calling `redis.async_increment(key, value=db_spend, refresh_ttl=True)`, which lowers to Redis `INCRBYFLOAT`. That is additive, not idempotent. The per-counter `asyncio.Lock` only coalesces seeders inside one process. With N pods sharing one Redis, on a cold key (cold start, TTL expiry, manual delete) every pod independently passes its lock + Redis re-check, reads the same `db_spend`, and issues `INCRBYFLOAT db_spend`. Final value: N x db_spend. Fix --- Use `redis.async_set_cache(key, value=db_spend, nx=True)` for the seed. SET NX is atomic across pods: exactly one writer initializes the key; losers read the winner's value via `async_get_cache`. This is the same idiom already used by `coalesced_window` in the same file, so the two seed paths are now consistent. Per-request deltas continue to use `INCRBYFLOAT` (correct - additive behaviour is what we want for increments, not for initial seed). Verification ------------ Live two-process repro against the same Postgres + Redis (DB spend = 506): Unpatched: 4/4 runs -> Redis counter = ~1012 (~2 x db_spend) Patched: 12/12 runs -> Redis counter = ~506 Unit tests (`test_proxy_server.py`): - New `test_primary_spend_counter_redis_concurrent_seed_does_not_double_seed` patches `_get_lock` to return a fresh lock per caller (otherwise the per-process lock masks the race), races two `coalesced` calls, and asserts final = 506 with exactly one of two SET NX attempts winning. - 4 existing tests updated for the new seed contract (SET NX for the seed, INCRBYFLOAT only for the per-request delta). - Full `spend_counter or reseed or budget` slice: 22 passed. Co-authored-by: Cursor <cursoragent@cursor.com> * test(spend_counter): make SET NX mock atomic so loser branch is exercised Greptile flagged that `redis_set_cache` in test_primary_spend_counter_redis_concurrent_seed_does_not_double_seed placed `await asyncio.sleep(0)` AFTER the NX membership check. Both concurrent tasks observed an empty `redis_store`, passed the guard, and both returned True - so the loser branch (else: read back winner's value) was never exercised. Fix the mock to model real atomic Redis SET NX: - Yield BEFORE the membership check so two concurrent callers interleave the way real SET NX does (first to resume runs check + write atomically and wins; second resumes after the key exists and loses). - Track set_cache return values; assert sorted([loser, winner]) so we know exactly one task wins and one loses. - Track async_get_cache calls that happen AFTER at least one SET NX has completed; assert at least one such read - that is the loser-path fallback (`current_value = float(cached)` when seeded is False). Verified by temporarily reverting the mock to the old order: the test now fails with `expected exactly one SET NX winner and one loser, got [True, True]`, exactly the failure mode Greptile described. No production code change. Co-authored-by: Cursor <cursoragent@cursor.com> * test(spend_counter): mock async_set_cache to populate redis_store in concurrent read+write test `test_concurrent_read_and_write_paths_share_one_db_query` mocks `async_increment` to populate the in-memory `redis_store`, but did not mock `async_set_cache`. After the SET-NX seed change in `coalesced()`, the seed step writes via `async_set_cache(nx=True)` (default AsyncMock, no `redis_store` write), so the simulated Redis stays empty after the first reseed. The second `get_current_spend` then sees a clean Redis miss, re-enters the DB read path, and the test fails with `expected 1 DB query, got 2`. Fix: add a `redis_set_cache` side_effect that updates `redis_store` on `nx=True` (and rejects when the key already exists), matching the pattern used by the four sibling tests fixed in this branch's first commit. Pre-existing assertions are unchanged. Full `tests/test_litellm/proxy/test_proxy_server.py`: 158 passed. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 0fb7104) (cherry picked from commit c621f58)
#28324) * fix(vertex_ai): omit function_call id on Vertex Gemini 3.5+ tool turns Vertex AI rejects `id` on function_call/function_response parts; only Google AI Studio accepts it for Gemini 3.5+ strict tool matching. Co-authored-by: Cursor <cursoragent@cursor.com> * Update litellm/llms/vertex_ai/gemini/vertex_and_google_ai_studio_gemini.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * fix(vertex_ai): forward custom_llm_provider in context caching Pass custom_llm_provider through to _gemini_convert_messages_with_history in the context caching path so Gemini 3.5+ tool-call `id` forwarding behaves consistently between cached and non-cached completions on Google AI Studio. Co-authored-by: Claude <claude@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <claude@anthropic.com> (cherry picked from commit fecf212) (cherry picked from commit 75c72c5)
(cherry picked from commit fcdf023)
…9343) * refactor(proxy/auth): normalize Bearer prefix in safe-hash helper UserAPIKeyAuth._safe_hash_litellm_api_key now strips a leading "Bearer "/"bearer " prefix before its existing sk-/JWT classification, so the helper produces the same hashed output regardless of whether the caller stripped the Authorization header prefix or passed the header value through unchanged. * refactor(proxy/auth): make Bearer-prefix strip case-insensitive Per RFC 7235 the HTTP authorization scheme token is case-insensitive. Replace the two-prefix loop with a single case-insensitive check so the helper normalizes "Bearer ", "bearer ", "BEARER ", and any mixed-case variant before classifying the remainder as sk- or JWT. The contract test gains coverage of "BEARER " and "BeArEr ". * test(mcp): align auth-handler test expectations with safe-hash helper The two MCP auth tests asserted that UserAPIKeyAuth(api_key="Bearer ...") retained the raw header bytes on the api_key field. _safe_hash_litellm_api_key now normalizes that input — stripping the Bearer prefix and hashing the resulting sk- key — so the expectations move to the normalized form: the bare token in the parametrize case, and hash_token("sk-...") in the backward-compat assertion. This matches what the real auth flow produces (the builder strips Bearer and the DB stores the hashed token), so the mocks now line up with production rather than with the un-normalized validator output. (cherry picked from commit 87b0e47)
…eroing counter (#29358) * fix(reset_budget): write only {spend, budget_reset_at} and stop pre-zeroing counter ResetBudgetJob's batched update_data path shipped the full key/user/team model on each reset. Prisma rejects object_permission_id and budget_limits on the update input type, so any row carrying those fields detonated the entire batch -- spend never reset, budget_reset_at never advanced. After v1.84.0 started populating object_permission_id on UI-created keys, this fires routinely. _reset_budget_common also zeroed the cross-pod spend counter before the DB write, so failed resets left enforcement reading 0 from the counter while the DB still held the over-budget spend, admitting requests past the cap until the counter naturally re-saturated from new reservations. Switch the write to per-row narrow updates ({spend, budget_reset_at}) via db.batch_, and move the counter invalidation out of _reset_budget_common so it only fires after the DB write commits. On DB-write failure the counter is left untouched, enforcement continues to block, and the next scheduler tick can retry without leaving a bypass window. Fixes #27730. * fix(reset_budget): address Greptile review on #29358 - Strengthen the bypass-half regression test: replace the for-loop over call_args_list (vacuously true when empty) with assert_not_called(), so the test would actually flag a re-introduction of counter-zeroing via any code path. - Add the same explanatory docstring on _write_user_reset_updates and _write_team_reset_updates that _write_key_reset_updates already has, so all three helpers point future maintainers at #27730. * test(reset_budget): update test_proxy_budget_reset for new batch-write path Same shape as the previous test_reset_budget_job.py update: keys/users/teams now write through prisma.db.batch_().<table>.update, not update_data, so the tests need a batcher mock and updated assertions. Adds: - _wire_batcher_for_test helper that returns a list which accumulates per-row batch updates captured from prisma_client.db.batch_(). - _attrify helper that wraps dict fixtures so getattr(item, "token") works alongside the dict item-access the fake_reset_* mocks rely on. The new narrow-write helpers use getattr to pull out the row's id, and would silently skip plain dicts otherwise. - Updates 3 partial_failure tests to assert against the batch-call list (rows by id, payload contains only {spend, budget_reset_at}) instead of update_data.assert_awaited_once + data_list inspection. - Updates test_reset_budget_continues_other_categories_on_failure: only budget + enduser still flow through update_data; key/user/team go through the batch path now. - Wires the batcher mock into 3 service_logger_*_success tests so commit() is actually awaitable and the success hook fires. These tests were silently passing locally only because the editable install in .venv pointed at the main repo, not the worktree — running pytest with PYTHONPATH overridden to the worktree (matching CI) reproduces the failures. (cherry picked from commit a06ec43)
…quest body (#29447) * fix: stop use_chat_completions_api flag from leaking into provider request body use_chat_completions_api is a LiteLLM control flag that forces the /responses -> /chat/completions bridge. It was missing from all_litellm_params, so get_non_default_completion_params treated it as a model-specific param and forwarded it to the upstream provider. A model-level "use_chat_completions_api: true" in the proxy config therefore reached the chat-completions path and was rejected by strict providers (OpenAI/Anthropic) with HTTP 400 for an unknown body field. Register it as a known internal param so it is stripped on every path (completion, the responses bridge that calls litellm.completion, and filter_out_litellm_params). Adds a regression test driving litellm.completion() with a mocked OpenAI client that asserts the flag never reaches the request body. * test: clarify extra_body assertion in use_chat_completions_api leak test Replace the misleading 'not in ... or {}' precedence idiom with an explicit parenthesized guard that also handles extra_body being None. (cherry picked from commit acbbfe9)
The #29311 cherry-pick onto stable/1.85.x carried the test test_route_streaming_logging_runs_async_handler_for_sdk_passthrough, which patches PassThroughStreamingHandler._build_passthrough_logging_result to verify the SDK-passthrough dispatch contract. The manual conflict resolution kept the per-endpoint if/elif/elif chain inline in _route_streaming_logging_to_handler (matching v1.84.4's resolution), so the patched attribute did not exist and the test errored at collection with AttributeError. Extract the chain into the static _build_passthrough_logging_result helper as #29089 originally designed it. _route_streaming_logging_to_handler now resolves (standard_logging_response_object, kwargs) through the helper and dispatches via dispatch_success_handlers; the helper itself is synchronous and CPU-bound, suitable for the unit test's patch target. Verified locally: tests/pass_through_unit_tests/test_unit_test_streaming.py passes (5/5) and tests/test_litellm/litellm_core_utils/test_litellm_logging.py passes (83/83). v1.84.4 ships with the same broken test; this strictly improves on that resolution. (cherry picked from commit 8824745)
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
Greptile SummaryThis PR backports seven fixes from the
Confidence Score: 4/5The changes are cherry-picked from already-released stable lines and carry their own tests; the core correctness of each fix is well-established. All seven commits are verbatim backports already running in production on 1.84.x/1.85.x. The
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/litellm_logging.py | Adds dispatch_success_handlers as a unified routing method with a has_dispatched_final_stream_success dedup guard; extracts _is_sync_litellm_request helper; modifies async_success_handler guard to allow assembled stream responses to bypass the should_run_logging check |
| litellm/litellm_core_utils/streaming_handler.py | Per-chunk sync success_handler now skipped for async SDK requests; final assembled response switches from paired async_success_handler + executor.submit to dispatch_success_handlers(prefer_async_handlers=True) |
| litellm/proxy/common_request_processing.py | Deferred stream logging paths (orphaned + guardrail) consolidated onto dispatch_success_handlers(prefer_async_handlers=True); sync executor.submit blocks removed |
| litellm/proxy/common_utils/reset_budget_job.py | Adds _write_{key,user,team}_reset_updates helpers that issue targeted Prisma updates for only {spend, budget_reset_at}; spend-counter invalidation moved to after the DB commit via _invalidate_spend_counter |
| litellm/proxy/db/spend_counter_reseed.py | Seeds Redis counter via SET NX instead of INCRBYFLOAT, preventing concurrent pods from multiplying the counter; loser pod reads the winner's value; return type changed from db_spend to current_value |
| litellm/proxy/_types.py | Bearer prefix stripping before token hashing in UserAPIKeyAuth.get_cache_key; normalizes keys arriving with Authorization: Bearer sk-... headers |
| litellm/llms/vertex_ai/gemini/vertex_and_google_ai_studio_gemini.py | Adds Gemini 3.5 Flash model support; introduces _forward_gemini_function_call_id gating tool-call id to Google AI Studio only; adds top_k to supported params; replaces parallel_tool_calls validation with silent drop when value=False and multiple tools are present |
| litellm/types/utils.py | Adds use_chat_completions_api to all_litellm_params so it is recognized as an internal flag and stripped before the request body reaches providers |
Comments Outside Diff (1)
-
litellm/llms/vertex_ai/gemini/vertex_and_google_ai_studio_gemini.py, line 1249-1265 (link)parallel_tool_callsdrop no longer respectsdrop_paramsThe old guard raised a validation error when
parallel_tool_calls=Falsewas sent withoutdrop_params=True. The new logic silently drops the param whenevervalue is Falseand there is more than one tool, regardless ofdrop_params. Clients that intentionally setparallel_tool_calls=Falsewith multiple tools on Gemini — and expect an error rather than a silent no-op — will now get an unacknowledged drop. This is intentional per the PR description (Responses API clients send this flag by default), but it bypasses the user-controlleddrop_paramsopt-in that the rest of the codebase uses for unsupported-param handling.
Reviews (1): Last reviewed commit: "chore: update uv.lock for 1.86.3" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
1808
to
+1812
| processed_chunk, None, None, cache_hit | ||
| ) | ||
| ) | ||
| ## SYNC LOGGING | ||
| self.logging_obj.success_handler(processed_chunk, None, None, cache_hit) | ||
| ## SYNC LOGGING — only for sync SDK entrypoints; async proxy paths export via async_success_handler | ||
| litellm_params = self.logging_obj.model_call_details.get("litellm_params", {}) |
Contributor
There was a problem hiding this comment.
success_handler is now only called per-chunk for sync SDK entrypoints. Any user who registers a sync callback (via litellm.success_callback) and consumes a streaming response through acompletion will no longer receive per-chunk notifications — only the final assembled response reaches sync callbacks via executor.submit inside dispatch_success_handlers. This is intentional for the duplicate-trace fix, but it's a quiet behaviour change for sync-callback users on async streaming paths.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
414
to
+440
| ) | ||
| return [LiteLLM_EndUserTable(**row.dict()) for row in rows] | ||
|
|
||
| async def _write_key_reset_updates( | ||
| self, updated_keys: List[LiteLLM_VerificationToken] | ||
| ) -> None: | ||
| """ | ||
| Write per-row {spend, budget_reset_at} updates for keys. | ||
|
|
||
| Avoids the batched full-model update path, which trips | ||
| prisma.errors.DataError on any row carrying object_permission_id or | ||
| budget_limits (see #27730). Both fields are rejected by Prisma's | ||
| update input type for LiteLLM_VerificationToken, and the failure | ||
| aborts the entire batch — silently leaving spend over the cap and | ||
| budget_reset_at unchanged forever. | ||
| """ | ||
| batcher = self.prisma_client.db.batch_() | ||
| for k in updated_keys: | ||
| token = getattr(k, "token", None) | ||
| if token is None: | ||
| continue | ||
| batcher.litellm_verificationtoken.update( | ||
| where={"token": token}, | ||
| data={"spend": 0, "budget_reset_at": k.budget_reset_at}, | ||
| ) | ||
| await batcher.commit() | ||
|
|
Contributor
There was a problem hiding this comment.
spend written as integer 0 instead of float 0.0
All three _write_*_reset_updates helpers pass data={"spend": 0, "budget_reset_at": ...} with an integer zero while the rest of the budget path uses 0.0 (e.g. item.spend = 0.0 in _reset_budget_item). Prisma accepts both, but the type mismatch could surface if any downstream code does a strict float comparison or if the Prisma schema enforces the type more strictly in a future migration.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
ryan-crabbe-berri
approved these changes
Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
Backports changes that already shipped in the 1.84.x and 1.85.x lines but never made it into 1.86.x. 1.86.2 was branched on May 16 and only ever received the npm builder fix (#28519) and the proxy path helper (#28547), so a handful of fixes that 1.84.4 and 1.85.3 carry were missing on the current stable line. This PR closes that gap and cuts 1.86.3
Linear ticket
N/A
What is included
Every commit here is cherry-picked verbatim from
stable/1.85.x(the same changes that shipped in 1.85.3 and 1.85.1), so the set matches what customers on 1.84.x and 1.85.x already run:idon Vertex Gemini 3.5+ tool turns. Vertex AI rejects theidthat Google AI Studio requires for strict tool matching, so forwarding is now scoped to AI Studio only. This is the follow-up that pairs with Day 0 support : Gemini 3.5 Flash #28268; shipping Day 0 support : Gemini 3.5 Flash #28268 without it breaks multi-turn tool calling on Vertex for every Gemini 3+ model, so the two travel together_build_passthrough_logging_resulthelper extraction it rides with{spend, budget_reset_at}and no longer pre-zeroes the counteruse_chat_completions_apiflag from leaking into the provider request bodyThe last two commits are the
cz bump --increment PATCH(1.86.2 -> 1.86.3) and the matchinguv lockPre-Submission checklist
make test-unitCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
These are verbatim backports of changes already merged, released, and validated on the 1.84.x and 1.85.x lines; each linked PR carries its own proof and review. The Gemini pair (#28268 with its regression fix #28324) is included together precisely so 1.86.x does not regress Vertex tool calling
Type
🆕 New Feature
🐛 Bug Fix
🚄 Infrastructure
Changes
See the commit list above. No new code is introduced beyond the cherry-picks, the version bump, and the lockfile refresh