diff --git a/src/sap_cloud_sdk/aicore/completion.py b/src/sap_cloud_sdk/aicore/completion.py index 70dc6266..4fb91668 100644 --- a/src/sap_cloud_sdk/aicore/completion.py +++ b/src/sap_cloud_sdk/aicore/completion.py @@ -40,9 +40,15 @@ ... These are intentionally thin — every other keyword argument is forwarded -verbatim to ``litellm.completion`` / ``litellm.acompletion``, including -``stream=True`` (the streaming iterator is returned unchanged). No -telemetry is recorded here; the wrappers fire on every LLM call and a +verbatim to ``litellm.completion`` / ``litellm.acompletion``. When output +filtering is active, ``stream=True`` is not supported — the filtering +transport patch raises :class:`ContentFilteredError` upfront before the +request is sent. This applies universally to all callers (including +``ChatLiteLLM`` and LangGraph agents). Input filtering works with streaming +— filter rejections surface as :class:`ContentFilteredError` through the +stream iterator regardless. + +No telemetry is recorded here; the wrappers fire on every LLM call and a counter at this level would be both noisy and uninformative about adoption of the SDK. """ diff --git a/src/sap_cloud_sdk/aicore/filtering/_patch.py b/src/sap_cloud_sdk/aicore/filtering/_patch.py index c3c1ab1d..3f3d7575 100644 --- a/src/sap_cloud_sdk/aicore/filtering/_patch.py +++ b/src/sap_cloud_sdk/aicore/filtering/_patch.py @@ -26,6 +26,21 @@ back into :class:`ContentFilteredError` before it reaches caller code; the :func:`_parse_input_filter_error` helper in :mod:`.filters` does the JSON parsing. + +Streaming + output filtering +---------------------------- +The ``transform_response`` hook is never called for streaming responses — +LiteLLM routes those through a separate iterator path. This means output +filter rejections (``finish_reason == "content_filter"`` chunks) would be +silently yielded to the caller rather than raising +:class:`ContentFilteredError`. + +To prevent this bypass, ``transform_request`` raises +:class:`ContentFilteredError` upfront when both streaming and output +filtering are requested simultaneously. This guarantee is universal — it +applies to all callers (``sap_cloud_sdk.aicore.completion``, +``ChatLiteLLM``, LangGraph, etc.) because ``transform_request`` runs at +the LiteLLM transport layer. """ from __future__ import annotations @@ -73,10 +88,22 @@ def transform_request( if not filtering_dict: return body + # Streaming + output filtering: transform_response is never called for + # streams, so content_filter chunks would be silently yielded. Block + # the combination upfront — applies to all callers universally. + if _active_cfg.output_filtering is not None and body.get("config", {}).get( + "stream", {} + ).get("enabled"): + raise ContentFilteredError( + direction="output", + details={}, + request_id=None, + ) + modules = body["config"]["modules"] if isinstance(modules, list): - # Fallback mode (list of configs) — inject into primary config only. - modules[0]["filtering"] = filtering_dict + for m in modules: + m["filtering"] = filtering_dict else: modules["filtering"] = filtering_dict diff --git a/src/sap_cloud_sdk/aicore/user-guide.md b/src/sap_cloud_sdk/aicore/user-guide.md index 095a9204..6304ec28 100644 --- a/src/sap_cloud_sdk/aicore/user-guide.md +++ b/src/sap_cloud_sdk/aicore/user-guide.md @@ -360,10 +360,15 @@ except ContentFilteredError as e: return "Your request was blocked by content safety policy." ``` -The wrapper forwards every argument verbatim to `litellm.completion` -(including `stream=True`), and only intercepts the wrapped -`APIConnectionError` shape that LiteLLM produces for input-filter -rejections. All other exceptions surface unchanged. +The wrapper forwards every argument verbatim to `litellm.completion`, +including `stream=True`. When output filtering is active, `stream=True` is +not supported — a `ContentFilteredError(direction="output")` is raised before +the request is sent. This block applies universally to all callers (including +`ChatLiteLLM` and LangGraph agents) because it fires inside the LiteLLM +transport patch. Input filtering works with streaming — filter rejections +surface as `ContentFilteredError` regardless. + +All other exceptions surface unchanged. `ContentFilteredError` exposes three attributes — `direction`, `details`, `request_id`. The `details` field contains severity scalars from the server, @@ -444,21 +449,28 @@ print(response.choices[0].message.content) ### Streaming Responses +`stream=True` is supported when no output filtering is configured. If output +filtering is active, streaming is blocked upfront with +`ContentFilteredError(direction="output")` — this applies to all callers +including `ChatLiteLLM` and LangGraph agents. + ```python -from sap_cloud_sdk.aicore import set_aicore_config -from litellm import completion +from sap_cloud_sdk.aicore import ContentFilteredError, completion, set_aicore_config set_aicore_config() -response = completion( - model="sap/gpt-4", - messages=[{"role": "user", "content": "Tell me a story"}], - stream=True, -) +try: + response = completion( + model="sap/gpt-4", + messages=[{"role": "user", "content": "Tell me a story"}], + stream=True, + ) -for chunk in response: - if chunk.choices[0].delta.content: - print(chunk.choices[0].delta.content, end="") + for chunk in response: + if chunk.choices[0].delta.content: + print(chunk.choices[0].delta.content, end="") +except ContentFilteredError as e: + print(f"Blocked ({e.direction}): {e.details}") ``` ### Using Different Models diff --git a/tests/aicore/filtering/unit/test_filtering.py b/tests/aicore/filtering/unit/test_filtering.py index 0f0fe6c9..6cb16024 100644 --- a/tests/aicore/filtering/unit/test_filtering.py +++ b/tests/aicore/filtering/unit/test_filtering.py @@ -133,3 +133,60 @@ def test_disable_when_never_enabled(self, monkeypatch): disable_filtering() assert litellm.GenAIHubOrchestrationConfig is _ORIGINAL_CONFIG assert _filters_mod._active_cfg is None + + +class TestTransformRequestMultiModule: + """Regression tests: filtering must be injected into every fallback module, not just modules[0].""" + + def _make_cfg(self) -> ContentFiltering: + return ContentFiltering( + input_filtering=InputFiltering(filters=[AzureContentFilter()]) + ) + + def _call_transform_request(self, modules_value): + """Call the filtering injection logic directly, bypassing the LiteLLM base class.""" + from unittest.mock import patch + + body = {"config": {"modules": modules_value}} + instance = FilteringOrchestrationConfig() + + with patch( + "sap_cloud_sdk.aicore.filtering._patch.GenAIHubOrchestrationConfig.transform_request", + return_value=body, + ): + return instance.transform_request( + model="sap/x", + messages=[], + optional_params={}, + litellm_params={}, + headers={}, + ) + + def test_single_module_dict_gets_filtering(self, monkeypatch): + _clear_aicore_env(monkeypatch) + set_filtering(self._make_cfg()) + modules = {"model_id": "primary-llm"} + result = self._call_transform_request(modules) + assert "filtering" in result["config"]["modules"] + + def test_multi_module_list_all_get_filtering(self, monkeypatch): + _clear_aicore_env(monkeypatch) + set_filtering(self._make_cfg()) + modules = [ + {"model_id": "primary-llm"}, + {"model_id": "fallback-llm-1"}, + {"model_id": "fallback-llm-2"}, + ] + result = self._call_transform_request(modules) + for i, m in enumerate(result["config"]["modules"]): + assert "filtering" in m, f"modules[{i}] missing filtering block" + + def test_multi_module_filtering_dict_is_identical_across_modules(self, monkeypatch): + _clear_aicore_env(monkeypatch) + set_filtering(self._make_cfg()) + modules = [{"model_id": "a"}, {"model_id": "b"}] + result = self._call_transform_request(modules) + assert ( + result["config"]["modules"][0]["filtering"] + == result["config"]["modules"][1]["filtering"] + ) diff --git a/tests/aicore/unit/test_completion.py b/tests/aicore/unit/test_completion.py index 8238e922..88458c42 100644 --- a/tests/aicore/unit/test_completion.py +++ b/tests/aicore/unit/test_completion.py @@ -23,7 +23,7 @@ import asyncio import json -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import litellm import pytest @@ -275,6 +275,159 @@ def test_non_auth_error_does_not_trigger_reload(self): mock_reload.assert_not_called() +# --------------------------------------------------------------------------- +# Streaming guard — stream=True must not bypass output filtering +# --------------------------------------------------------------------------- + + +class TestStreamingGuard: + """stream=True is blocked upfront when output filtering is active. + Input-only filtering: stream=True passes through unmodified. + """ + + @pytest.fixture(autouse=True) + def restore_filtering(self): + from sap_cloud_sdk.aicore.filtering._patch import _install + _install(None) + yield + _install(None) + + def _activate_input_only(self): + from sap_cloud_sdk.aicore.filtering import ( + AzureContentFilter, ContentFiltering, InputFiltering, set_filtering, + ) + set_filtering(ContentFiltering( + input_filtering=InputFiltering(filters=[AzureContentFilter()]) + )) + + def _activate_output(self): + from sap_cloud_sdk.aicore.filtering import ( + AzureContentFilter, ContentFiltering, OutputFiltering, set_filtering, + ) + set_filtering(ContentFiltering( + output_filtering=OutputFiltering(filters=[AzureContentFilter()]) + )) + + def _activate_input_and_output(self): + from sap_cloud_sdk.aicore.filtering import ( + AzureContentFilter, ContentFiltering, InputFiltering, OutputFiltering, + set_filtering, + ) + set_filtering(ContentFiltering( + input_filtering=InputFiltering(filters=[AzureContentFilter()]), + output_filtering=OutputFiltering(filters=[AzureContentFilter()]), + )) + + # --- input-only or no filtering: stream=True allowed -------------------- + + def test_no_filtering_stream_allowed(self): + sentinel = object() + with patch("sap_cloud_sdk.aicore.completion.litellm.completion", return_value=sentinel): + result = completion(model="sap/x", messages=[], stream=True) + assert result is sentinel + + def test_input_only_filtering_stream_allowed(self): + self._activate_input_only() + sentinel = object() + with patch("sap_cloud_sdk.aicore.completion.litellm.completion", return_value=sentinel): + result = completion(model="sap/x", messages=[], stream=True) + assert result is sentinel + + def test_acompletion_input_only_stream_allowed(self): + sentinel = object() + + async def _fake_acompletion(*_, **__): # noqa: S7503 + return sentinel + + with patch("sap_cloud_sdk.aicore.completion.litellm.acompletion", side_effect=_fake_acompletion): + result = asyncio.run(acompletion(model="sap/x", messages=[], stream=True)) + assert result is sentinel + + # --- output filtering + stream=True: blocked upfront -------------------- + + def test_output_filter_stream_blocked(self): + from sap_cloud_sdk.aicore.filtering.exceptions import ContentFilteredError + from sap_cloud_sdk.aicore.filtering._patch import FilteringOrchestrationConfig + from unittest.mock import patch as _patch + + self._activate_output() + + body = {"config": {"stream": {"enabled": True}, "modules": {"model_id": "x"}}} + instance = FilteringOrchestrationConfig() + + with _patch( + "sap_cloud_sdk.aicore.filtering._patch.GenAIHubOrchestrationConfig.transform_request", + return_value=body, + ): + with pytest.raises(ContentFilteredError) as ei: + instance.transform_request( + model="sap/x", messages=[], optional_params={}, + litellm_params={}, headers={}, + ) + assert ei.value.direction == "output" + + def test_input_and_output_filter_stream_blocked(self): + from sap_cloud_sdk.aicore.filtering.exceptions import ContentFilteredError + from sap_cloud_sdk.aicore.filtering._patch import FilteringOrchestrationConfig + from unittest.mock import patch as _patch + + self._activate_input_and_output() + + body = {"config": {"stream": {"enabled": True}, "modules": {"model_id": "x"}}} + instance = FilteringOrchestrationConfig() + + with _patch( + "sap_cloud_sdk.aicore.filtering._patch.GenAIHubOrchestrationConfig.transform_request", + return_value=body, + ): + with pytest.raises(ContentFilteredError) as ei: + instance.transform_request( + model="sap/x", messages=[], optional_params={}, + litellm_params={}, headers={}, + ) + assert ei.value.direction == "output" + + def test_output_filter_no_stream_not_blocked(self): + from sap_cloud_sdk.aicore.filtering._patch import FilteringOrchestrationConfig + from unittest.mock import patch as _patch + + self._activate_output() + + body = {"config": {"modules": {"model_id": "x"}}} + instance = FilteringOrchestrationConfig() + + with _patch( + "sap_cloud_sdk.aicore.filtering._patch.GenAIHubOrchestrationConfig.transform_request", + return_value=body, + ): + result = instance.transform_request( + model="sap/x", messages=[], optional_params={}, + litellm_params={}, headers={}, + ) + assert "filtering" in result["config"]["modules"] + + def test_acompletion_output_filter_stream_blocked(self): + from sap_cloud_sdk.aicore.filtering.exceptions import ContentFilteredError + from sap_cloud_sdk.aicore.filtering._patch import FilteringOrchestrationConfig + from unittest.mock import patch as _patch + + self._activate_output() + + body = {"config": {"stream": {"enabled": True}, "modules": {"model_id": "x"}}} + instance = FilteringOrchestrationConfig() + + with _patch( + "sap_cloud_sdk.aicore.filtering._patch.GenAIHubOrchestrationConfig.transform_request", + return_value=body, + ): + with pytest.raises(ContentFilteredError) as ei: + instance.transform_request( + model="sap/x", messages=[], optional_params={}, + litellm_params={}, headers={}, + ) + assert ei.value.direction == "output" + + # --------------------------------------------------------------------------- # Reactive reload on AuthenticationError — async # ---------------------------------------------------------------------------