From 0e6c4279b6387a26ee43e7ef8b8454bdee65295e Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Mon, 31 Aug 2026 21:35:32 -0700 Subject: [PATCH] fix(guardrails): record not_run evaluation when scoping leaves nothing to scan Replaces the metadata transfer approach: that block read top-level litellm_metadata which guardrail info never populates, and the SLP builder already reads the nested bucket it lands in, so it was dead code and is reverted. Real cause of missing evaluations: process_input_messages skips apply_guardrail entirely when message scoping (skip_system_message, skip_tool, scan_only_tool_results) leaves no scannable content, so the guardrail shows up in applied_guardrails with no guardrail_information entry. Now records a not_run entry unless the guardrail records its own information. --- .../chat/guardrail_translation/handler.py | 9 ++ .../proxy/hooks/proxy_track_cost_callback.py | 14 -- .../test_openai_guardrail_handler.py | 50 +++++++ .../hooks/test_proxy_track_cost_callback.py | 132 ++++++++---------- 4 files changed, 117 insertions(+), 88 deletions(-) diff --git a/litellm/llms/openai/chat/guardrail_translation/handler.py b/litellm/llms/openai/chat/guardrail_translation/handler.py index de15fefe943..76a7f33454a 100644 --- a/litellm/llms/openai/chat/guardrail_translation/handler.py +++ b/litellm/llms/openai/chat/guardrail_translation/handler.py @@ -191,6 +191,15 @@ class OpenAIChatCompletionsHandler(BaseTranslation): task_mappings=tool_call_task_mappings, ) + elif not guardrail_to_apply.records_own_guardrail_information: + # Every guardrail in applied_guardrails needs a persisted evaluation record, + # or request logs report it as silently missing (LIT-6314). + guardrail_to_apply.add_standard_logging_guardrail_information_to_request_data( + guardrail_json_response="no scannable content after message scoping", + request_data=data, + guardrail_status="not_run", + ) + verbose_proxy_logger.debug( "OpenAI Chat Completions: Processed input messages: %s", data.get("messages"), diff --git a/litellm/proxy/hooks/proxy_track_cost_callback.py b/litellm/proxy/hooks/proxy_track_cost_callback.py index b78884f7d95..47aafda2337 100644 --- a/litellm/proxy/hooks/proxy_track_cost_callback.py +++ b/litellm/proxy/hooks/proxy_track_cost_callback.py @@ -269,20 +269,6 @@ class _ProxyDBLogger(CustomLogger): served_model_id=sl_object.get("model_id") if sl_object is not None else None, router=get_llm_router(), ) - # LIT-6314: post-call guardrail evaluations run after SLP is built, so - # guardrail_information is missing from SLP. Populate it from post-call evals - # before spend-log write so reports show accurate guardrail results. - if sl_object is not None: - litellm_metadata: Final = kwargs.get("litellm_metadata") - guardrail_info_from_hooks: Final = ( - litellm_metadata.get("standard_logging_guardrail_information") - if isinstance(litellm_metadata, dict) - else None - ) - if guardrail_info_from_hooks is not None and not sl_object.get("guardrail_information"): - sl_object["guardrail_information"] = ( - guardrail_info_from_hooks # mutable-ok: populate SLP before spend-log write - ) if response_cost is not None: user_api_key: Final = metadata.get("user_api_key", None) diff --git a/tests/test_litellm/llms/openai/chat/guardrail_translation/test_openai_guardrail_handler.py b/tests/test_litellm/llms/openai/chat/guardrail_translation/test_openai_guardrail_handler.py index a29e0be4655..24060e94e54 100644 --- a/tests/test_litellm/llms/openai/chat/guardrail_translation/test_openai_guardrail_handler.py +++ b/tests/test_litellm/llms/openai/chat/guardrail_translation/test_openai_guardrail_handler.py @@ -1559,3 +1559,53 @@ class TestScanOnlyToolResults: assert data["messages"][3]["content"] == "page says [BLOCKED] here" assert data["messages"][3]["tool_call_id"] == "call_1" assert data["messages"][4]["content"] == "and then?" + + +class TestNoScannableContentRecordsNotRun: + """LIT-6314: a guardrail whose scoping leaves nothing to scan must still persist an evaluation record""" + + def _system_only_data(self) -> dict: + return {"messages": [{"role": "system", "content": "SYSTEM-PROMPT"}]} + + def _recorded_entries(self, data: dict) -> list: + metadata = data.get("metadata") or data.get("litellm_metadata") or {} + return metadata.get("standard_logging_guardrail_information") or [] + + @pytest.mark.asyncio + async def test_skipped_scan_records_not_run_entry(self): + handler = OpenAIChatCompletionsHandler() + guardrail = MockGuardrail(guardrail_name="skip-system-guardrail") + guardrail.skip_system_message_in_guardrail = True + data = self._system_only_data() + + await handler.process_input_messages(data=data, guardrail_to_apply=guardrail) + + assert guardrail.last_inputs is None, "nothing survived scoping, apply_guardrail must not run" + entries = self._recorded_entries(data) + assert len(entries) == 1 + assert entries[0]["guardrail_name"] == "skip-system-guardrail" + assert entries[0]["guardrail_status"] == "not_run" + + @pytest.mark.asyncio + async def test_self_recording_guardrail_is_left_alone(self): + handler = OpenAIChatCompletionsHandler() + guardrail = MockGuardrail(guardrail_name="self-recording-guardrail") + guardrail.skip_system_message_in_guardrail = True + guardrail.records_own_guardrail_information = True + data = self._system_only_data() + + await handler.process_input_messages(data=data, guardrail_to_apply=guardrail) + + assert guardrail.last_inputs is None + assert self._recorded_entries(data) == [] + + @pytest.mark.asyncio + async def test_scannable_content_records_no_extra_entry(self): + handler = OpenAIChatCompletionsHandler() + guardrail = MockGuardrail(guardrail_name="normal-guardrail") + data = {"messages": [{"role": "user", "content": "hello"}]} + + await handler.process_input_messages(data=data, guardrail_to_apply=guardrail) + + assert guardrail.last_inputs is not None + assert all(e.get("guardrail_status") != "not_run" for e in self._recorded_entries(data)) diff --git a/tests/test_litellm/proxy/hooks/test_proxy_track_cost_callback.py b/tests/test_litellm/proxy/hooks/test_proxy_track_cost_callback.py index 2d5857f95e5..8043a1aca3f 100644 --- a/tests/test_litellm/proxy/hooks/test_proxy_track_cost_callback.py +++ b/tests/test_litellm/proxy/hooks/test_proxy_track_cost_callback.py @@ -1,3 +1,4 @@ + from datetime import datetime from unittest.mock import AsyncMock, MagicMock, patch @@ -69,7 +70,9 @@ async def test_async_post_call_failure_hook(): # Check that metadata was properly updated assert "litellm_params" in call_args["kwargs"] - assert call_args["kwargs"]["litellm_params"]["proxy_server_request"] == {"request_id": "test_request_id"} + assert call_args["kwargs"]["litellm_params"]["proxy_server_request"] == { + "request_id": "test_request_id" + } metadata = call_args["kwargs"]["litellm_params"]["metadata"] assert metadata["user_api_key"] == "test_api_key" assert metadata["status"] == "failure" @@ -333,7 +336,9 @@ async def test_should_continue_failure_tracking_when_budget_release_fails(): ) assert mock_invalidate_budget_reservation_counters.await_count == 1 assert ( - mock_invalidate_budget_reservation_counters.await_args.kwargs["budget_reservation"] + mock_invalidate_budget_reservation_counters.await_args.kwargs[ + "budget_reservation" + ] is user_api_key_dict.budget_reservation ) assert user_api_key_dict.budget_reservation["finalized"] is True @@ -428,21 +433,36 @@ def test_get_budget_reservation_from_metadata_handles_dict_auth_object(): "entries": [{"counter_key": "spend:key:test_api_key"}], } - assert _get_budget_reservation_from_metadata(metadata={"user_api_key_auth": dict(UserAPIKeyAuth())}) is None assert ( _get_budget_reservation_from_metadata( - metadata={"user_api_key_auth": UserAPIKeyAuth(budget_reservation=budget_reservation)} + metadata={"user_api_key_auth": dict(UserAPIKeyAuth())} + ) + is None + ) + assert ( + _get_budget_reservation_from_metadata( + metadata={ + "user_api_key_auth": UserAPIKeyAuth( + budget_reservation=budget_reservation + ) + } ) == budget_reservation ) assert ( _get_budget_reservation_from_metadata( - metadata={"user_api_key_auth": dict(UserAPIKeyAuth(budget_reservation=budget_reservation))} + metadata={ + "user_api_key_auth": dict( + UserAPIKeyAuth(budget_reservation=budget_reservation) + ) + } ) == budget_reservation ) assert ( - _get_budget_reservation_from_metadata(metadata={"user_api_key_budget_reservation": budget_reservation}) + _get_budget_reservation_from_metadata( + metadata={"user_api_key_budget_reservation": budget_reservation} + ) is budget_reservation ) @@ -450,7 +470,9 @@ def test_get_budget_reservation_from_metadata_handles_dict_auth_object(): @pytest.mark.asyncio async def test_update_database_and_spend_counters_releases_reservation_when_db_update_fails(): proxy_logging_obj = MagicMock() - proxy_logging_obj.db_spend_update_writer.update_database = AsyncMock(side_effect=Exception("db unavailable")) + proxy_logging_obj.db_spend_update_writer.update_database = AsyncMock( + side_effect=Exception("db unavailable") + ) increment_spend_counters = AsyncMock() budget_reservation = {"reserved_cost": 0.5, "entries": []} @@ -486,7 +508,9 @@ async def test_update_database_and_spend_counters_releases_reservation_when_db_u async def test_update_database_and_spend_counters_preserves_db_exception_when_release_fails(): proxy_logging_obj = MagicMock() db_exception = RuntimeError("db unavailable") - proxy_logging_obj.db_spend_update_writer.update_database = AsyncMock(side_effect=db_exception) + proxy_logging_obj.db_spend_update_writer.update_database = AsyncMock( + side_effect=db_exception + ) increment_spend_counters = AsyncMock() budget_reservation = {"reserved_cost": 0.5, "entries": []} @@ -530,8 +554,12 @@ async def test_update_database_and_spend_counters_preserves_db_exception_when_re budget_reservation=budget_reservation, ) assert mock_log_exception.call_count == 2 - mock_log_exception.assert_any_call("Failed to release budget reservation after database update failed") - mock_log_exception.assert_any_call("Failed to invalidate budget reservation counters after release failed") + mock_log_exception.assert_any_call( + "Failed to release budget reservation after database update failed" + ) + mock_log_exception.assert_any_call( + "Failed to invalidate budget reservation counters after release failed" + ) increment_spend_counters.assert_not_awaited() @@ -1073,7 +1101,10 @@ async def test_async_post_call_failure_hook_propagates_trace_id_from_logging_obj # standard_logging_object should have been propagated from logging obj assert call_kwargs.get("standard_logging_object") is not None - assert call_kwargs["standard_logging_object"]["trace_id"] == "trace-id-from-logging-obj" + assert ( + call_kwargs["standard_logging_object"]["trace_id"] + == "trace-id-from-logging-obj" + ) # litellm_trace_id should also be propagated as a fallback assert call_kwargs.get("litellm_trace_id") == "trace-id-from-logging-obj" @@ -1660,7 +1691,9 @@ async def test_async_post_call_failure_hook_records_recovered_partial_spend(): "metadata": {}, "proxy_server_request": {"request_id": "rid"}, "response_cost": 3.5e-05, - "combined_usage_object": Usage(prompt_tokens=30, completion_tokens=1, total_tokens=31), + "combined_usage_object": Usage( + prompt_tokens=30, completion_tokens=1, total_tokens=31 + ), } with patch( @@ -1739,10 +1772,15 @@ async def test_track_cost_callback_enriches_user_id_for_mcp_style_metadata(): assert mock_increment.call_args.kwargs["team_id"] == "team-123" assert mock_increment.call_args.kwargs["org_id"] == "org-456" - update_kwargs = mock_proxy_logging.db_spend_update_writer.update_database.await_args.kwargs + update_kwargs = ( + mock_proxy_logging.db_spend_update_writer.update_database.await_args.kwargs + ) assert update_kwargs["user_id"] == "mcp-user@example.com" assert update_kwargs["team_id"] == "team-123" - assert kwargs["litellm_params"]["metadata"]["user_api_key_user_id"] == "mcp-user@example.com" + assert ( + kwargs["litellm_params"]["metadata"]["user_api_key_user_id"] + == "mcp-user@example.com" + ) @pytest.mark.parametrize( @@ -1790,7 +1828,9 @@ def test_should_track_cost_callback_pass_through_without_owner(call_type, expect ], ) @pytest.mark.asyncio -async def test_track_cost_callback_logs_unauthenticated_pass_through_request(call_type, expect_spend_log): +async def test_track_cost_callback_logs_unauthenticated_pass_through_request( + call_type, expect_spend_log +): """Regression for LIT-3782: a pass-through request with auth=false reaches the cost callback with no key/user/team/end-user. Before the fix the spend-log write was skipped and the request never appeared in request/usage logs. It @@ -1836,7 +1876,9 @@ async def test_track_cost_callback_logs_unauthenticated_pass_through_request(cal end_time=datetime.now(), ) - assert mock_proxy_logging.db_spend_update_writer.update_database.await_count == (1 if expect_spend_log else 0) + assert mock_proxy_logging.db_spend_update_writer.update_database.await_count == ( + 1 if expect_spend_log else 0 + ) class _FakeDeploymentLookup: @@ -1947,61 +1989,3 @@ async def test_spend_counters_keep_every_granted_group_when_the_deployment_is_un ) assert charged == ("premium", "tier0") - - -@pytest.mark.asyncio -async def test_proxy_track_cost_callback_carries_guardrail_info_to_sl_object(): - """ - LIT-6314 regression: post-call guardrails append evaluations to - request_data["litellm_metadata"]["standard_logging_guardrail_information"], - but the standard_logging_object (built pre-post-call) lacks this field. - The callback must populate standard_logging_object["guardrail_information"] - from post-call evals before spend-log write so reports show results. - """ - logger = _ProxyDBLogger() - guardrail_info = [ - { - "guardrail_name": "bedrock-guard", - "guardrail_status": "success", - "guardrail_cost": 0.0001, - } - ] - sl_object = {"response_cost": 0.01} - kwargs = { - "model": "gpt-4", - "messages": [{"role": "user", "content": "Hello"}], - "litellm_metadata": {"standard_logging_guardrail_information": guardrail_info}, - "litellm_params": { - "metadata": {"user_api_key": "test_key"}, - }, - "call_type": CallTypes.completion.value, - "response_cost": 0.01, - "standard_logging_object": sl_object, - } - - with ( - patch( # test-quality-ok: spend counters are a proxy_server global the callback reads lazily, no seam - "litellm.proxy.proxy_server.increment_spend_counters", - new_callable=AsyncMock, - ), - patch( # test-quality-ok: update_cache is a proxy_server global the callback reads lazily, no seam - "litellm.proxy.proxy_server.update_cache", - new_callable=AsyncMock, - ), - patch( # test-quality-ok: callback imports proxy_logging_obj off proxy_server in its body, no seam - "litellm.proxy.proxy_server.proxy_logging_obj", - ) as mock_proxy_logging, - ): - mock_proxy_logging.db_spend_update_writer.update_database = AsyncMock() - mock_proxy_logging.slack_alerting_instance.customer_spend_alert = AsyncMock() - mock_proxy_logging.failed_tracking_alert = AsyncMock() - - await logger._PROXY_track_cost_callback( - kwargs=kwargs, - completion_response=None, - start_time=datetime.now(), - end_time=datetime.now(), - ) - - assert mock_proxy_logging.db_spend_update_writer.update_database.await_count == 1 - assert sl_object.get("guardrail_information") == guardrail_info