-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix(guardrails): walk Responses-API text taxonomy in shared content helpers #32542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4df1417
0cfaf50
20dc5a3
7e42d65
04bd0fc
ff5823d
a5bfcc4
18d3a6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,9 @@ def is_text_content_call_type(call_type: str) -> bool: | |
| return call_type in TEXT_CONTENT_CALL_TYPES | ||
|
|
||
|
|
||
| TEXT_PART_TYPES: FrozenSet[str] = frozenset({"text", "input_text", "output_text"}) | ||
|
|
||
|
|
||
| def _iter_text_parts_in_content(content: Any) -> Iterator[str]: | ||
| """Yield text fragments from a ``message.content`` value (string or | ||
| multimodal list). Non-text parts (images, audio, …) are skipped.""" | ||
|
|
@@ -48,7 +51,7 @@ def _iter_text_parts_in_content(content: Any) -> Iterator[str]: | |
| continue | ||
| if not isinstance(part, dict): | ||
| continue | ||
| if part.get("type") == "text": | ||
| if part.get("type") in TEXT_PART_TYPES: | ||
| text = part.get("text") | ||
| if isinstance(text, str) and text: | ||
| yield text | ||
|
|
@@ -58,14 +61,20 @@ def _coerce_input_to_messages(input_value: Any) -> List[Dict[str, Any]]: | |
| """Coerce a Responses-API ``data["input"]`` value into chat-style messages.""" | ||
| if isinstance(input_value, str): | ||
| return [{"role": "user", "content": input_value}] | ||
| if isinstance(input_value, list): | ||
| if input_value and all(isinstance(item, dict) and "role" in item for item in input_value): | ||
| return list(input_value) | ||
| # Mixed lists (content-part dicts + bare strings) and pure | ||
| # string/dict lists all become a single user message; the content | ||
| # iterator below handles each element type uniformly. | ||
| return [{"role": "user", "content": input_value}] | ||
| return [] | ||
| if not isinstance(input_value, list): | ||
| return [] | ||
| messages: List[Dict[str, Any]] = [] | ||
|
ryan-crabbe-berri marked this conversation as resolved.
|
||
| for item in input_value: | ||
| if isinstance(item, str): | ||
| messages.append({"role": "user", "content": item}) | ||
| elif isinstance(item, dict): | ||
| if item.get("type") in TEXT_PART_TYPES: | ||
| messages.append({"role": item.get("role") or "user", "content": [item]}) | ||
| elif "content" in item: | ||
| messages.append({"role": item.get("role") or "user", "content": item["content"]}) | ||
| elif item.get("type") == "function_call_output" and "output" in item: | ||
| messages.append({"role": item.get("role") or "tool", "content": item["output"]}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cato analyze missing role coercionMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 18d3a6f. Configure here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not a regression, Cato skipped check before. |
||
| return messages | ||
|
|
||
|
|
||
| def _iter_inspection_messages(data: Dict[str, Any]) -> Iterator[Dict[str, Any]]: | ||
|
|
@@ -112,7 +121,7 @@ def _rewrite_content(content: Any) -> Any: | |
| new_parts.append(visit(part)) | ||
| elif ( | ||
| isinstance(part, dict) | ||
| and part.get("type") == "text" | ||
| and part.get("type") in TEXT_PART_TYPES | ||
| and isinstance(part.get("text"), str) | ||
| and part["text"] | ||
| ): | ||
|
|
@@ -136,25 +145,20 @@ def _rewrite_content(content: Any) -> Any: | |
| data["input"] = visit(input_value) | ||
| return visited | ||
| if isinstance(input_value, list): | ||
| # List of full messages: rewrite each message's content. | ||
| if input_value and all(isinstance(item, dict) and "role" in item for item in input_value): | ||
| for item in input_value: | ||
| if "content" in item: | ||
| item["content"] = _rewrite_content(item["content"]) | ||
| return visited | ||
| # List of content parts and/or bare strings: rewrite in place. | ||
| for idx, item in enumerate(input_value): | ||
| if isinstance(item, str) and item: | ||
| visited += 1 | ||
| input_value[idx] = visit(item) | ||
| elif ( | ||
| isinstance(item, dict) | ||
| and item.get("type") == "text" | ||
| and isinstance(item.get("text"), str) | ||
| and item["text"] | ||
| ): | ||
| visited += 1 | ||
| input_value[idx] = {**item, "text": visit(item["text"])} | ||
| if isinstance(item, str): | ||
| if item: | ||
| visited += 1 | ||
| input_value[idx] = visit(item) | ||
| elif isinstance(item, dict): | ||
| if item.get("type") in TEXT_PART_TYPES: | ||
| if isinstance(item.get("text"), str) and item["text"]: | ||
| visited += 1 | ||
| input_value[idx] = {**item, "text": visit(item["text"])} | ||
| elif "content" in item: | ||
| item["content"] = _rewrite_content(item["content"]) | ||
| elif item.get("type") == "function_call_output" and "output" in item: | ||
| item["output"] = _rewrite_content(item["output"]) | ||
| return visited | ||
|
|
||
| return visited | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| """Tests for the AIM guardrail's inspection-payload construction.""" | ||
|
|
||
| from litellm.proxy.guardrails.guardrail_hooks.aim.aim import AimGuardrail | ||
|
|
||
|
|
||
| def test_aim_inspection_messages_coerces_chat_completions_tool_role_to_user(): | ||
| """LIT-4294: A valid chat-completions ``role: "tool"`` message carries a | ||
| ``tool_call_id``, but the inspection flatten drops every field except | ||
| ``role`` and ``content``. A bare ``tool`` message without ``tool_call_id`` | ||
| is schema-invalid per the OpenAI chat schema, and the customer's writeup | ||
| reproduced AIM's ``/fw/v1/analyze`` returning 422 on exactly that shape. | ||
| The AIM POST collapses the role to ``user``; the outbound request to the | ||
| LLM is untouched.""" | ||
| data = { | ||
| "messages": [ | ||
| {"role": "user", "content": "weather in SF"}, | ||
| { | ||
| "role": "assistant", | ||
| "content": "", | ||
| "tool_calls": [ | ||
| { | ||
| "id": "c1", | ||
| "type": "function", | ||
| "function": {"name": "get_weather", "arguments": "{}"}, | ||
| } | ||
| ], | ||
| }, | ||
| {"role": "tool", "tool_call_id": "c1", "content": "sunny"}, | ||
| ] | ||
| } | ||
| assert AimGuardrail._build_aim_inspection_messages(data) == [ | ||
| {"role": "user", "content": "weather in SF"}, | ||
| {"role": "user", "content": "sunny"}, | ||
| ] | ||
|
|
||
|
|
||
| def test_aim_inspection_messages_coerces_non_standard_caller_role_to_user(): | ||
| """LIT-4294: A caller-supplied role outside {system, user, assistant} | ||
| (e.g. ``developer``, ``function``) is coerced to ``user`` for the AIM | ||
| POST, since AIM validates the payload against the OpenAI chat schema | ||
| and rejects unknown roles the same way it rejects bare ``tool``.""" | ||
| data = { | ||
| "messages": [ | ||
| {"role": "developer", "content": "system-ish instruction"}, | ||
| {"role": "user", "content": "normal user text"}, | ||
| ] | ||
| } | ||
| assert AimGuardrail._build_aim_inspection_messages(data) == [ | ||
| {"role": "user", "content": "system-ish instruction"}, | ||
| {"role": "user", "content": "normal user text"}, | ||
| ] | ||
|
|
||
|
|
||
| def test_aim_inspection_messages_coerces_responses_function_call_output_role(): | ||
| """LIT-4294: the shared helper synthesises ``role: "tool"`` for a | ||
| Responses ``function_call_output`` item (semantic equivalent of | ||
| chat-completions tool messages). AIM's schema-validating POST cannot | ||
| carry ``tool_call_id`` in the flat inspection payload, so AIM collapses | ||
| that ``tool`` role to ``user`` locally before POSTing.""" | ||
| data = { | ||
| "input": [ | ||
| { | ||
| "type": "function_call_output", | ||
| "call_id": "c1", | ||
| "output": [{"type": "input_text", "text": "sunny"}], | ||
| }, | ||
| ] | ||
| } | ||
| assert AimGuardrail._build_aim_inspection_messages(data) == [ | ||
| {"role": "user", "content": "sunny"}, | ||
| ] | ||
|
|
||
|
|
||
| def test_aim_inspection_messages_preserves_safe_roles(): | ||
| """Safe roles pass through untouched — the coercion only fires for | ||
| roles the OpenAI chat schema flatten cannot represent standalone.""" | ||
| data = { | ||
| "messages": [ | ||
| {"role": "system", "content": "be helpful"}, | ||
| {"role": "user", "content": "hi"}, | ||
| {"role": "assistant", "content": "hello"}, | ||
| ] | ||
| } | ||
| assert AimGuardrail._build_aim_inspection_messages(data) == [ | ||
| {"role": "system", "content": "be helpful"}, | ||
| {"role": "user", "content": "hi"}, | ||
| {"role": "assistant", "content": "hello"}, | ||
| ] |


Uh oh!
There was an error while loading. Please reload this page.