fix(guardrails): log full OpenAI Moderation response in post-call callbacks - #23554
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes two independent bugs that prevented OpenAI Moderation guardrail results from reaching downstream logging callbacks (Langfuse, Datadog, custom loggers) in the post-call path.
Key observations:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/guardrails/guardrail_hooks/openai/moderations.py | Bug 2 fix: stashes full moderation response in metadata and overrides _process_response/_process_error to log it. Early-return path (no text) correctly falls back to "allow". Cleanup via .pop() is correct when metadata is non-empty, but when metadata was never populated (e.g. non-dict request_data), _process_response silently skips cleanup without issue. |
| litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py | Single-line change forwarding real request data as request_data=data to process_output_response — core of Bug 1 fix, straightforward and correct. |
| litellm/llms/base_llm/guardrail_translation/base_translation.py | Adds optional request_data parameter to the abstract process_output_response signature — clean interface update, no behavioral changes. |
| litellm/llms/openai/chat/guardrail_translation/handler.py | When real request_data is provided, response is injected if absent but litellm_metadata is never populated — a behavioral change from the throwaway-dict path where litellm_metadata was always set from user_api_key_dict. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/openai/test_moderations.py | Three new mock-only tests covering safe content logging, harmful content intervention logging, and end-to-end request_data passthrough via UnifiedLLMGuardrails; harmful content test omits cleanup assertion for _openai_moderation_response. |
Sequence Diagram
sequenceDiagram
participant Proxy as Proxy Request (data dict)
participant UG as UnifiedLLMGuardrails
participant ET as EndpointTranslation<br/>(e.g. OpenAIChatCompletionsHandler)
participant OAM as OpenAIModerationGuardrail<br/>(@log_guardrail_information)
participant Dec as log_guardrail_information<br/>decorator
participant Log as StandardLogging<br/>(Langfuse / Datadog)
Note over Proxy,Log: Post-call guardrail flow (after Bug 1 + Bug 2 fixes)
Proxy->>UG: async_post_call_success_hook(data=request_data)
UG->>ET: process_output_response(..., request_data=data)
Note over ET: Bug 1 fix: passes real data dict<br/>instead of throwaway dict
ET->>Dec: apply_guardrail(inputs, request_data=data)
Dec->>OAM: apply_guardrail body runs
OAM->>OAM: async_make_request(text)
OAM->>OAM: stash moderation_response.model_dump()<br/>into data["metadata"]["_openai_moderation_response"]
Note over OAM: Bug 2 fix: full API response stashed
OAM-->>Dec: return inputs (or raise HTTPException)
alt Safe content
Dec->>OAM: _process_response(response=inputs, request_data=data)
OAM->>OAM: pop("_openai_moderation_response") → full dict
OAM->>Proxy: add_standard_logging_guardrail_information<br/>into data["metadata"] (real dict, not throwaway)
else Harmful content
Dec->>OAM: _process_error(e=HTTPException, request_data=data)
OAM->>OAM: pop("_openai_moderation_response") → full dict
OAM->>Proxy: add_standard_logging_guardrail_information<br/>with guardrail_intervened status
OAM-->>Dec: raise e
end
Proxy->>Log: StandardLoggingPayload includes full moderation response
Comments Outside Diff (3)
-
litellm/llms/openai/chat/guardrail_translation/handler.py, line 156-158 (link)litellm_metadatasilently dropped in proxy post-call pathIn the throwaway-dict path (
request_data is None),litellm_metadatais always populated fromuser_api_key_dictbeforeapply_guardrailis called. In the real-proxy path (request_datais the live dict), the new code only injectsresponsebut never injectslitellm_metadata. Any guardrail that readsrequest_data.get("litellm_metadata")(e.g. to enforce per-user policies duringapply_guardrail) will silently findNonein the proxy post-call path even thoughuser_api_key_dictis available.The same omission exists in all six handlers changed by this PR:
openai/chat,openai/completion,openai/responses,openai/transcriptions,anthropic/chat, anda2a/chat. -
tests/test_litellm/proxy/guardrails/guardrail_hooks/openai/test_moderations.py, line 626-632 (link)Missing cleanup assertion for harmful-content case
test_openai_moderation_guardrail_logs_full_response_safe_contentcorrectly asserts that_openai_moderation_responseis cleaned up from metadata after_process_responseruns (line 560). The harmful-content counterpart (test_openai_moderation_guardrail_logs_full_response_harmful_content) does not make the same assertion after_process_errorruns. If_process_errorever stopped calling.pop(), this test would not catch the regression.Consider adding:
# Internal key cleaned up by _process_error (.pop()) assert "_openai_moderation_response" not in request_data["metadata"]
-
litellm/proxy/guardrails/guardrail_hooks/openai/moderations.py, line 257-261 (link)or {}creates a detached dict when metadata is falsymetadata = ( request_data.get("metadata") or {} if isinstance(request_data, dict) else {} )
If
request_data["metadata"]exists but evaluates to a falsy value (e.g.0, empty string, orFalse),or {}creates a brand-new dict instead of returning the stored value. The.pop()on line 266 then operates on this orphan dict — leaving the realrequest_data["metadata"]unchanged and the_openai_moderation_responsekey un-cleaned.In practice the only falsy value that occurs is
{}(empty dict, meaning no API call was made), in which case there's nothing to clean up. But usingrequest_data.get("metadata", {})is more correct and avoids the ambiguity. The same pattern appears in_process_error(lines 298-302).
Last reviewed commit: 7d0856b
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _guardrails_event: Optional[asyncio.Event] = None | ||
| litellm_logging_obj = data.get("litellm_logging_obj") | ||
| if litellm_logging_obj is not None: | ||
| _guardrails_event = asyncio.Event() | ||
| litellm_logging_obj._post_call_guardrails_event = _guardrails_event |
There was a problem hiding this comment.
Event created unconditionally for every proxy request
The asyncio.Event is created and attached to every request that has a litellm_logging_obj, even when no guardrail callbacks are configured. This causes async_success_handler to serialize behind the full execution of post_call_success_hook (including the other_callbacks loop for non-guardrail loggers) for all proxy requests, not just those involving guardrails.
If a slow custom logger (in other_callbacks) takes several seconds, logging callbacks in async_success_handler are blocked for that duration on every request. Since the event is also only set after other_callbacks finish (not just after the guardrail loop), the synchronization scope is wider than necessary.
A more targeted approach would only create the event when there are actually guardrail callbacks that will run:
# Only introduce the synchronisation point when there are
# guardrails that will write to request_data["metadata"].
_guardrails_event: Optional[asyncio.Event] = None
litellm_logging_obj = data.get("litellm_logging_obj")
# Determine upfront whether any guardrail will actually run
_has_guardrail_callbacks = any(
isinstance(cb, CustomGuardrail)
and cb.should_run_guardrail(data=data, event_type=GuardrailEventHooks.post_call)
for cb in litellm.callbacks
if not isinstance(cb, str)
)
if litellm_logging_obj is not None and _has_guardrail_callbacks:
_guardrails_event = asyncio.Event()
litellm_logging_obj._post_call_guardrails_event = _guardrails_eventAt minimum, the event should be set immediately after the guardrail loop (before other_callbacks) rather than after all callbacks complete, so non-guardrail custom loggers don't delay the background logging task.
| # Stash full moderation response in request_data for logging | ||
| # (Model Armor pattern — avoids race conditions from storing on self) | ||
| if isinstance(request_data, dict): | ||
| metadata = request_data.setdefault("metadata", {}) | ||
| metadata["_openai_moderation_response"] = moderation_response.model_dump() |
There was a problem hiding this comment.
Internal key _openai_moderation_response persists in metadata
The raw model_dump() of the moderation response is stashed into request_data["metadata"]["_openai_moderation_response"] for use by _process_response / _process_error, but it is never removed after being consumed. This means the full moderation payload (id, model, all categories, all scores) becomes a permanent fixture in the request metadata that flows to every downstream logger (Langfuse, Datadog, custom loggers).
Consider deleting the key in _process_response and _process_error after extracting it, or using .pop() at extraction time:
# In _process_response / _process_error
guardrail_response = metadata.pop("_openai_moderation_response", {})This mirrors the "use-once" pattern already employed by Model Armor and keeps metadata clean for downstream consumers.
| def _process_response( | ||
| self, | ||
| response: Optional[Dict], | ||
| request_data: dict, | ||
| start_time: Optional[float] = None, | ||
| end_time: Optional[float] = None, | ||
| duration: Optional[float] = None, | ||
| event_type: Optional[GuardrailEventHooks] = None, | ||
| original_inputs: Optional[Dict] = None, | ||
| ): | ||
| """ | ||
| Override to log the full OpenAI Moderation API response instead of | ||
| the decorator's simplified "allow"/"mask" string. | ||
|
|
||
| Follows the Model Armor pattern (model_armor.py:325-360). | ||
| """ | ||
| metadata = ( | ||
| request_data.get("metadata", {}) if isinstance(request_data, dict) else {} | ||
| ) | ||
|
|
||
| guardrail_response = metadata.get("_openai_moderation_response", {}) | ||
|
|
||
| self.add_standard_logging_guardrail_information_to_request_data( | ||
| guardrail_json_response=guardrail_response, | ||
| request_data=request_data, | ||
| guardrail_status="success", | ||
| duration=duration, | ||
| start_time=start_time, | ||
| end_time=end_time, | ||
| event_type=event_type, | ||
| ) | ||
| return response |
There was a problem hiding this comment.
Empty-dict logged when no text is available to moderate
apply_guardrail returns early (without making an API call) when text_to_moderate is None or empty:
if not text_to_moderate:
return inputs # _openai_moderation_response never stashedThe @log_guardrail_information decorator still invokes _process_response for this early-return path. Because _openai_moderation_response was never set in metadata, _process_response logs guardrail_response = {} (empty dict) with guardrail_status = "success". The base-class implementation would log "allow" in this case, which is more informative and consistent.
You should guard against this or fall back to "allow" when the stash is absent:
guardrail_response = metadata.get("_openai_moderation_response") or "allow"
|
|
||
| # Step 2: Apply guardrail to all texts and tool calls in batch | ||
| if texts_to_check or tool_calls_to_check: | ||
| # Create a request_data dict with response info and user API key metadata | ||
| request_data: dict = {"response": response} | ||
| if request_data is None: | ||
| # Create a request_data dict with response info and user API key metadata | ||
| request_data = {"response": response} | ||
|
|
||
| # Add user API key metadata with prefixed keys | ||
| user_metadata = self.transform_user_api_key_dict_to_metadata( | ||
| user_api_key_dict | ||
| ) | ||
| if user_metadata: | ||
| request_data["litellm_metadata"] = user_metadata | ||
| # Add user API key metadata with prefixed keys | ||
| user_metadata = self.transform_user_api_key_dict_to_metadata( | ||
| user_api_key_dict | ||
| ) | ||
| if user_metadata: | ||
| request_data["litellm_metadata"] = user_metadata | ||
|
|
||
| inputs = GenericGuardrailAPIInputs(texts=texts_to_check) | ||
| if images_to_check: |
There was a problem hiding this comment.
request_data["response"] no longer populated when real request_data is passed
When request_data is provided (the fix for Bug 1), the handlers skip adding "response" to it. However, CrowdStrikeAIDRGuardrail._build_guard_input_for_response (at crowdstrike_aidr.py:163) explicitly reads request_data.get("response") to build the guard payload for post-call checks. With this change, that guardrail will always log a warning and return None for output responses, silently disabling its post-call inspection.
The same pattern applies to all six handler files in this PR (openai/chat, openai/completion, openai/responses, openai/transcriptions, anthropic/chat, a2a/chat).
To maintain backward compatibility, the real response object should be injected into the provided request_data if it isn't already present:
if request_data is None:
request_data = {"response": response}
user_metadata = self.transform_user_api_key_dict_to_metadata(user_api_key_dict)
if user_metadata:
request_data["litellm_metadata"] = user_metadata
else:
# Inject the response so downstream guardrails can read request_data["response"]
if "response" not in request_data:
request_data["response"] = response
user_metadata = self.transform_user_api_key_dict_to_metadata(user_api_key_dict)
if user_metadata and "litellm_metadata" not in request_data:
request_data["litellm_metadata"] = user_metadataThis same fix is needed across all six translation handlers.
| # Re-read guardrail info from metadata into the SLO now that | ||
| # guardrails are done. | ||
| slo = self.model_call_details.get("standard_logging_object") | ||
| if isinstance(slo, dict): | ||
| metadata = self.model_call_details.get("litellm_params", {}).get( | ||
| "metadata", {} | ||
| ) | ||
| guardrail_info = metadata.get( | ||
| "standard_logging_guardrail_information" | ||
| ) | ||
| if guardrail_info: | ||
| slo["guardrail_information"] = guardrail_info |
There was a problem hiding this comment.
model_call_details["litellm_params"]["metadata"] may not share identity with proxy data["metadata"]
After unblocking on the event, guardrail info is re-read from self.model_call_details["litellm_params"]["metadata"]. However, model_call_details is populated from the kwargs captured at the time the Logging object is created, while add_standard_logging_guardrail_information_to_request_data writes into data["metadata"] (the live proxy request dict). If these two metadata dicts are different Python objects (e.g., copied rather than shared by reference during request setup), the re-read will find guardrail_info = None and silently skip the SLO update.
The direct SLO patch already performed by proxy/utils.py (before signalling the event) provides a safety net, but the re-read path provides no value and gives a false sense of redundancy. Consider verifying that the two metadata dicts share identity, or removing the re-read in favour of relying solely on the direct patch in proxy/utils.py.
# Re-read path is only reliable if these are the same dict object:
# self.model_call_details["litellm_params"]["metadata"] ←→ data["metadata"]
# The proxy/utils.py already patches slo["guardrail_information"] before
# setting the event, so this re-read is only needed as a fallback.
2ec73c4 to
099e400
Compare
099e400 to
a996fa4
Compare
a996fa4 to
78761e9
Compare
78761e9 to
6196549
Compare
…ed for logging Two independent bugs prevented the OpenAI Moderation guardrail response from reaching downstream loggers (e.g. async_log_success_event) when configured in post_call mode: 1. Lost request_data: The unified guardrail dispatcher's post-call path created a throwaway dict in each translation handler instead of forwarding the real proxy request_data. Guardrail logging info was written to a discarded object. Fix: thread request_data through process_output_response in all 13 BaseTranslation handlers. 2. Response reduced to "allow": The @log_guardrail_information decorator collapsed the full moderation API response (categories, scores, flagged) to the string "allow". Fix: override _process_response/_process_error on OpenAIModerationGuardrail (Model Armor pattern) to log the full moderation response dict.
6196549 to
7d0856b
Compare
|
@greptileai is it fine now? I'll fix bug 3 in a separate PR, if so please adjust the confidence score and review accordingly |
|
Yes, this looks good! The PR is well-structured with clear fixes for both bugs: Bug 1 (request_data threading):
Bug 2 (full moderation response):
Tests:
Separating Bug 3: The only minor note: the PR description mentions "13 translation handlers" but you're updating 15 files (13 handlers + base + unified_guardrail). Just a documentation detail, doesn't affect the code. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
1 participant
Post-call OpenAI Moderation guardrail results were not reaching downstream logging callbacks (Langfuse, Datadog, custom loggers). Two independent bugs:
process_output_response()created a throwawayrequest_datadict, so guardrail info written by the@log_guardrail_informationdecorator was lost before reachingStandardLoggingPayload. Fixed by threading the realrequest_datafrom the unified guardrail dispatcher through all 13BaseTranslationhandlers.The
@log_guardrail_informationdecorator reduced the full moderation API response (categories, scores, flagged status) to the string"allow". Fixed by overriding_process_responseand_process_erroronOpenAIModerationGuardrailto stash and log the full response (following the established Model Armor pattern).Relevant issues
Related to #13690 (Bedrock guardrail response logging — same class of issue) and #13321 (Bedrock Guardrail Error Response Incomplete).
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🐛 Bug Fix
Changes
Bug 1: Thread
request_datathrough post-call dispatch chain (15 files)litellm/llms/base_llm/guardrail_translation/base_translation.py— Addrequest_data: Optional[dict] = Noneparameter to abstractprocess_output_response()litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py— Forwarddataasrequest_data=datainasync_post_call_success_hookrequest_dataparameter. The 6 handlers that create a throwaway dict (openai/chat,openai/responses,openai/completion,openai/transcriptions,anthropic/chat,a2a/chat) now use the real dict when provided and injectresponsefor CrowdStrike compatibility. The remaining 7 handlers (pass_through,openai/speech,openai/embeddings,openai/image_generation,cohere/rerank,mistral/ocr,mcp_server) accept the parameter for signature compatibility.Bug 2: Log full moderation response (1 file)
litellm/proxy/guardrails/guardrail_hooks/openai/moderations.py— Stashmoderation_response.model_dump()inrequest_data["metadata"]duringapply_guardrail, override_process_responseand_process_errorto log full moderation API response (categories, category_scores, flagged) instead of"allow"string. Uses.pop()for cleanup so the internal key doesn't leak to downstream loggers. Follows established Model Armor pattern.Tests (1 file)
tests/test_litellm/proxy/guardrails/guardrail_hooks/openai/test_moderations.py— 3 new tests:test_openai_moderation_guardrail_logs_full_response_safe_content— verifies full moderation dict (not"allow") inguardrail_responsetest_openai_moderation_guardrail_logs_full_response_harmful_content— verifiesguardrail_intervenedstatus with full response (not exception string)test_openai_moderation_post_call_request_data_passthrough— verifies guardrail info flows throughUnifiedLLMGuardrailsto realrequest_dataNot in this PR
The timing race between the background logging task and post-call guardrails (Bug 3) will be addressed in a separate PR. This PR ensures that when guardrail info is written, it goes to the right place (Bug 1) with the right content (Bug 2).