From fe7a48318dbc3beae0df52c41a8d4290162847ec Mon Sep 17 00:00:00 2001 From: songkuan-zheng <252822057+songkuan-zheng@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:47:48 +0000 Subject: [PATCH] fix(zai): restore supports_reasoning gate + use monkeypatch in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile P2 (PR #31085, commit 0af6ba42e2): 1. Restore `litellm.supports_reasoning(...)` gate in `get_supported_openai_params`. The previous commit removed the gate entirely so `thinking`/`reasoning_effort` were added unconditionally for every ZAI model — that violates the team's model-flag pattern (capability flags live in `model_prices_and_context_window.json` and are read via `supports_reasoning`, so a new model added without the flag won't silently accept params the upstream rejects). Keep the registry update for the GLM-4.5 family from the original PR; the gate now unlocks reasoning params via that registry change instead of bypassing it. The `_map_openai_params` reasoning branch is also gated on `param in supported` for symmetry. 2. Replace direct `litellm.disable_aiohttp_transport = True` writes in the test file with `monkeypatch.setattr(...)` so pytest's fixture teardown restores the original value. Direct module-level writes leak into every test that runs later in the same session. Test updates: - `TestZaiSupportedParamsWhitelistReasoning` now sets `LITELLM_LOCAL_MODEL_COST_MAP=True` in an autouse fixture so the registry update lands in `litellm.model_cost` before the gate runs. - Dropped `glm-5` from the parametrize list (not in the registry); the remaining 8 GLM-4.5+ models all carry `supports_reasoning: true` after this PR's registry update. - Added `test_reasoning_params_excluded_when_registry_flag_missing` to pin the gate behavior: a model not in the registry must NOT pick up `thinking`/`reasoning_effort` in its whitelist. - Renamed `test_thinking_works_on_glm_4_5_without_registry_flag` → `..._via_registry_flag` and updated the docstring; the test now asserts the registry-flag path (which is what the PR ships) instead of asserting the gate is bypassed. --- litellm/llms/zai/chat/transformation.py | 4 +- .../llms/zai/test_zai_provider.py | 62 +++++++++++++------ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/litellm/llms/zai/chat/transformation.py b/litellm/llms/zai/chat/transformation.py index 7a39b83cabd..9b083fcb8eb 100644 --- a/litellm/llms/zai/chat/transformation.py +++ b/litellm/llms/zai/chat/transformation.py @@ -50,7 +50,9 @@ class ZAIChatConfig(OpenAIGPTConfig): import litellm try: - if litellm.supports_reasoning(model=model, custom_llm_provider=self.custom_llm_provider): + if litellm.supports_reasoning( + model=model, custom_llm_provider=self.custom_llm_provider + ): base_params.extend(_REASONING_PARAMS) except Exception: pass diff --git a/tests/test_litellm/llms/zai/test_zai_provider.py b/tests/test_litellm/llms/zai/test_zai_provider.py index 415cd8e72d7..771787b1129 100644 --- a/tests/test_litellm/llms/zai/test_zai_provider.py +++ b/tests/test_litellm/llms/zai/test_zai_provider.py @@ -146,7 +146,7 @@ def test_glm47_cost_calculation(): async def test_zai_completion_call(respx_mock, zai_response, monkeypatch): """Test completion call with zai provider using mocked response""" monkeypatch.setenv("ZAI_API_KEY", "test-api-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_response @@ -172,7 +172,7 @@ async def test_zai_completion_call(respx_mock, zai_response, monkeypatch): def test_zai_sync_completion(respx_mock, zai_response, monkeypatch): """Test synchronous completion call""" monkeypatch.setenv("ZAI_API_KEY", "test-api-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_response @@ -212,15 +212,20 @@ def _captured_body(respx_mock): class TestZaiSupportedParamsWhitelistReasoning: - """`thinking` and `reasoning_effort` must be in the whitelist for every - GLM-4.5+ model, independently of the registry's `supports_reasoning` - flag. The prior gate `if litellm.supports_reasoning(model): base_params.append("thinking")` - silently broke any model whose registry entry was incomplete; the - entire GLM-4.5 family in `model_prices_and_context_window_backup.json` - was missing the flag despite docs.z.ai listing GLM-4.5 as the first - model with `thinking` support + """`thinking` and `reasoning_effort` enter the whitelist only when the + registry marks the model `supports_reasoning: true`. The registry + update in this PR adds the flag to the entire GLM-4.5 family + (previously every GLM-4.5 entry in + `model_prices_and_context_window_backup.json` was missing the flag + despite docs.z.ai listing GLM-4.5 as the first model with `thinking` + support), so the gate now unlocks reasoning params for all of them. """ + @pytest.fixture(autouse=True) + def _use_local_model_cost(self, monkeypatch): + monkeypatch.setenv("LITELLM_LOCAL_MODEL_COST_MAP", "True") + litellm.model_cost = litellm.get_model_cost_map(url="") + @pytest.mark.parametrize( "model", [ @@ -232,7 +237,6 @@ class TestZaiSupportedParamsWhitelistReasoning: "glm-4.5-flash", "glm-4.6", "glm-4.7", - "glm-5", ], ) def test_reasoning_params_in_whitelist(self, model): @@ -242,6 +246,23 @@ class TestZaiSupportedParamsWhitelistReasoning: assert "thinking" in params assert "reasoning_effort" in params + def test_reasoning_params_excluded_when_registry_flag_missing(self, monkeypatch): + """Regression guard for the gate. A model whose registry entry + does NOT mark `supports_reasoning: true` must keep `thinking` + and `reasoning_effort` OUT of the whitelist — otherwise a new + ZAI model added to the registry without the flag silently + accepts reasoning kwargs that the upstream API will reject. + """ + from litellm.llms.zai.chat.transformation import ZAIChatConfig + + # Synthetic model that won't match any registry entry or + # wildcard pattern. + params = ZAIChatConfig().get_supported_openai_params( + model="glm-no-such-future-model-xyz" + ) + assert "thinking" not in params + assert "reasoning_effort" not in params + class TestZaiReasoningParamsLandInExtraBody: """The OpenAI Python SDK rejects unknown top-level kwargs (e.g. @@ -318,7 +339,7 @@ class TestZaiReasoningParamsLandInExtraBody: as a top-level field (the OpenAI SDK flattened `extra_body`) """ monkeypatch.setenv("ZAI_API_KEY", "test-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_thinking_response ) @@ -338,7 +359,7 @@ class TestZaiReasoningParamsLandInExtraBody: self, respx_mock, zai_thinking_response, monkeypatch ): monkeypatch.setenv("ZAI_API_KEY", "test-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_thinking_response ) @@ -357,7 +378,7 @@ class TestZaiReasoningParamsLandInExtraBody: self, respx_mock, zai_thinking_response, monkeypatch ): monkeypatch.setenv("ZAI_API_KEY", "test-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_thinking_response ) @@ -372,16 +393,19 @@ class TestZaiReasoningParamsLandInExtraBody: assert body["reasoning_effort"] == "none" @pytest.mark.asyncio - async def test_thinking_works_on_glm_4_5_without_registry_flag( + async def test_thinking_works_on_glm_4_5_via_registry_flag( self, respx_mock, zai_thinking_response, monkeypatch ): - """Even when a registry entry is missing `supports_reasoning`, - ZAIChatConfig must still allow `thinking`. The prior gate - silently dropped the field for the entire GLM-4.5 family in - the registry; this test pins the unconditional contract + """End-to-end: GLM-4.5 carries `supports_reasoning: true` in the + registry, so the gate in `get_supported_openai_params` lets + `thinking` through and the SDK boundary lands it in the HTTP + body. Without the registry update this test would fail with the + SDK rejecting `thinking` as an unsupported param. """ monkeypatch.setenv("ZAI_API_KEY", "test-key") - litellm.disable_aiohttp_transport = True + monkeypatch.setenv("LITELLM_LOCAL_MODEL_COST_MAP", "True") + litellm.model_cost = litellm.get_model_cost_map(url="") + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) respx_mock.post("https://api.z.ai/api/paas/v4/chat/completions").respond( json=zai_thinking_response )