[https://nvbugs/6384951] Fix incorrect error reporting

2ez4bz · 2ez4bz · commit 739d5760254d · 2026-07-01T21:18:46.000-07:00
* Why?

tau2-bench required a workaround that could misreport errors.

* What?

This commit fixes that with typed checks and adds tests for it.

Signed-off-by: William Zhang &lt;133824995+2ez4bz@users.noreply.github.com&gt;
diff --git a/tensorrt_llm/serve/chat_utils.py b/tensorrt_llm/serve/chat_utils.py
@@ -11,6 +11,7 @@
     ChatCompletionContentPartParam as OpenAIChatCompletionContentPartParam
 from openai.types.chat import (ChatCompletionContentPartTextParam,
                                ChatCompletionMessageParam)
+from pydantic import BaseModel, ConfigDict, TypeAdapter, ValidationError
 from transformers import AutoConfig
 from typing_extensions import Required
 
@@ -270,6 +271,109 @@ def parse_chat_message_content(
     return result
 
 
+# The below 2 models are for tau2-bench workarounds (see `_parse_fallback_tool_calls` for details).
+class _FallbackFunctionCall(BaseModel):
+    """Lenient, typed view of a tool call's `function` for the fallback path."""
+    model_config = ConfigDict(extra="allow")
+
+    name: str | None = None
+    arguments: str | dict[str, Any] | None = None
+
+
+class _FallbackToolCall(BaseModel):
+    """Lenient, typed view of a single tool call for the fallback path."""
+    model_config = ConfigDict(extra="allow")
+
+    id: str | None = None
+    type: str | None = None
+    function: _FallbackFunctionCall
+
+
+_FALLBACK_TOOL_CALLS_ADAPTER = TypeAdapter(list[_FallbackToolCall])
+
+
+def _format_tool_call_error(errors: list[dict[str, Any]]) -> str:
+    """Render the first Pydantic error for fallback tool calls as a client-facing message."""
+    err = errors[0]
+    loc = err.get("loc", ())
+    index = loc[0] if loc and isinstance(loc[0], int) else 0
+    error_type = err.get("type", "")
+    # Path within the tool call, after the leading list index.
+    field = ".".join(str(p) for p in loc[1:])
+    target = f"tool_calls[{index}].{field}" if field else f"tool_calls[{index}]"
+
+    if error_type == "missing":
+        return f"{target} is required."
+    # Pydantic's default message for these leaks the internal model name, so phrase the "value must
+    # be an object" case explicitly.
+    if error_type in ("model_type", "dict_type", "model_attributes_type"):
+        return f"{target} must be an object."
+    return f"{target} is invalid: {err.get('msg', 'validation error')}"
+
+
+def _validate_fallback_tool_calls(
+        tool_calls: list[Any]) -> list[dict[str, Any]]:
+    """Re-validate raw, Pydantic-rejected tool calls from the openai_server through the model above.
+
+    This keeps the tau2-bench leniency that motivated the raw-JSON fallback while restoring type
+    safety: malformed input raises `ValueError` (mapped to HTTP 400 by the server) instead of
+    crashing the handler with an opaque `KeyError` / `TypeError`/ `JSONDecodeError` from unvalidated
+    dicts.
+    """
+    try:
+        validated = _FALLBACK_TOOL_CALLS_ADAPTER.validate_python(tool_calls)
+    except ValidationError as e:
+        raise ValueError(_format_tool_call_error(e.errors())) from e
+    # `exclude_unset` preserves the caller's original shape (e.g. omitted `id` / `type` are not
+    # re-introduced), while `extra="allow"` keeps any additional fields the client sent.
+    return [tc.model_dump(exclude_unset=True) for tc in validated]
+
+
+def _normalize_tool_call_arguments(index: int, item: Any) -> dict[str, Any]:
+    """Normalize `function.arguments` to the internal dict form."""
+    item = dict(item)
+    item["function"] = dict(item["function"])
+
+    arguments = item["function"].get("arguments")
+    if arguments is None:
+        item["function"]["arguments"] = {}
+    elif isinstance(arguments, str):
+        try:
+            arguments = json.loads(arguments)
+        except json.JSONDecodeError as e:
+            raise ValueError(
+                f"tool_calls[{index}].function.arguments must be valid JSON."
+            ) from e
+        if not isinstance(arguments, dict):
+            raise ValueError(
+                f"tool_calls[{index}].function.arguments must be a JSON object."
+            )
+        item["function"]["arguments"] = arguments
+    elif isinstance(arguments, dict):
+        item["function"]["arguments"] = arguments
+    else:
+        raise ValueError(
+            f"tool_calls[{index}].function.arguments must be an object.")
+    return item
+
+
+def _parse_fallback_tool_calls(tool_calls: list[Any]) -> list[dict[str, Any]]:
+    """Parse raw tool-call lists accepted only by the tau2-bench fallback path.
+
+    `openai_server.py` first attempts strict OpenAI request validation. Some tau2-bench requests
+    send tool calls in a shape that fails that strict model, so the server falls back to parsing the
+    raw JSON payload and passes a plain list here. Because that list has skipped Pydantic
+    validation, keep the compatibility workaround contained in this helper and re-apply the minimum
+    checks we rely on: each tool call and `function` must be object-shaped, malformed inputs should
+    become client-facing `ValueError`s, and `function.arguments` must normalize to a JSON object.
+    """
+    tool_calls = _validate_fallback_tool_calls(tool_calls)
+    return [
+        _normalize_tool_call_arguments(index, item)
+        for index, item in enumerate(tool_calls)
+    ]
+
+
 # Adapted from: https://github.com/vllm-project/vllm/blob/4574d48bab9c4e38b7c0a830eeefc8f0980e8c58/vllm/entrypoints/chat_utils.py#L1406
 def _parse_assistant_message_content(message: Dict[str, Any]) -> Dict[str, Any]:
     result = {}
@@ -282,25 +386,16 @@ def _parse_assistant_message_content(message: Dict[str, Any]) -> Dict[str, Any]:
 
     tool_calls = message.get("tool_calls")
     if tool_calls is not None:
-        # Materialize Pydantic v2 ValidatorIterator (single-use) to a list.
-        if not isinstance(tool_calls, list):
+        if isinstance(tool_calls, list):
+            result["tool_calls"] = _parse_fallback_tool_calls(tool_calls)
+        else:
+            # The strict parse path delivers tool_calls as a single-use Pydantic `ValidatorIterator`
+            # of already-validated OpenAI tool calls, so only materialize and normalize arguments.
             tool_calls = list(tool_calls)
-
-        result["tool_calls"] = []
-        for item in tool_calls:
-            # Bypass pydantic check to WAR `tau2-bench-telecom` ill-format tool_call.
-            item = dict(item)
-            if "function" in item:
-                item["function"] = dict(item["function"])
-
-            if content := item["function"].get("arguments"):
-                if isinstance(content, str):
-                    item["function"]["arguments"] = json.loads(content)
-                else:
-                    item["function"]["arguments"] = content
-            else:
-                item["function"]["arguments"] = {}
-            result["tool_calls"].append(item)
+            result["tool_calls"] = [
+                _normalize_tool_call_arguments(index, item)
+                for index, item in enumerate(tool_calls)
+            ]
 
     return result
 
diff --git a/tests/unittest/llmapi/apps/test_chat_utils.py b/tests/unittest/llmapi/apps/test_chat_utils.py
@@ -117,6 +117,131 @@ def test_assistant_message_with_empty_tool_arguments(self, mock_mm_data_tracker)
         }
         assert result == expected
 
+    def test_assistant_message_tool_call_missing_function(self, mock_mm_data_tracker) -> None:
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [{"id": "call_123", "type": "function"}],
+        }
+
+        with pytest.raises(ValueError, match=r"tool_calls\[0\]\.function is required"):
+            parse_chat_message_content(message, mock_mm_data_tracker)
+
+    def test_assistant_message_tool_call_invalid_json_arguments(self, mock_mm_data_tracker) -> None:
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [
+                {
+                    "id": "call_123",
+                    "type": "function",
+                    "function": {
+                        "name": "get_weather",
+                        "arguments": "not json {",
+                    },
+                }
+            ],
+        }
+
+        with pytest.raises(
+            ValueError,
+            match=r"tool_calls\[0\]\.function\.arguments must be valid JSON",
+        ):
+            parse_chat_message_content(message, mock_mm_data_tracker)
+
+    @pytest.mark.parametrize(
+        ("arguments", "match"),
+        [
+            ("", r"tool_calls\[0\]\.function\.arguments must be valid JSON"),
+            ("[]", r"tool_calls\[0\]\.function\.arguments must be a JSON object"),
+            ("0", r"tool_calls\[0\]\.function\.arguments must be a JSON object"),
+        ],
+    )
+    def test_assistant_message_tool_call_rejects_non_object_json_arguments(
+        self, mock_mm_data_tracker, arguments, match
+    ) -> None:
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [
+                {
+                    "id": "call_123",
+                    "type": "function",
+                    "function": {
+                        "name": "get_weather",
+                        "arguments": arguments,
+                    },
+                }
+            ],
+        }
+
+        with pytest.raises(ValueError, match=match):
+            parse_chat_message_content(message, mock_mm_data_tracker)
+
+    @pytest.mark.parametrize(
+        "function_value",
+        [None, 123, "hi", ["a", "b"]],
+    )
+    def test_assistant_message_tool_call_non_object_function(
+        self, mock_mm_data_tracker, function_value
+    ) -> None:
+        # A present-but-non-object `function` must raise a clean ValueError (HTTP 400) rather than
+        # an opaque TypeError/ValueError from dict() coercion.
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [{"id": "x", "type": "function", "function": function_value}],
+        }
+
+        with pytest.raises(ValueError, match=r"tool_calls\[0\]\.function"):
+            parse_chat_message_content(message, mock_mm_data_tracker)
+
+    @pytest.mark.parametrize("tool_call", [123, "foo", None])
+    def test_assistant_message_tool_call_non_object_item(
+        self, mock_mm_data_tracker, tool_call
+    ) -> None:
+        """A tool_call that is not an object must raise a clean ValueError."""
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [tool_call],
+        }
+
+        with pytest.raises(ValueError, match=r"tool_calls\[0\]"):
+            parse_chat_message_content(message, mock_mm_data_tracker)
+
+    def test_assistant_message_tool_call_preserves_extra_fields(self, mock_mm_data_tracker) -> None:
+        # Extra fields and omitted optional id/type are preserved through the lenient fallback
+        # validation (the tau2-bench leniency).
+        message = {
+            "role": "assistant",
+            "content": None,
+            "tool_calls": [
+                {
+                    # No `id`/`type`; an arbitrary extra field.
+                    "index": 0,
+                    "function": {
+                        "name": "get_weather",
+                        "arguments": '{"location": "NYC"}',
+                        "extra": "kept",
+                    },
+                }
+            ],
+        }
+
+        result = parse_chat_message_content(message, mock_mm_data_tracker)
+
+        assert result["tool_calls"] == [
+            {
+                "index": 0,
+                "function": {
+                    "name": "get_weather",
+                    "arguments": {"location": "NYC"},
+                    "extra": "kept",
+                },
+            }
+        ]
+
     def test_assistant_message_with_multiple_tool_calls(self, mock_mm_data_tracker):
         """Test parsing an assistant message with multiple tool calls."""
         message = {
