Log when drop_params discards caller-specified params - #7
Merged
Conversation
jwbron
force-pushed
the
drop-params-visibility-fork
branch
from
July 25, 2026 20:48
bf2ea98 to
2ee533e
Compare
| "they were meant to control is unchanged. To send them anyway, pass " | ||
| "allowed_openai_params=%s.", | ||
| list(dropped), | ||
| model, |
| "allowed_openai_params=%s.", | ||
| list(dropped), | ||
| model, | ||
| custom_llm_provider, |
`drop_params` exists so an unsupported parameter does not fail the whole
request, and that tradeoff is right. But dropping a parameter changes
generation behaviour, and today it happens with no signal at all: the branch
that pops the params is a bare loop with no logging. A `reasoning_effort`,
`temperature` or penalty set in a proxy config simply never reaches the
provider, and nothing in the logs or the response says so. The config and the
wire disagree, silently and indefinitely.
This is easiest to hit on a provider whose supported-param set is derived from
the model-cost map. `OpenrouterConfig.get_supported_openai_params` advertises
`reasoning_effort`/`thinking` only when `litellm.supports_reasoning(model)` is
true, which reads that map — so a model ABSENT from the map answers False and
the gate fails closed:
qwen/qwen3-max in_map=False supports_reasoning=False
moonshotai/kimi-k2-thinking in_map=False supports_reasoning=False
deepseek/deepseek-r1 in_map=True supports_reasoning=True
OpenRouter adds models faster than the map tracks them, so an unflagged slug
is a routine state rather than an exotic one, and `reasoning_effort` set on
one is a silent no-op. Diagnosing that currently means reading
`get_optional_params` and the model map by hand.
Warns rather than debugs, because the user asked for something and did not get
it. Deduped by (provider, model, dropped-param set) so a route that drops the
same params on every request logs once instead of flooding; the dedupe set is
bounded so a long-lived proxy serving many models cannot grow it without
limit, and past the cap it stops recording rather than stops warning.
No behaviour change beyond the log line: the params dropped are exactly the
params dropped before.
jwbron
force-pushed
the
drop-params-visibility-fork
branch
from
July 25, 2026 21:03
2ee533e to
e3d9e48
Compare
The message only named the per-request kwarg form, but the people who hit this are overwhelmingly proxy operators reading a config.yaml, for whom that phrasing reads as not applicable. allowed_openai_params is settable in a model_list entry's litellm_params (LiteLLM_Params is ConfigDict(extra="allow")) and does reach get_optional_params from there; verified end to end through a Router with the transport mocked: reasoning_effort is absent from the wire by default and present once allowed_openai_params is set in litellm_params. Covered by a test so the message cannot silently lose either remedy.
This was referenced Jul 25, 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
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.
Found while investigating why egg's agents were ignoring a configured
reasoning_effort(jwbron/egg#3599).The problem
drop_paramsexists so an unsupported parameter does not fail the whole request, and that tradeoff is right. But dropping a parameter changes generation behaviour, and today it happens with no signal at all — the branch that pops them is a bare loop with no logging:So a
reasoning_effort,temperatureor penalty set in a proxy config simply never reaches the provider, and nothing in the logs or the response says so. The config and the wire disagree, silently and indefinitely.Why it is easy to hit
Providers whose supported-param set is derived from the model-cost map fail closed on models the map doesn't know.
OpenrouterConfig.get_supported_openai_paramsadvertisesreasoning_effort/thinkingonly whenlitellm.supports_reasoning(model)is true, which reads that map:supports_reasoningqwen/qwen3-maxFalsemoonshotai/kimi-k2-thinkingFalsedeepseek/deepseek-r1TrueOpenRouter adds models faster than the map tracks them, so an unflagged slug is a routine state rather than an exotic one, and
reasoning_effortset on one is a silent no-op. Diagnosing that today means readingget_optional_paramsand the model map by hand; that is how this was found, and it took a while.This is not an OpenRouter quirk. OpenRouter is just where I hit it. Nineteen provider configs currently call
supports_reasoningfrom insideget_supported_openai_params, and the ones that use it as a bare gate fail closed in exactly the same way for any model the map does not carry:Reproduce the list rather than trusting the count, since it moves with every release:
The shape matters more than the count, and not every caller fails the same way.
AzureOpenAIO1Configcallssupports_reasoningtoo but fails open: a deployment name the map does not know is assumed reasoning-capable and getsreasoning_effortadvertised unconditionally. Same call, opposite failure mode. So the question to ask of any provider is not "does it consult the map" but "what does it do when the model is absent", and this warning is useful precisely because it answers that question at runtime instead of by reading source.The change
One warning naming the dropped params, model and provider:
Design points:
(provider, model, sorted dropped params), because the same params are dropped on every request for a given route and a per-call warning would be pure noise.drop_paramsis off, since the caller already gets a loudUnsupportedParamsError.No behaviour change beyond the log line: the params dropped are exactly the params dropped before.
Scope
Applied to the chat-completions
get_optional_paramspath only — the one that carries essentially all traffic. The transcription / image-gen / embeddings paths have their own near-identical drop sites and could take the same helper; happy to extend if preferred.Testing
5 tests in
tests/test_litellm/test_utils.py::TestDropParamsVisibility: warns on drop, warns once across repeat calls, warns separately per model, stays silent when all params are supported, stays silent whendrop_paramsis off.tests/test_litellm/test_utils.pygoes 179 → 184 passed with an identical set of 21 pre-existing failures (missing optional redis/azure/gcp deps in my env), verified by diffing the failure list with and without the change.