[Bugfix] Chat Completions Harmony Refactor Clean up (#45464)

Signed-off-by: Yifan Zong <[email protected]>
Co-authored-by: Ben Browning <[email protected]>

tests/parser/test_harmony.py

@@ -118,12 +118,17 @@ def tool_call_payloads(delta_message) -> list:
     ]
 
 
-def combined_tool_arguments(delta_message) -> dict[int, str]:
-    combined: dict[int, str] = {}
-    for tool_call in tool_call_payloads(delta_message):
-        combined.setdefault(tool_call.index, "")
-        combined[tool_call.index] += tool_call.function.arguments
-    return combined
+def tool_call_entries(delta_message) -> list[tuple[int, str | None, str | None]]:
+    if delta_message is None or not delta_message.tool_calls:
+        return []
+    return [
+        (
+            tool_call.index,
+            tool_call.function.name if tool_call.function else None,
+            tool_call.function.arguments if tool_call.function else None,
+        )
+        for tool_call in delta_message.tool_calls
+    ]
 
 
 class TestParse:
@@ -481,18 +486,14 @@ class TestParseDelta:
         assert first_delta is not None
         assert first_delta.reasoning == "Thinking"
         assert first_delta.content is None
-        assert [tool.function.name for tool in tool_call_headers(first_delta)] == [
-            "get_weather"
+        assert tool_call_entries(first_delta) == [
+            (0, "get_weather", '{"location": '),
         ]
-        assert combined_tool_arguments(first_delta) == {0: '{"location": '}
-        assert {tool.index for tool in first_delta.tool_calls} == {0}
 
         assert second_delta is not None
         assert second_delta.reasoning is None
         assert second_delta.content is None
-        assert not tool_call_headers(second_delta)
-        assert combined_tool_arguments(second_delta) == {0: '"Paris"}'}
-        assert {tool.index for tool in second_delta.tool_calls} == {0}
+        assert tool_call_entries(second_delta) == [(0, None, '"Paris"}')]
 
     def test_commentary_preamble_streaming(self, gpt_oss_tokenizer, chat_request):
         parser = HarmonyParser(gpt_oss_tokenizer)
@@ -601,8 +602,7 @@ class TestParseDelta:
         assert delta is not None
         assert delta.reasoning == "Reasoning about query..."
         assert delta.content == "Done"
-        assert [tool.function.name for tool in tool_call_headers(delta)] == ["search"]
-        assert combined_tool_arguments(delta) == {0: '{"query": "vllm"}'}
+        assert tool_call_entries(delta) == [(0, "search", '{"query": "vllm"}')]
 
     def test_tool_index_across_calls(self, gpt_oss_tokenizer, chat_request):
         parser = HarmonyParser(gpt_oss_tokenizer)
@@ -665,22 +665,22 @@ class TestParseDelta:
             finished=False,
         )
 
+        assert tool_call_entries(first_delta) == [
+            (0, "tool_a", '{"a": 1}'),
+            (1, "tool_b", '{"b": '),
+        ]
         assert [tool.index for tool in tool_call_headers(first_delta)] == [0, 1]
-        assert combined_tool_arguments(first_delta) == {
-            0: '{"a": 1}',
-            1: '{"b": ',
-        }
 
         assert second_delta is not None
+        assert tool_call_entries(second_delta) == [(1, None, "2")]
         assert [tool.index for tool in tool_call_payloads(second_delta)] == [1]
-        assert combined_tool_arguments(second_delta) == {1: "2"}
 
         assert third_delta is not None
         assert third_delta.content == "Done"
-        assert combined_tool_arguments(third_delta) == {
-            1: "}",
-            2: '{"c": 3}',
-        }
+        assert tool_call_entries(third_delta) == [
+            (1, None, "}"),
+            (2, "tool_c", '{"c": 3}'),
+        ]
         assert [tool.index for tool in tool_call_headers(third_delta)] == [2]
 
 

vllm/entrypoints/serve/render/serving.py

@@ -189,17 +189,18 @@ class OpenAIServingRender:
         self.trust_request_chat_template = trust_request_chat_template
         self.enable_auto_tools = enable_auto_tools
         self.exclude_tools_when_tool_choice_none = exclude_tools_when_tool_choice_none
+        self.use_harmony = model_config.hf_config.model_type == "gpt_oss"
         self.parser: type[Parser] | None = ParserManager.get_parser(
             tool_parser_name=tool_parser,
             reasoning_parser_name=reasoning_parser,
             enable_auto_tools=enable_auto_tools,
             model_name=model_config.model,
+            is_harmony=self.use_harmony,
         )
         self.default_chat_template_kwargs: dict[str, Any] = (
             default_chat_template_kwargs or {}
         )
         self.log_error_stack = log_error_stack
-        self.use_harmony = model_config.hf_config.model_type == "gpt_oss"
         self.supports_browsing = False
         self.supports_code_interpreter = False
 

vllm/parser/harmony.py

@@ -68,18 +68,18 @@ class HarmonyParser(DelegatingParser):
     def __init__(self, tokenizer, tools=None, *args, **kwargs):
         super().__init__(tokenizer, tools, *args, **kwargs)
 
-        if self._reasoning_parser and not isinstance(
-            self._reasoning_parser, GptOssReasoningParser
+        if self.reasoning_parser and not isinstance(
+            self.reasoning_parser, GptOssReasoningParser
         ):
             raise ValueError(
                 "Harmony requires GptOssReasoningParser, "
-                f"got {self._reasoning_parser.__class__.__name__}."
+                f"got {self.reasoning_parser.__class__.__name__}."
             )
 
-        if self._tool_parser and not isinstance(self._tool_parser, GptOssToolParser):
+        if self.tool_parser and not isinstance(self.tool_parser, GptOssToolParser):
             raise ValueError(
                 "Harmony requires GptOssToolParser, "
-                f"got {self._tool_parser.__class__.__name__}."
+                f"got {self.tool_parser.__class__.__name__}."
             )
 
         self._harmony_parser = get_streamable_parser_for_assistant()
@@ -209,11 +209,11 @@ class HarmonyParser(DelegatingParser):
                 segment.channel, segment.recipient
             )
             match segment_type:
-                case _SegmentType.REASONING:
+                case _SegmentType.REASONING if self.reasoning_parser:
                     combined_reasoning += segment.delta
                 case _SegmentType.CONTENT:
                     combined_content += segment.delta
-                case _SegmentType.TOOL:
+                case _SegmentType.TOOL if self.tool_parser:
                     assert segment.recipient is not None
                     if prev_recipient != segment.recipient:
                         tool_name = extract_function_from_recipient(segment.recipient)
@@ -233,13 +233,20 @@ class HarmonyParser(DelegatingParser):
                         self._next_tool_call_index += 1
                         prev_recipient = segment.recipient
                     elif segment.delta:
-                        tool_call_index = self._next_tool_call_index - 1
-                        tool_messages.append(
-                            DeltaToolCall(
-                                index=tool_call_index,
-                                function=DeltaFunctionCall(arguments=segment.delta),
+                        idx = self._next_tool_call_index - 1
+                        if tool_messages:
+                            tool_msg = tool_messages[-1]
+                            assert tool_msg.index == idx
+                            fn = tool_msg.function
+                            assert fn is not None and fn.arguments is not None
+                            fn.arguments += segment.delta
+                        else:
+                            tool_messages.append(
+                                DeltaToolCall(
+                                    index=idx,
+                                    function=DeltaFunctionCall(arguments=segment.delta),
+                                )
                             )
-                        )
 
         if not combined_content and not combined_reasoning and not tool_messages:
             return None