fix(proxy): strip tool config from health check probes (#31008) - #31057
Conversation
Health checks reuse the deployment's litellm_params, so a deployment with extra_body.tools (e.g. openrouter:web_search) sent those server tools on the trivial probe and OpenRouter 500'd it, while real completions on the same deployment succeeded. _update_litellm_params_for_health_check now works on a copy and strips tools and tool_choice from both the top level and extra_body before probing, since a trivial probe never needs tool configuration
Greptile SummaryThis PR fixes health check probes failing for deployments that configure provider server tools (e.g.
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to health probe construction, creates a copy so caller params are unaffected, and is covered by new regression tests. The fix is minimal and well-contained: it creates a shallow copy of No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/health_check.py | Adds _HEALTH_CHECK_STRIPPED_REQUEST_KEYS frozenset and strips tools/tool_choice from both top-level params and extra_body at the start of _update_litellm_params_for_health_check, working on a copy so caller params are never mutated. |
| tests/test_litellm/proxy/test_health_check_max_tokens.py | Adds two new mock-only regression tests: one verifying tools/tool_choice are stripped from both top-level and extra_body while other extra_body fields survive, and one verifying no crash when extra_body is absent. |
Reviews (3): Last reviewed commit: "test(health_check): cover probe tool-str..." | Re-trigger Greptile
| _HEALTH_CHECK_STRIPPED_REQUEST_KEYS: frozenset[str] = frozenset( | ||
| {"tools", "tool_choice"} | ||
| ) |
There was a problem hiding this comment.
The older OpenAI
functions/function_call keys (the predecessor to tools/tool_choice) are not included in the strip set. Some providers still accept or even require this older format, and a deployment configured with functions in its litellm_params or extra_body would have the same 500-on-probe failure that this PR is fixing. Worth adding them here while the mechanism is being established.
| _HEALTH_CHECK_STRIPPED_REQUEST_KEYS: frozenset[str] = frozenset( | |
| {"tools", "tool_choice"} | |
| ) | |
| _HEALTH_CHECK_STRIPPED_REQUEST_KEYS: frozenset[str] = frozenset( | |
| {"tools", "tool_choice", "functions", "function_call"} | |
| ) |
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!
Greptile SummaryThis PR fixes a bug where health check probes inherited
Confidence Score: 4/5Safe to merge; the change is well-scoped and guarded by two targeted unit tests The fix is straightforward and the tests verify both the stripping and no-mutation guarantees. One edge case remains: when all keys in litellm/proxy/health_check.py — the empty
|
| Filename | Overview |
|---|---|
| litellm/proxy/health_check.py | Adds _HEALTH_CHECK_STRIPPED_REQUEST_KEYS constant and rebuilds litellm_params as a shallow copy at the top of _update_litellm_params_for_health_check, stripping tools/tool_choice from both the top-level dict and extra_body; minor edge case where an all-tools extra_body becomes {} |
| tests/litellm_utils_tests/test_health_check.py | Adds two new unit tests covering tool-config stripping and no-mutation guarantees; both are pure function tests with no network calls, consistent with the test directory's rules |
Reviews (2): Last reviewed commit: "fix(proxy): strip tool config from healt..." | Re-trigger Greptile
| if isinstance(_extra_body, dict): | ||
| litellm_params["extra_body"] = { | ||
| k: v | ||
| for k, v in _extra_body.items() | ||
| if k not in _HEALTH_CHECK_STRIPPED_REQUEST_KEYS | ||
| } |
There was a problem hiding this comment.
When every key in
extra_body is a stripped key (e.g. a deployment that only sets extra_body.tools), the result is litellm_params["extra_body"] = {}. Most providers will ignore an empty dict, but some may treat the presence of the key differently than its absence. Dropping the empty dict is safer and keeps the probe equivalent to one without an extra_body.
| if isinstance(_extra_body, dict): | |
| litellm_params["extra_body"] = { | |
| k: v | |
| for k, v in _extra_body.items() | |
| if k not in _HEALTH_CHECK_STRIPPED_REQUEST_KEYS | |
| } | |
| if isinstance(_extra_body, dict): | |
| _stripped_extra_body = { | |
| k: v | |
| for k, v in _extra_body.items() | |
| if k not in _HEALTH_CHECK_STRIPPED_REQUEST_KEYS | |
| } | |
| if _stripped_extra_body: | |
| litellm_params["extra_body"] = _stripped_extra_body | |
| else: | |
| litellm_params.pop("extra_body", None) |
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!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
tests/litellm_utils_tests is not run by make test-unit or the codecov job, so the regression added there reported 0% patch coverage. Moved it to tests/test_litellm/proxy/test_health_check_max_tokens.py, the unit suite where _update_litellm_params_for_health_check is already exercised, and reverted the litellm_utils copy
The "Check UI API Types Sync" failure is independent of this PR. The check runs only because its path filter matches That base drift has been accumulating. Happy to open a separate PR against staging that runs |
|
Thanks for the contribution! A couple of things:
|
|
Ran into this same class of problem building an OpenRouter-first agent harness, so I read through the patch — two notes and a question from the sidelines: What this covers, from the diff: the probe works on a copy, Scope note for maintainers: the second repro in #31008 (deployment tools + a very low client One question: OpenRouter's web search can also be configured via |
Relevant issues
Fixes #31008
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Config (
health_check_extra_body.yaml, needsOPENROUTER_API_KEYin.env):Before the fix (run on the unpatched branch), the deployment is reported unhealthy because the probe inherits
extra_body.tools:After the fix, the same deployment is healthy and a real completion still uses web search (tools are preserved for live traffic, only probes strip them):
Type
🐛 Bug Fix
Changes
Background and on-demand health checks reuse the deployment's
litellm_params, so a deployment withextra_body.tools(e.g.openrouter:web_search) sent those server tools on the trivial probe and OpenRouter 500'd it, while real completions on the same deployment succeeded._update_litellm_params_for_health_checknow works on a copy and stripstoolsandtool_choicefrom both the top level andextra_bodybefore probing, since a trivial probe never needs tool configurationtool_choiceis removed alongsidetoolsbecause some providers reject atool_choicewith notools. Otherextra_bodyfields such as provider routing are preserved, and the caller's params are no longer mutated; a regression test intests/litellm_utils_tests/test_health_check.pypins both the stripping and the no-mutation behavior