Skip to content

Commit d152f56

Browse files
committed
fix(h2o): gate the rename to chat completions and to max_tokens-discarding routes
Two review findings, both confirmed by execution on the 1.93.0 tree this targets. Blocker: the hook broke /v1/messages on Azure-routed models. The deployment-hook dispatch is not chat-specific, it runs for every @client-decorated async entrypoint, and litellm.anthropic_messages declares max_tokens as a REQUIRED parameter. Popping it made litellm's own wrapper raise TypeError on the following await original_function(...), outside this hook's try/except, with a traceback that never names the hook. Reproduced against a real install: identical call, OK without the hook registered, TypeError with it. Gate on call_type so only completion/acompletion are rewritten. This also covers atext_completion, whose endpoint has no max_completion_tokens, and aembedding. Non-blocking: a configured max_completion_tokens is no longer a trigger on its own. convert_model_to_litellm_config sets it for every reasoning model, Azure or not, and only the Azure branch adds the drop, so the old predicate would strip max_tokens from non-Azure providers and leave those requests with no ceiling at all. Trigger on the drop list or an Azure route instead, which keeps the drop-less reasoning-Azure case firing and closes the previously documented caller-supplied-ceiling edge. Zero non-Azure entries reach this today, so the change is latent-only. Verified after the change: anthropic_messages OK, acompletion still renames max_tokens=50 to max_completion_tokens=50. Both new tests fail on the previous head. Tests: 22.
1 parent 32c89a5 commit d152f56

2 files changed

Lines changed: 126 additions & 24 deletions

File tree

litellm/integrations/h2o/litellm_max_tokens_rename_hook.py

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,33 +46,44 @@
4646
-----
4747
Fires only when BOTH hold for the deployment actually selected:
4848
49+
* the call is a chat completion, and
4950
* the request carries a positive integer `max_tokens`, and
50-
* that deployment expects `max_completion_tokens` (it has one configured,
51-
or `max_tokens` in its `additional_drop_params`).
51+
* the selected deployment is one that discards or rejects `max_tokens`,
52+
i.e. it lists `max_tokens` in `additional_drop_params`, or it is an Azure
53+
route.
54+
55+
A configured `max_completion_tokens` is deliberately NOT a trigger on its own.
56+
`convert_model_to_litellm_config` sets it for every reasoning model, Azure or
57+
not (`use_completion_tokens = is_reasoning_model or is_azure_provider`), and
58+
only the Azure branch adds the drop. Treating it as a trigger would strip
59+
`max_tokens` from non-Azure providers that need it, leaving those requests with
60+
no ceiling at all, which is worse than the bug being fixed. It also cannot be
61+
distinguished from a caller-supplied value once kwargs are merged.
5262
5363
Deployments that natively accept `max_tokens` (Anthropic, Bedrock, vLLM,
5464
non-2025 Azure) are left untouched, including when they share a model group
5565
with an Azure deployment.
5666
5767
Two scope limits worth knowing:
5868
59-
* ASYNC PATH ONLY. `async_pre_call_deployment_hook` is dispatched by the
60-
@client decorator's ASYNC wrapper (litellm/utils.py, in wrapper_async).
61-
The sync wrapper does not dispatch it, so a direct sync
69+
* CHAT COMPLETIONS ONLY, enforced via `call_type`. The dispatch is NOT
70+
chat-specific: `wrapper_async` runs it for every @client-decorated async
71+
entrypoint. `litellm.anthropic_messages` (which backs /v1/messages, and
72+
which bridges Azure and other non-Anthropic providers) declares
73+
`max_tokens: int` as a REQUIRED parameter, so popping it there makes
74+
litellm's own wrapper raise
75+
`TypeError: anthropic_messages() missing 1 required positional argument`
76+
on the following `await original_function(*args, **kwargs)`. That is
77+
outside this hook's try/except and cannot be caught here, and the
78+
traceback never names this hook. `atext_completion` (/v1/completions has
79+
no `max_completion_tokens`) and `aembedding` reach the same dispatch.
80+
81+
* ASYNC PATH ONLY. The dispatch lives in the @client decorator's ASYNC
82+
wrapper; the sync wrapper does not dispatch it, so a direct sync
6283
`litellm.completion()` or `Router._completion()` bypasses this hook. That
6384
is fine for how the hook is deployed: it is registered only in the proxy
6485
config, and the proxy maps /chat/completions to `acompletion`
65-
(proxy/route_llm_request.py), so all proxied traffic takes the async path.
66-
Code embedding litellm in-process and calling sync `completion()` would
67-
not get the rename.
68-
69-
* The predicate reads the MERGED kwargs, which cannot distinguish a
70-
deployment-configured `max_completion_tokens` from a caller-supplied one.
71-
A request sending BOTH `max_tokens` and `max_completion_tokens` to a
72-
max_tokens-native deployment is therefore rewritten. The reported case,
73-
and everything h2oGPTe core emits, sends `max_tokens` alone. Keying only
74-
on `additional_drop_params` would remove this edge, at the cost of not
75-
firing for an Azure deployment configured with a ceiling but no drop.
86+
(proxy/route_llm_request.py), so all proxied traffic is async.
7687
7788
INTERACTION WITH THE CAP HOOK
7889
-----------------------------
@@ -122,25 +133,45 @@ def _positive_int(value: Any) -> Optional[int]:
122133
return value if value > 0 else None
123134

124135
@staticmethod
125-
def _deployment_uses_completion_tokens(kwargs: Dict[str, Any]) -> bool:
126-
"""True when the SELECTED deployment expects `max_completion_tokens`.
136+
def _is_chat_completion(call_type: Any) -> bool:
137+
"""True only for the chat-completion call types.
138+
139+
`call_type` is a `CallTypes` enum member (or None for an unrecognised
140+
entrypoint), so compare on its value and tolerate a bare string.
141+
"""
142+
return getattr(call_type, "value", call_type) in ("completion", "acompletion")
143+
144+
@staticmethod
145+
def _deployment_discards_max_tokens(kwargs: Dict[str, Any]) -> bool:
146+
"""True when the SELECTED deployment would discard or reject
147+
`max_tokens`, so the caller's limit only survives as
148+
`max_completion_tokens`.
127149
128150
Read straight off the merged kwargs rather than the router, so a mixed
129151
model group is judged per selected deployment instead of per group.
130152
"""
131-
if kwargs.get("max_completion_tokens") is not None:
132-
return True
133153
drop = kwargs.get("additional_drop_params") or []
134-
return isinstance(drop, (list, tuple)) and "max_tokens" in drop
154+
if isinstance(drop, (list, tuple)) and "max_tokens" in drop:
155+
return True
156+
# Azure 2025+ rejects max_tokens outright. Reasoning Azure entries are
157+
# exempt from the drop above but still need the rename, so recognise
158+
# the route itself. custom_llm_provider is not always populated at this
159+
# point, so the prefixed model string is the primary signal.
160+
if kwargs.get("custom_llm_provider") == "azure":
161+
return True
162+
model = kwargs.get("model")
163+
return isinstance(model, str) and model.startswith("azure/")
135164

136165
async def async_pre_call_deployment_hook(
137166
self, kwargs: Dict[str, Any], call_type: Any
138167
) -> Optional[dict]:
139168
try:
169+
if not self._is_chat_completion(call_type):
170+
return None
140171
requested = self._positive_int(kwargs.get("max_tokens"))
141172
if requested is None:
142173
return None
143-
if not self._deployment_uses_completion_tokens(kwargs):
174+
if not self._deployment_discards_max_tokens(kwargs):
144175
return None
145176

146177
# A configured deployment ceiling (or a caller-supplied value) must

tests/test_litellm/integrations/h2o/test_max_tokens_rename_hook.py

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,42 @@ async def test_plain_deployment_with_no_params_is_a_noop(hook):
130130

131131
@pytest.mark.asyncio
132132
async def test_malformed_drop_params_never_propagates(hook):
133-
"""A non-list additional_drop_params must not fail the request."""
134-
out = await _run(hook, {"model": "azure/gpt-4o-mini", "max_tokens": 50,
133+
"""A non-list additional_drop_params must not raise. On a non-Azure model
134+
it is the only possible signal, so the request is left alone."""
135+
out = await _run(hook, {"model": "openrouter/some-model", "max_tokens": 50,
135136
"additional_drop_params": "max_tokens"})
136137
assert out is None
137138

138139

140+
@pytest.mark.asyncio
141+
async def test_azure_route_alone_triggers_the_rename(hook):
142+
"""Reasoning Azure entries get max_completion_tokens but are exempt from
143+
the drop, so the route itself has to be recognised."""
144+
out = await _run(hook, {"model": "azure/gpt-5-mini", "max_tokens": 50,
145+
"max_completion_tokens": 16384})
146+
assert out["max_completion_tokens"] == 50
147+
assert "max_tokens" not in out
148+
149+
150+
@pytest.mark.asyncio
151+
async def test_non_azure_ceiling_alone_does_not_trigger_the_rename(hook):
152+
"""convert_model_to_litellm_config sets max_completion_tokens for EVERY
153+
reasoning model, Azure or not, and only the Azure branch adds the drop.
154+
Renaming on a non-Azure provider would strip max_tokens and leave the
155+
request with no ceiling at all, which is worse than the bug being fixed."""
156+
out = await _run(hook, {"model": "openrouter/grok-4", "max_tokens": 50,
157+
"max_completion_tokens": 16384})
158+
assert out is None
159+
160+
161+
@pytest.mark.asyncio
162+
async def test_native_deployment_with_caller_supplied_ceiling_is_untouched(hook):
163+
"""A caller sending BOTH fields to a max_tokens-native deployment must not
164+
have max_tokens stripped."""
165+
out = await _run(hook, dict(BEDROCK_KWARGS, max_completion_tokens=4096))
166+
assert out is None
167+
168+
139169
@pytest.mark.asyncio
140170
async def test_other_kwargs_are_preserved(hook):
141171
"""The hook returns the FULL kwargs dict; the dispatcher replaces kwargs
@@ -235,3 +265,44 @@ async def test_litellm_dispatch_leaves_a_max_tokens_native_deployment_alone():
235265
assert seen is not None
236266
assert seen["max_tokens"] == 50, seen
237267
assert seen.get("max_completion_tokens") is None, seen
268+
269+
270+
@pytest.mark.asyncio
271+
async def test_anthropic_messages_survives_the_rename_hook():
272+
"""litellm.anthropic_messages backs /v1/messages and bridges Azure, and it
273+
declares max_tokens as a REQUIRED parameter. The deployment-hook dispatch
274+
is not chat-specific, so popping max_tokens there made litellm's own
275+
@client wrapper raise
276+
`TypeError: anthropic_messages() missing 1 required positional argument`
277+
on the next `await original_function(...)`. That is outside this hook's
278+
try/except, and the traceback never names the hook.
279+
"""
280+
import litellm
281+
282+
previous = litellm.callbacks
283+
litellm.callbacks = [MaxTokensRenameHook()]
284+
try:
285+
await litellm.anthropic_messages(
286+
model="azure/gpt-4o-mini",
287+
messages=[{"role": "user", "content": "hi"}],
288+
max_tokens=50,
289+
max_completion_tokens=16384, # merged in by the router
290+
additional_drop_params=["max_tokens"],
291+
api_key="dummy",
292+
api_version="2025-04-01-preview",
293+
api_base="https://example.openai.azure.com",
294+
mock_response="ok",
295+
)
296+
finally:
297+
litellm.callbacks = previous
298+
299+
300+
@pytest.mark.asyncio
301+
async def test_non_chat_call_types_are_skipped():
302+
"""Same guard at the unit level, for the other entrypoints that reach this
303+
dispatch: /v1/completions has no max_completion_tokens, and embeddings
304+
have no notion of one."""
305+
hook = MaxTokensRenameHook()
306+
for call_type in ("anthropic_messages", "atext_completion", "aembedding", None):
307+
out = await hook.async_pre_call_deployment_hook(dict(AZURE_KWARGS), call_type)
308+
assert out is None, f"{call_type} must not be rewritten"

0 commit comments

Comments
 (0)