Merge PR #19884 into litellm_pr_review_000005

litellm/litellm_core_utils/get_litellm_params.py

@@ -1,19 +1,48 @@
 from typing import Optional
 
 
+# Pre-define optional kwargs keys as frozenset for O(1) lookups
+# These are extracted from kwargs only if present, avoiding unnecessary .get() calls
+_OPTIONAL_KWARGS_KEYS = frozenset({
+    "azure_ad_token",
+    "tenant_id",
+    "client_id",
+    "client_secret",
+    "azure_username",
+    "azure_password",
+    "azure_scope",
+    "timeout",
+    "bucket_name",
+    "vertex_credentials",
+    "vertex_project",
+    "vertex_location",
+    "vertex_ai_project",
+    "vertex_ai_location",
+    "vertex_ai_credentials",
+    "aws_region_name",
+    "aws_access_key_id",
+    "aws_secret_access_key",
+    "aws_session_token",
+    "aws_session_name",
+    "aws_profile_name",
+    "aws_role_name",
+    "aws_web_identity_token",
+    "aws_sts_endpoint",
+    "aws_external_id",
+    "aws_bedrock_runtime_endpoint",
+    "tpm",
+    "rpm",
+})
+
+
 def _get_base_model_from_litellm_call_metadata(
     metadata: Optional[dict],
 ) -> Optional[str]:
     if metadata is None:
         return None
-
-    if metadata is not None:
-        model_info = metadata.get("model_info", {})
-
-        if model_info is not None:
-            base_model = model_info.get("base_model", None)
-            if base_model is not None:
-                return base_model
+    model_info = metadata.get("model_info")
+    if model_info:
+        return model_info.get("base_model")
     return None
 
 
@@ -66,6 +95,7 @@ def get_litellm_params(
     litellm_request_debug: Optional[bool] = None,
     **kwargs,
 ) -> dict:
+    # Build base dict with explicit parameters (always included)
     litellm_params = {
         "acompletion": acompletion,
         "api_key": api_key,
@@ -112,37 +142,15 @@ def get_litellm_params(
         "ssl_verify": ssl_verify,
         "merge_reasoning_content_in_choices": merge_reasoning_content_in_choices,
         "api_version": api_version,
-        "azure_ad_token": kwargs.get("azure_ad_token"),
-        "tenant_id": kwargs.get("tenant_id"),
-        "client_id": kwargs.get("client_id"),
-        "client_secret": kwargs.get("client_secret"),
-        "azure_username": kwargs.get("azure_username"),
-        "azure_password": kwargs.get("azure_password"),
-        "azure_scope": kwargs.get("azure_scope"),
         "max_retries": max_retries,
-        "timeout": kwargs.get("timeout"),
-        "bucket_name": kwargs.get("bucket_name"),
-        "vertex_credentials": kwargs.get("vertex_credentials"),
-        "vertex_project": kwargs.get("vertex_project"),
-        "vertex_location": kwargs.get("vertex_location"),
-        "vertex_ai_project": kwargs.get("vertex_ai_project"),
-        "vertex_ai_location": kwargs.get("vertex_ai_location"),
-        "vertex_ai_credentials": kwargs.get("vertex_ai_credentials"),
         "use_litellm_proxy": use_litellm_proxy,
         "litellm_request_debug": litellm_request_debug,
-        "aws_region_name": kwargs.get("aws_region_name"),
-        # AWS credentials for Bedrock/Sagemaker
-        "aws_access_key_id": kwargs.get("aws_access_key_id"),
-        "aws_secret_access_key": kwargs.get("aws_secret_access_key"),
-        "aws_session_token": kwargs.get("aws_session_token"),
-        "aws_session_name": kwargs.get("aws_session_name"),
-        "aws_profile_name": kwargs.get("aws_profile_name"),
-        "aws_role_name": kwargs.get("aws_role_name"),
-        "aws_web_identity_token": kwargs.get("aws_web_identity_token"),
-        "aws_sts_endpoint": kwargs.get("aws_sts_endpoint"),
-        "aws_external_id": kwargs.get("aws_external_id"),
-        "aws_bedrock_runtime_endpoint": kwargs.get("aws_bedrock_runtime_endpoint"),
-        "tpm": kwargs.get("tpm"),
-        "rpm": kwargs.get("rpm"),
     }
+
+    # Sparse extraction: only add kwargs keys that are actually present
+    if kwargs:
+        for key in _OPTIONAL_KWARGS_KEYS:
+            if key in kwargs:
+                litellm_params[key] = kwargs[key]
+
     return litellm_params

tests/test_litellm/litellm_core_utils/test_get_litellm_params.py

@@ -0,0 +1,127 @@
+"""
+Tests for get_litellm_params and related helpers.
+
+Ensures backward compatibility after sparse kwargs extraction optimization.
+"""
+
+import pytest
+
+from litellm.litellm_core_utils.get_litellm_params import (
+    _OPTIONAL_KWARGS_KEYS,
+    _get_base_model_from_litellm_call_metadata,
+    get_litellm_params,
+)
+
+
+class TestGetBaseModelFromLitellmCallMetadata:
+    def test_none_metadata_returns_none(self):
+        assert _get_base_model_from_litellm_call_metadata(None) is None
+
+    def test_empty_metadata_returns_none(self):
+        assert _get_base_model_from_litellm_call_metadata({}) is None
+
+    def test_missing_model_info_returns_none(self):
+        assert _get_base_model_from_litellm_call_metadata({"foo": "bar"}) is None
+
+    def test_model_info_none_returns_none(self):
+        assert _get_base_model_from_litellm_call_metadata({"model_info": None}) is None
+
+    def test_model_info_empty_dict_returns_none(self):
+        assert _get_base_model_from_litellm_call_metadata({"model_info": {}}) is None
+
+    def test_returns_base_model(self):
+        result = _get_base_model_from_litellm_call_metadata(
+            {"model_info": {"base_model": "gpt-4"}}
+        )
+        assert result == "gpt-4"
+
+
+class TestGetLitellmParamsKwargsExtraction:
+    """Verify that optional kwargs are correctly extracted via sparse extraction."""
+
+    def test_no_kwargs_omits_optional_keys(self):
+        """When no kwargs passed, optional keys should not be in result."""
+        result = get_litellm_params(api_key="test-key")
+        for key in _OPTIONAL_KWARGS_KEYS:
+            assert key not in result
+
+    def test_present_kwargs_are_extracted(self):
+        result = get_litellm_params(
+            aws_region_name="us-east-1",
+            timeout=30,
+            rpm=100,
+        )
+        assert result["aws_region_name"] == "us-east-1"
+        assert result["timeout"] == 30
+        assert result["rpm"] == 100
+
+    def test_subset_of_kwargs_only_includes_provided(self):
+        """Only provided kwargs appear, others remain absent."""
+        result = get_litellm_params(azure_ad_token="token123")
+        assert result["azure_ad_token"] == "token123"
+        assert "aws_region_name" not in result
+        assert "timeout" not in result
+
+    def test_unknown_kwargs_are_ignored(self):
+        result = get_litellm_params(some_random_kwarg="value")
+        assert "some_random_kwarg" not in result
+
+    def test_all_optional_kwargs_extractable(self):
+        """Every key in _OPTIONAL_KWARGS_KEYS can be extracted."""
+        kwargs = {key: f"val_{key}" for key in _OPTIONAL_KWARGS_KEYS}
+        result = get_litellm_params(**kwargs)
+        for key in _OPTIONAL_KWARGS_KEYS:
+            assert result[key] == f"val_{key}"
+
+
+class TestGetLitellmParamsBaseModel:
+    """Verify base_model resolution precedence."""
+
+    def test_explicit_base_model_takes_precedence(self):
+        result = get_litellm_params(
+            base_model="explicit",
+            metadata={"model_info": {"base_model": "from-metadata"}},
+        )
+        assert result["base_model"] == "explicit"
+
+    def test_falls_back_to_metadata(self):
+        result = get_litellm_params(
+            metadata={"model_info": {"base_model": "from-metadata"}}
+        )
+        assert result["base_model"] == "from-metadata"
+
+    def test_none_when_no_source(self):
+        result = get_litellm_params()
+        assert result["base_model"] is None
+
+
+class TestGetLitellmParamsExplicitFields:
+    """Verify explicit parameters are always present in the result."""
+
+    def test_explicit_params_always_present(self):
+        result = get_litellm_params()
+        # Spot-check a few explicit keys that should always be in the dict
+        expected_keys = [
+            "acompletion",
+            "api_key",
+            "force_timeout",
+            "verbose",
+            "custom_llm_provider",
+            "api_base",
+            "metadata",
+            "model_info",
+            "max_retries",
+            "ssl_verify",
+            "api_version",
+        ]
+        for key in expected_keys:
+            assert key in result
+
+    def test_no_log_from_kwargs(self):
+        """no-log can come via **kwargs as well as the explicit param."""
+        result = get_litellm_params(**{"no-log": True})
+        assert result["no-log"] is True
+
+    def test_no_log_from_explicit_param(self):
+        result = get_litellm_params(no_log=True)
+        assert result["no-log"] is True