From b876e2b04d545aeaa75d662658212110a846bc36 Mon Sep 17 00:00:00 2001 From: privatedeskai Date: Wed, 2 Sep 2026 09:31:29 -0700 Subject: [PATCH] fix(providers): address Greptile review on SAGG test file Remove unused MagicMock/patch imports (F401, blocking required lint checks). Remove the live-network completion test: tests/e2e/CLAUDE.md's Hard Rules forbid substituting a unit test for e2e feature coverage, and this env-var-gated call doesn't fit the e2e harness either (no proxy, no ProxyClient, no coverage-registry marker) - a proper e2e addition is a separate, larger piece of work, out of scope here. The real live proof stays in the PR description's Proof of Fix section, where CLAUDE.md says it belongs. Trim docstrings and inline comments that only restated the assertion on the next line, per CLAUDE.md's comment policy - kept the two that explain non-obvious behavior (first-slash-only provider splitting, why the param mapping matters). --- .../llms/openai_like/test_sagg_provider.py | 78 ++----------------- 1 file changed, 7 insertions(+), 71 deletions(-) diff --git a/tests/test_litellm/llms/openai_like/test_sagg_provider.py b/tests/test_litellm/llms/openai_like/test_sagg_provider.py index b36b7a61135..7eef238c02b 100644 --- a/tests/test_litellm/llms/openai_like/test_sagg_provider.py +++ b/tests/test_litellm/llms/openai_like/test_sagg_provider.py @@ -7,12 +7,6 @@ inference gateway with automatic multi-provider failover. import os import sys -from unittest.mock import MagicMock, patch - -try: - import pytest -except ImportError: - pytest = None # Add workspace to path workspace_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../..")) @@ -22,27 +16,18 @@ import litellm class TestSaggProviderConfig: - """Test SAGG provider configuration""" - def test_sagg_in_provider_list(self): - """Test that sagg is in the provider list""" from litellm import LlmProviders - # Verify sagg is in the enum assert hasattr(LlmProviders, "SAGG") assert LlmProviders.SAGG.value == "sagg" - - # Verify it's in the provider list assert "sagg" in litellm.provider_list def test_sagg_json_config_exists(self): - """Test that sagg is configured in providers.json""" from litellm.llms.openai_like.json_loader import JSONProviderRegistry - # Verify sagg is loaded assert JSONProviderRegistry.exists("sagg") - # Get sagg config sagg = JSONProviderRegistry.get("sagg") assert sagg is not None assert sagg.base_url == "https://api.privatedeskai.com/v1" @@ -50,11 +35,10 @@ class TestSaggProviderConfig: assert sagg.param_mappings.get("max_completion_tokens") == "max_tokens" def test_sagg_provider_resolution(self): - """Test that provider resolution finds sagg - the real model id - contains its own slash (deepseek-ai/DeepSeek-V4-Flash-0731), so - this also confirms provider-prefix splitting only splits on the - FIRST slash, same as e.g. pinstripes/ps/glm-4.5-air elsewhere in - this same registry.""" + """The real model id contains its own slash + (deepseek-ai/DeepSeek-V4-Flash-0731), so this also confirms + provider-prefix splitting only splits on the FIRST slash, same + as e.g. pinstripes/ps/glm-4.5-air elsewhere in this registry.""" from litellm.litellm_core_utils.get_llm_provider_logic import get_llm_provider model, provider, api_key, api_base = get_llm_provider( @@ -69,10 +53,8 @@ class TestSaggProviderConfig: assert api_base == "https://api.privatedeskai.com/v1" def test_sagg_router_config(self): - """Test that sagg can be used in Router configuration""" from litellm import Router - # This should not raise "Unsupported provider - sagg" router = Router( model_list=[ { @@ -85,17 +67,13 @@ class TestSaggProviderConfig: ] ) - # Verify the deployment was created successfully assert len(router.model_list) == 1 assert router.model_list[0]["model_name"] == "sagg-deepseek" def test_sagg_parameter_mapping(self): - """Test that max_completion_tokens is mapped to max_tokens for - sagg - SAGG's own request parser only recognizes max_tokens - (cmd/gateway/billing.go's chatRequestForBilling struct has no - max_completion_tokens field at all), so a caller sending the - newer OpenAI param name needs this mapping to actually take - effect server-side, not be silently dropped.""" + """SAGG's own request parser only recognizes max_tokens, so a + caller sending max_completion_tokens needs this mapping to + actually take effect server-side rather than being dropped.""" from litellm.llms.openai_like.dynamic_config import create_config_class from litellm.llms.openai_like.json_loader import JSONProviderRegistry @@ -115,48 +93,6 @@ class TestSaggProviderConfig: assert result["temperature"] == 0.7 -class TestSaggIntegration: - """Integration tests for SAGG provider""" - - def test_sagg_completion_basic(self): - """Test basic completion call to SAGG""" - # Skip test if API key not set in environment - if not os.environ.get("SAGG_API_KEY"): - if pytest: - pytest.skip("SAGG_API_KEY not set") - return - - try: - response = litellm.completion( - model="sagg/deepseek-ai/DeepSeek-V4-Flash-0731", - messages=[ - { - "role": "user", - "content": "Say 'test successful' and nothing else", - } - ], - max_tokens=10, - ) - - assert response is not None - assert hasattr(response, "choices") - assert len(response.choices) > 0 - assert hasattr(response.choices[0], "message") - assert hasattr(response.choices[0].message, "content") - assert response.choices[0].message.content is not None - - content = response.choices[0].message.content.lower() - assert len(content) > 0 - - print(f"✓ SAGG completion successful: {response.choices[0].message.content}") - - except Exception as e: - if pytest: - pytest.fail(f"SAGG completion failed: {str(e)}") - else: - raise - - if __name__ == "__main__": print("Testing SAGG Provider...")