From a0994fe8860cc0e91e2e495f807c71458e56e213 Mon Sep 17 00:00:00 2001 From: Vasilisa Parshikova Date: Tue, 24 Mar 2026 13:18:11 +0400 Subject: [PATCH] (sap) fix after bot review --- litellm/llms/sap/chat/models.py | 9 ++- litellm/llms/sap/chat/transformation.py | 4 +- .../llms/sap/chat/test_sap_transformation.py | 56 ++++++++++--------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/litellm/llms/sap/chat/models.py b/litellm/llms/sap/chat/models.py index d5b37473745..aca485e8b31 100644 --- a/litellm/llms/sap/chat/models.py +++ b/litellm/llms/sap/chat/models.py @@ -391,10 +391,13 @@ class MaskingModuleConfig(BaseModel): has_providers = self.providers is not None has_masking_providers = self.masking_providers is not None - if has_providers == has_masking_providers: + if not has_providers and not has_masking_providers: raise ValueError( - "For SAP Masking Module Config must set exactly one of: 'providers' or 'masking_providers' " - "DEPRECATED: parameter 'masking_providers' will be removed Sept 15, 2026. Use 'providers' instead." + "For SAP Masking Module Config you must provide 'providers'." + ) + if has_providers and has_masking_providers: + raise ValueError( + "For SAP Masking Module Config you must set exactly one of: 'providers' or 'masking_providers', not both." ) if has_masking_providers: diff --git a/litellm/llms/sap/chat/transformation.py b/litellm/llms/sap/chat/transformation.py index 90d985daecf..33176495f05 100755 --- a/litellm/llms/sap/chat/transformation.py +++ b/litellm/llms/sap/chat/transformation.py @@ -281,9 +281,9 @@ class GenAIHubOrchestrationConfig(OpenAIGPTConfig): if "delimiters" in stream_options: stream_config["delimiters"] = stream_options.get("delimiters") - placeholder_values = optional_params.pop("placeholder_values", {}) + placeholder_values = optional_params.pop("placeholder_values", None) placeholder_values = ( - {"placeholder_values": placeholder_values} if placeholder_values else {} + {"placeholder_values": placeholder_values} if placeholder_values is not None else {} ) fallback_modules = optional_params.pop("fallback_sap_modules", []) diff --git a/tests/test_litellm/llms/sap/chat/test_sap_transformation.py b/tests/test_litellm/llms/sap/chat/test_sap_transformation.py index 9bb62e95541..15ce1c85e8f 100644 --- a/tests/test_litellm/llms/sap/chat/test_sap_transformation.py +++ b/tests/test_litellm/llms/sap/chat/test_sap_transformation.py @@ -268,9 +268,12 @@ class TestSAPTransformationIntegration: litellm_params={}, headers={} ) - assert config["config"]["modules"]["grounding"] == grounding_config assert config["placeholder_values"] == placeholder_values - assert config["config"]["modules"]["prompt_templating"]["model"]["params"] == {} + modules = config["config"]["modules"] + assert modules["grounding"]["type"] == "document_grounding_service" + assert modules["grounding"]["config"]["placeholders"]["output"] == "grounding_response" + assert modules["grounding"]["config"]["filters"][0]["data_repository_type"] == "vector" + assert modules["prompt_templating"]["model"]["params"] == {} def test_grounding_search_config_rejects_both_count_fields(self, mock_config): with pytest.raises(ValidationError): @@ -534,27 +537,28 @@ class TestSAPTransformationIntegration: 'target_language': 'fr-FR'} } } - - config = mock_config.transform_request( - model="gpt-4o", - messages=[{"role": "user", "content": "Hello."}], - optional_params={'deployment_url': "shouldn't be in results", - "fallback_sap_modules": [{"model": "sap/gpt-5", - "messages": [{"role": "user", "content": "Hello world!"}], - "translation": translation_config - }] - , - }, - litellm_params={}, - headers={} - ) - assert "translation" not in config["config"]["modules"][0] - translation = config["config"]["modules"][1]["translation"] - assert translation["input"]["config"]["source_language"] == "en-US" - assert translation["input"]["config"]["target_language"] == "de-DE" - assert translation["output"]["config"]["target_language"] == "fr-FR" - assert config["config"]["modules"][1]["prompt_templating"]["model"]["name"] == "gpt-5" - assert config["config"]["modules"][0]["prompt_templating"]["model"]["name"] == "gpt-4o" - assert config["config"]["modules"][0]["prompt_templating"]["model"]["params"] == {} - assert config["config"]["modules"][1]["prompt_templating"]["prompt"]["template"][0]["content"] == "Hello world!" - assert config["config"]["modules"][0]["prompt_templating"]["prompt"]["template"][0]["content"] == "Hello." + for model in ["sap/gpt-5", "gpt-5"]: + config = mock_config.transform_request( + model="gpt-4o", + messages=[{"role": "user", "content": "Hello."}], + optional_params={'deployment_url': "shouldn't be in results", + "fallback_sap_modules": [{"model": model, + "messages": [{"role": "user", "content": "Hello world!"}], + "translation": translation_config + }] + , + }, + litellm_params={}, + headers={} + ) + assert "translation" not in config["config"]["modules"][0] + translation = config["config"]["modules"][1]["translation"] + assert translation["input"]["config"]["source_language"] == "en-US" + assert translation["input"]["config"]["target_language"] == "de-DE" + assert translation["output"]["config"]["target_language"] == "fr-FR" + assert config["config"]["modules"][1]["prompt_templating"]["model"]["name"] == "gpt-5" + assert config["config"]["modules"][0]["prompt_templating"]["model"]["name"] == "gpt-4o" + assert config["config"]["modules"][0]["prompt_templating"]["model"]["params"] == {} + assert config["config"]["modules"][1]["prompt_templating"]["prompt"]["template"][0]["content"] == "Hello world!" + assert config["config"]["modules"][0]["prompt_templating"]["prompt"]["template"][0]["content"] == "Hello." + assert config["config"]["modules"][1]["translation"]["input"]["type"] == "sap_document_translation"