fix(proxy): post-call guardrail response not captured for logging - #23910
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR correctly fixes two independent bugs that prevented post-call OpenAI Moderation guardrail results from reaching downstream logging callbacks (Langfuse, Datadog, custom loggers). Bug 1 — Throwaway Bug 2 — Full moderation response reduced to Key observations:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/guardrails/guardrail_hooks/openai/moderations.py | Core of bug-fix #2: adds apply_guardrail stashing of full moderation API response, and overrides _process_response/_process_error to recover and log it via the Model Armor pattern. Implementation is correct; or {} guard safely handles metadata=None. One minor edge case: the stash can be lost when apply_guardrail is called with a falsy empty dict (SDK/direct-call path only, not proxy path). |
| litellm/proxy/guardrails/guardrail_hooks/unified_guardrail/unified_guardrail.py | Core of bug-fix #1: three call sites in async_post_call_success_hook and async_post_call_streaming_iterator_hook now pass request_data=data (the real per-request dict) instead of a throwaway, ensuring guardrail info written by the decorator is visible to downstream logging. |
| litellm/llms/base_llm/guardrail_translation/base_translation.py | Interface change: process_output_response and process_output_streaming_response abstract methods each gain a new optional request_data: Optional[dict] = None parameter, making the API backward-compatible. All 13 concrete handler subclasses are updated consistently. |
| litellm/llms/openai/chat/guardrail_translation/handler.py | Uses the real request_data when provided (proxy path) and falls back to a local dict when None (SDK path). litellm_metadata injection is preserved for third-party guardrails via if "litellm_metadata" not in request_data guard. Both non-streaming and streaming paths are updated. |
| litellm/llms/anthropic/chat/guardrail_translation/handler.py | Consistent pattern with OpenAI chat handler. Streaming path previously passed hard-coded request_data={} to apply_guardrail; now threads the real dict through, or falls back to {} when request_data is None. |
| litellm/litellm_core_utils/litellm_logging.py | Pure cosmetic reformatting — Black-style multi-line assignments rewritten to parenthesised single-line form throughout. No functional changes whatsoever. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/openai/test_moderations.py | Five new tests added (all mock-based, no real network calls): safe-content full response logging, harmful-content full response logging, post-call request_data passthrough, and two edge cases for metadata=None in _process_response/_process_error. Good coverage of the two bug fixes. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/openai/test_openai_moderation_streaming.py | Adds streaming end-of-stream request_data passthrough test that verifies guardrail info lands in the real request_data dict (not a throwaway) after the streaming iterator hook completes. Mock-based, no real network calls. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/unified_guardrails/test_unified_guardrail.py | Two modifications: adds request_data=None to an inline mock's signature to match the updated base class interface, and reformats inline list/dict literals. No assertions weakened. |
| litellm/proxy/utils.py | Purely cosmetic reformatting of multi-line dict-access assignments and SQL heredocs. No functional changes. |
Sequence Diagram
sequenceDiagram
participant LB as LiteLLM Proxy
participant UG as UnifiedLLMGuardrails
participant HN as BaseTranslation Handler<br/>(e.g. OpenAIChatCompletionsHandler)
participant OM as OpenAIModerationGuardrail
participant Dec as @log_guardrail_information<br/>decorator
participant SL as StandardLoggingPayload<br/>(Langfuse / DD / OTEL)
Note over LB,SL: BUG 1 FIX: Real request_data threaded through
LB->>UG: async_post_call_success_hook(data=request_data, response)
UG->>HN: process_output_response(response, guardrail, request_data=data)
Note over HN: Old: request_data = {} (throwaway)<br/>New: request_data = passed-through real dict
HN->>Dec: apply_guardrail(inputs, request_data=request_data)
Note over Dec,OM: BUG 2 FIX: Full moderation response preserved
Dec->>OM: apply_guardrail body executes
OM->>OM: async_make_request(text)
OM->>OM: stash: request_data["metadata"]["_openai_moderation_response"] = full_response
alt Content not flagged
OM-->>Dec: return inputs
Dec->>OM: _process_response(request_data=request_data)
OM->>OM: metadata.pop("_openai_moderation_response", "allow") → full_response
OM->>OM: add_standard_logging_guardrail_information_to_request_data(full_response, ...)
OM-->>HN: return inputs
HN-->>UG: return response
UG-->>SL: request_data["metadata"]["standard_logging_guardrail_information"] = [full_response]
else Content flagged (HTTPException 400)
OM->>Dec: raise HTTPException
Dec->>OM: _process_error(e, request_data=request_data)
OM->>OM: metadata.pop("_openai_moderation_response", e) → full_response
OM->>OM: add_standard_logging_guardrail_information_to_request_data(full_response, "guardrail_intervened")
OM->>Dec: raise e
Dec->>UG: propagate HTTPException
end
Comments Outside Diff (2)
-
litellm/llms/anthropic/chat/guardrail_translation/handler.py, line 628 (link)Falsy empty-dict silently drops stashed moderation response in SDK path
When
request_data is None,request_data if request_data is not None else {}passes a fresh empty{}toapply_guardrail. Because{}is falsy, the@log_guardrail_informationdecorator evaluateskwargs.get("request_data") or {}as a second fresh{}— a different object.apply_guardrailstashes_openai_moderation_responsein the first{}, but_process_responsepops from the second{}and falls back to"allow", silently discarding the full moderation response.This only affects the direct SDK / non-proxy path (the proxy always passes a non-empty dict). The same pattern appears in
litellm/llms/openai/responses/guardrail_translation/handler.pyat threeapply_guardrailcall-sites (lines 934, 943, and 952).A safer fallback would make it explicit:
Using
request_data or {}is identical torequest_data if request_data is not None else {}but makes it clear the caller accepts the falsy-empty behaviour. Alternatively, a non-empty sentinel (e.g.{"_sdk_path": True}) would let the decorator resolve to the same reference; this would fully fix the SDK path as well. -
litellm/proxy/guardrails/guardrail_hooks/openai/moderations.py, line 315-322 (link)_process_errorraises before returning a value — callers cannot use the return value_process_erroralways ends withraise e, soreturn self._process_error(...)in the@log_guardrail_informationdecorator never evaluates to a value. This is functionally correct (the exception propagates), but the method's declared return type implicitly promises a value. The base class_process_errorhas the same shape, so this is not a regression — just worth noting for clarity.If
add_standard_logging_guardrail_information_to_request_dataitself raises (unlikely but possible), it would suppresseand surface the internal error instead. A defensive approach would be:try: self.add_standard_logging_guardrail_information_to_request_data(...) finally: raise e
This is a low-priority style note, consistent with the Model Armor pattern the PR follows.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (7): Last reviewed commit: "fix(test): add request_data param to tes..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| metadata = ( | ||
| request_data.get("metadata", {}) if isinstance(request_data, dict) else {} | ||
| ) | ||
|
|
||
| # .pop() cleans up the internal key so it doesn't leak to downstream | ||
| # loggers. Falls back to "allow" when no moderation call was made | ||
| # (e.g. no text to moderate — early return in apply_guardrail). | ||
| guardrail_response = metadata.pop("_openai_moderation_response", "allow") |
There was a problem hiding this comment.
metadata.pop() raises AttributeError when request_data["metadata"] is None
dict.get(key, default) returns the stored value even when it is None; the default only applies when the key is absent. So when request_data = {"metadata": None}, request_data.get("metadata", {}) returns None, and the subsequent .pop() call raises AttributeError: 'NoneType' object has no attribute 'pop'.
The base-class method add_standard_logging_guardrail_information_to_request_data already guards against this case (it resets None to {}), but both overrides perform .pop() before that guard runs.
The same issue exists in _process_error at line 301.
# Current (fragile):
metadata = (
request_data.get("metadata", {}) if isinstance(request_data, dict) else {}
)
guardrail_response = metadata.pop("_openai_moderation_response", "allow")| metadata = ( | |
| request_data.get("metadata", {}) if isinstance(request_data, dict) else {} | |
| ) | |
| # .pop() cleans up the internal key so it doesn't leak to downstream | |
| # loggers. Falls back to "allow" when no moderation call was made | |
| # (e.g. no text to moderate — early return in apply_guardrail). | |
| guardrail_response = metadata.pop("_openai_moderation_response", "allow") | |
| metadata = ( | |
| request_data.get("metadata") or {} | |
| if isinstance(request_data, dict) | |
| else {} | |
| ) | |
| # .pop() cleans up the internal key so it doesn't leak to downstream | |
| # loggers. Falls back to "allow" when no moderation call was made | |
| # (e.g. no text to moderate — early return in apply_guardrail). | |
| guardrail_response = metadata.pop("_openai_moderation_response", "allow") |
0bc16b0 to
0f13c99
Compare
| 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.
setdefault does not guard against metadata = None
dict.setdefault(key, default) only sets the key when it is absent. When request_data = {"metadata": None} the key is already present, so setdefault returns None — and the immediately following item-assignment metadata["_openai_moderation_response"] = ... raises TypeError: 'NoneType' object does not support item assignment.
This causes the guardrail to fail entirely (a TypeError bubbles up instead of the content-moderation result), which is the same class of defect that was fixed in _process_response / _process_error with or {}. The same guard is needed here:
# Current (fragile):
if isinstance(request_data, dict):
metadata = request_data.setdefault("metadata", {})
metadata["_openai_moderation_response"] = moderation_response.model_dump()| if isinstance(request_data, dict): | |
| metadata = request_data.setdefault("metadata", {}) | |
| metadata["_openai_moderation_response"] = moderation_response.model_dump() | |
| # Stash full moderation response in request_data for logging | |
| # (Model Armor pattern — per-request dict avoids race conditions) | |
| if isinstance(request_data, dict): | |
| existing = request_data.get("metadata") | |
| if not isinstance(existing, dict): | |
| request_data["metadata"] = {} | |
| request_data["metadata"]["_openai_moderation_response"] = moderation_response.model_dump() |
This mirrors the or {} guard already used in _process_response (line 258) and _process_error (line 297), and the null-reset already performed in add_standard_logging_guardrail_information_to_request_data (custom_guardrail.py:627-628).
0f13c99 to
dbff310
Compare
…ed for logging Two independent bugs prevented post-call OpenAI Moderation guardrail results from reaching downstream logging callbacks (Langfuse, Datadog). Bug 1: process_output_response() created a throwaway request_data dict, so guardrail info written by @log_guardrail_information was discarded. Fixed by threading the real request_data from the unified guardrail dispatcher through all 13 BaseTranslation handlers, with litellm_metadata injection preserved for third-party guardrails (Zscaler, Prompt Security). Also extended to process_output_streaming_response for consistency. Bug 2: The @log_guardrail_information decorator collapsed the full moderation API response (categories, scores, flagged status) to "allow". Fixed by overriding _process_response/_process_error on OpenAIModerationGuardrail to stash and log the full response, following the established Model Armor pattern.
…kward compat, test coverage - Pass request_data to end-of-stream process_output_streaming_response call - Restore inputs.update() in OCR handler for third-party guardrail providers - Add streaming end-to-end test for guardrail logging passthrough
…so pop() mutates the real dict
05d8d7c to
fa7ccf0
Compare
…post-call-logging fix(proxy): post-call guardrail response not captured for logging
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, withlitellm_metadatainjection preserved for third-party guardrails that read it during post-call (Zscaler, Prompt Security, Generic Guardrail API, Gray Swan).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). Uses.pop()for cleanup so internal keys don't leak to downstream loggers.Relevant issues
Related to #13690 (Bedrock guardrail response logging — same class of issue) and #13321 (Bedrock Guardrail Error Response Incomplete).
Supersedes #23554 (same fixes, cleaner implementation addressing all review feedback).
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)