Commit 6985f08
refactor: move the max_tokens resolution into an h2o hook, reverting the core edits (#28)
* refactor: move the max_tokens resolution into an h2o hook, reverting the core edits
#26 implemented the max_tokens / max_completion_tokens resolution in core
litellm — `get_optional_params` plus a `get_preferred_max_tokens_param` method on
BaseConfig and four provider configs, a param threaded through `main.py`, entries
in `types/utils.py` and `types/router.py`, a constant in `types/llms/azure.py`,
and the regenerated dashboard `schema.d.ts` that the types change forced. Eleven
upstream files.
That is the wrong trade for this fork. `h2o-main` is rebuilt as
`<upstream-tag>` + the h2o file delta on every version bump, so each of those
eleven is re-applied by hand every time, while a file added under
`integrations/h2o/` is additive and costs nothing. Reviewer feedback on #26 was
exactly this.
So: all eleven core files are reverted to their upstream state, the core-utils
module and its tests are removed, and the entire behaviour now lives in
`litellm/integrations/h2o/litellm_max_tokens_resolution_hook.py`. Net change
against the upstream base is two new files and zero edits.
Nothing was given up to do it. Verified against the UNMODIFIED tree that
`async_pre_call_deployment_hook` can reach everything the core version used:
* `api_version`, `additional_drop_params`, `model_info` and the
`use_max_completion_tokens` directive are all in its kwargs for the SELECTED
deployment (the directive survives `LiteLLM_Params`' extra="allow"), so a
mixed model group is still judged per member;
* it runs BEFORE param mapping, so collapsing there still fixes the last-wins
ordering rather than only renaming a field — confirmed by driving real
`acompletion` and reading the mapped params;
* `types/*` was only needed to stop the directive leaking to the provider; the
hook pops it instead, on EVERY call type, which removes that need entirely;
* reasoning-model detection asks litellm's own `is_o_series_model` /
`is_model_gpt_5_model` rather than adding methods to those configs, so there
is no hardcoded model-name list here either;
* `get_supported_openai_params` is called directly, so a target the provider
does not accept is still never forced.
The one capability a core implementation has that this does not: the deployment
hook dispatch lives only in the `@client` decorator's ASYNC wrapper, so a direct
sync in-process `litellm.completion()` bypasses it. Not a capability we use —
this hook is registered only in the proxy config and the proxy maps
/chat/completions to `acompletion`.
The risk that comes back with a hook is the one that broke #25, and it is now
gated from evidence rather than guesswork. That dispatch is NOT chat-specific;
observed call types reaching it are `acompletion`, `anthropic_messages` and
`atext_completion`. `litellm.anthropic_messages` declares `max_tokens` as a
REQUIRED parameter, so popping it raises
`TypeError: anthropic_messages() missing 1 required positional argument`
outside the hook's try/except. Gated on `call_type` in
{completion, acompletion}, with tests pinning that /v1/messages returns
normally, /v1/completions keeps its `max_tokens`, and the directive is stripped
even on the gated types so it cannot leak there.
Carries forward every hardening from #27 (which this supersedes): floats coerced
the way AnthropicConfig already coerces them, unusable values left exactly as
they arrived, a dropped field never used as a target, non-str api_version
treated as unrecognizable rather than raising.
Verification, all measured:
* 136 unit + end-to-end tests in the new file.
* The h2ogpte#11992 model matrix through the hook: 214/214, including 182
with-limit/without-limit comparisons across 26 model routes x 7 param sets
with 0 perturbed — so function calling, tool_choice, parallel_tool_calls,
response_format and every sampling param are untouched.
* Per-provider, client 50 against a 64000 ceiling: azure
max_completion_tokens 50, anthropic max_tokens 50, bedrock maxTokens 50,
gemini max_output_tokens 50, o3/gpt-5 max_completion_tokens 50.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: regenerate schema.d.ts for a pre-existing /v1/router/models drift
Not part of this change. The "Check UI API Types Sync" workflow regenerates
schema.d.ts from the live proxy spec and diffs it, and the version checked in on
h2o-main is missing a /v1/router/models route that was added without
regenerating. That check therefore fails on any PR touching litellm/types/** —
including this one, which touches them only to revert them.
This is the exact diff CI computes, minus the two use_max_completion_tokens
lines that belonged to the core implementation this PR removes. It is a
generated file, cheap to re-apply after a rebuild, unlike the eleven hand-edited
upstream files this PR takes out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: mirror litellm's api_version fallback for Azure deployments that omit it
A capability lost in the move out of core, found by the h2ogpt cross-model
matrix rather than by hand: an Azure deployment with NO configured api_version
resolved the output-token field against nothing and sent `max_tokens`.
The core implementation ran inside `get_optional_params`, where litellm has
already applied its fallback chain — `litellm.api_version`, then
`AZURE_API_VERSION`, then `litellm.AZURE_DEFAULT_API_VERSION`, which is a 2025
version today. The hook sees only what the deployment configured, so a missing
api_version read as "unrecognizable" and the request went out on the field Azure
2025 rejects.
Now mirrors that chain, including its `or`-based falsiness so an empty-string
api_version resolves against the version the request is actually sent with.
Latent for us — `convert_model_to_litellm_config` always emits an api_version —
but a real gap for any hand-written deployment, and cheap to close.
Matrix back to 1120/1120 across 35 model routes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: restore #25's dropped-field trigger, and drop the unjustified pre-2025 rename
Two corrections, one from auditing what #25 (achraf-mer) covered and
one from testing against a live Azure deployment.
RESTORED FROM #25 — a dropped caller field must MOVE, not vanish. #25's
predicate fired whenever the selected deployment listed `max_tokens` in
`additional_drop_params`, on ANY provider: such a deployment accepts only the
other field, so moving the value across is what keeps the caller's limit rather
than letting the drop destroy it. That is the whole defect.
This implementation only covered it for Azure, via the api_version rule. So a
non-Azure deployment carrying that drop — which an operator can write by hand,
and which h2ogpt's own `_drop()` writes into the same list — silently lost the
caller's limit. Now: when every field the caller sent is being dropped and the
other field is eligible, the value moves there. Not in tension with "never
resurrect a dropped param", since the target still has to be eligible.
Covered for hosted_vllm, openai and anthropic, plus both directions and the
both-dropped case where there is nowhere to move it.
REMOVED — the pre-2025 Azure reverse rename. This returned `max_tokens` for
older api_versions, reasoning that they predate `max_completion_tokens`.
Measured against the live `h2ogpt2` deployment, that is simply false:
api-version 2024-02-01 max_completion_tokens -> 200 finish=length tokens=50
api-version 2024-08-01-preview max_completion_tokens -> 200 finish=length tokens=50
api-version 2025-04-01-preview max_tokens -> 200 finish=length tokens=50
Every api-version tested accepts EITHER field on its own. Only the pair fails:
both -> Setting 'max_tokens' and 'max_completion_tokens' at the same time
is not supported.
So renaming a lone `max_completion_tokens` on a pre-2025 deployment mutated a
request that already worked, for no measured benefit. Pre-2025 now declares no
preference, which does not weaken anything: the pair is what Azure rejects, and
the no-preference branch still collapses it onto `max_tokens`.
Worth recording alongside that: "Azure 2025+ rejects max_tokens" — the premise
behind the original workaround — does not reproduce on these deployments either.
It is model-specific (Azure's wording is "not supported with this model"), which
is why the o-series/gpt-5 preference is keyed on the model rather than only the
api_version. The load-bearing behaviour is the collapse plus tighter-wins, not
the field-name routing.
189 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: a directive pointing at a dropped field must not silently unbound the request
Review round 5. One behavioural defect and a docstring claim my own measurements
disproved.
CONTRADICTORY CONFIG SILENTLY LOST THE CEILING. An operator writing both
`use_max_completion_tokens: false` (send max_tokens) and
`additional_drop_params: ["max_tokens"]` (strip max_tokens) got the directive
honoured literally: the value stayed on max_tokens, litellm then stripped it, and
the request went upstream with NO ceiling and no error. That is exactly the class
of failure the rest of this file guards against.
An ineligible preference — dropped or unsupported — now falls through with no
preference instead of being returned, so the dropped-field rescue moves the value
somewhere that survives. The drop is still respected: the dropped field is never
sent. Same fix covers the mirror case (an o-series preference for
max_completion_tokens when the operator dropped that field: the caller's own
max_tokens is eligible, so it stays and nothing is resurrected).
DOCSTRING CORRECTED. It said Azure 2025+ api_versions reject `max_tokens`. That
is what shaped the original workaround, and it does not hold in general — the
live `h2ogpt2` deployment accepts either field on its own on 2024-02-01,
2024-08-01-preview and 2025-04-01-preview alike, and only the PAIR fails. Azure's
wording is "not supported with this model", so the single-field rejection is
model-specific. Recorded, along with the fact that the pair rejection is not
Azure-specific either: raw OpenAI returns the same 400.
Also pinned by round 5, all previously unexercised: a malformed
`additional_drop_params` (string / dict / int) is a no-op rather than a crash,
matching litellm's own list-only `_should_drop_param`; no false-positive
reasoning detection across the seven non-reasoning model routes we actually serve
(delegated detection is substring-based, so this is worth holding); the caller's
kwargs dict is never mutated in place; and a request with no `model` key does not
raise.
205 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: the fail-safe must not be fail-open for the directive
Review round 6 (subagent). The `except` returned None, which makes litellm keep
the ORIGINAL kwargs — including `use_max_completion_tokens`. Since this branch
removes that key from `all_litellm_params` and `GenericLiteLLMParams`, the hook
popping it is the ONLY thing between it and the request body, so an unexpected
error inside the resolution turned into the exact 400 measured against an
unpatched proxy:
{"max_tokens": 50, "use_max_completion_tokens": false, ...}
"Leave the request exactly as it arrived" is the right policy for the token
fields and the wrong one for the directive, which exists only because this hook
consumes it. The strip now happens before the `try` and the `except` returns that
stripped copy, so an internal bug degrades to "no resolution" instead of "every
request to this deployment 400s". Two tests, including one that throws before the
call-type gate.
Also drops a duplicated `_provider_and_model(kwargs)` call that unpacked one half
of the tuple each time.
207 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: two ways a request could leave with a HIGHER limit than anyone asked for
Review round 6 (subagent on #28). Both findings had the same shape as the defect
this PR exists to fix — a request going out with a looser ceiling than either the
caller or the operator set — and neither had a test.
1. GARBAGE BESIDE A USABLE CEILING SILENTLY APPLIED THE CEILING and suppressed the
provider's error. `values` filtered the unusable entry out, but the pop loop
still removed the field carrying it, so rule 3 ("never invent a ceiling from an
unusable value") only held when EVERY field was unusable. On azure/gpt-4o-mini
with a `max_completion_tokens: 64000` ceiling:
client max_tokens="50" before: both fields -> Azure 400
after: max_completion_tokens: 64000 -> 200
A client asking for 50 got 64000 — h2ogpte#11992's exact symptom — and the loud
error was gone. On anthropic it overwrote the caller's own field with the
ceiling. Now: if any present field holds a non-None value that is not a usable
limit, NOTHING changes.
An explicitly-None field counts as ABSENT rather than as garbage — `None` is the
OpenAI SDK default and litellm strips None-valued params, so it never reaches
the wire. Verified that ordinary requests carry no None-valued token key at all,
so this cannot disable the resolution for normal traffic.
2. THE FLOAT COERCION MADE THE CAP HOOK BYPASSABLE. `MaxTokensCapHook._cap_in`
clipped `isinstance(v, int)` only, so a float sailed past the deployment
ceiling. That was survivable while nothing normalised floats — the provider
rejected the float and the request failed loudly (only AnthropicConfig coerced
it). Coercing floats for every provider turned that into an accepted over-cap
request:
model_info.max_output_tokens = 8192
client max_tokens=99999 -> cap clips -> 8192
client max_tokens=99999.0 -> cap SKIPS -> 99999
`_cap_in` now clips floats (bool excluded, NaN/inf fall through). Fixed in the
sibling hook rather than by dropping the coercion, because the ceiling should
hold against a float regardless of this hook — and the two now have a coupled
contract, documented as such.
Also from the same review:
* A malformed directive meant its OPPOSITE, silently. `_directive_target`
correctly refused truthiness, but the api_version rule then supplied
`max_completion_tokens` anyway, so `use_max_completion_tokens: "false"` / `0` /
`"no"` produced the inverse of intent on Azure 2025. The shapes an operator
plausibly writes are now recognised explicitly; anything else is a logged
no-op, not a guess.
* Reasoning detection is scoped to providers whose model string really is an
OpenAI model id. litellm's detectors are substring matches, so
`hosted_vllm/o1-local` was renamed to `max_completion_tokens`, which TGI and
older vLLM ignore — the ceiling silently vanishes, and
`get_supported_openai_params` is no guard since it claims that field for every
openai-compatible provider.
* The pair is now collapsed even when NEITHER field is eligible (an mt-only
provider whose deployment also drops max_tokens). Emitting both left
`UnsupportedParamsError` on the table, and "collapse the pair" is the
load-bearing guarantee.
* THE CAP-HOOK INTERACTION TEST WAS VACUOUS: it reimplemented the cap hook
locally and asserted only clip-then-resolve == resolve-then-clip. Mutation
testing showed it passed unchanged with `min` replaced by `max` AND with
`_target_field` stubbed to None — a symmetry-only assertion cannot catch a
symmetric bug, which is precisely how finding 2 got through. It now imports the
real hook and asserts absolute values, including the float case.
* `get_secret` -> `get_secret_str` for AZURE_API_VERSION, matching litellm's own
chain: plain `get_secret` performs a secret-manager fetch when one is
configured and applies bool coercion, neither wanted for a version string in a
per-request hook.
* The sync-path limit is documented honestly — it is not just "no resolution",
it also leaks the directive into `extra_body`.
* The hook is added to the package docstring's registrable-hooks index.
245 tests, up from 207.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: collapse the pair on /v1/completions too, which removing the drop exposed
Review round 6 (subagent on BerriAI#1009). Skipping `atext_completion` outright was
wrong once h2ogpt stopped emitting `additional_drop_params: ["max_tokens"]`.
Measured against the real fork on an Azure text-completion deployment:
/v1/completions, main (drop present): {'max_completion_tokens': 16384}
/v1/completions, this PR (no drop): {'max_tokens': 50,
'max_completion_tokens': 16384}
Both fields, i.e. moving toward the very 400 the drop existed to prevent. The
route is marginal but the regression is real and it is caused by this work.
`/v1/completions` has no `max_completion_tokens`, so it must never get the
rename — but it does need the pair COLLAPSED, always onto `max_tokens`. Handled
as its own call-type set with a forced target rather than folded into
CHAT_CALL_TYPES, so the rename can't leak there. Safe in a way
`anthropic_messages` is not: `atext_completion` does not declare `max_tokens` as
a required parameter, so removing the other field cannot make litellm's own
wrapper raise.
246 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 60f5ee9 commit 6985f08
17 files changed
Lines changed: 1586 additions & 836 deletions
File tree
- litellm
- integrations/h2o
- litellm_core_utils
- llms
- azure/chat
- base_llm/chat
- openai/chat
- types
- llms
- tests/test_litellm
- integrations/h2o
- litellm_core_utils
- ui/litellm-dashboard/src/lib/http
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
126 | 126 | | |
127 | 127 | | |
128 | 128 | | |
129 | | - | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
130 | 150 | | |
131 | 151 | | |
132 | 152 | | |
| |||
Lines changed: 618 additions & 0 deletions
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | 59 | | |
71 | 60 | | |
72 | 61 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
| |||
149 | 148 | | |
150 | 149 | | |
151 | 150 | | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | 151 | | |
181 | 152 | | |
182 | 153 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | | - | |
202 | | - | |
203 | | - | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | 195 | | |
212 | 196 | | |
213 | 197 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
188 | 188 | | |
189 | 189 | | |
190 | 190 | | |
191 | | - | |
192 | | - | |
193 | | - | |
194 | | - | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | 191 | | |
202 | 192 | | |
203 | 193 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | 90 | | |
103 | 91 | | |
104 | 92 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5254 | 5254 | | |
5255 | 5255 | | |
5256 | 5256 | | |
5257 | | - | |
5258 | | - | |
5259 | | - | |
5260 | | - | |
5261 | 5257 | | |
5262 | 5258 | | |
5263 | 5259 | | |
| |||
0 commit comments