Fix tool call constrained decoding and parsing for models with native formats (#21593)

2026-04-10 20:37:23 -07:00
parent c2821dfbe9
commit 7c6db40540
9 changed files with 306 additions and 61 deletions
@@ -822,5 +822,77 @@ class ServingChatTestCase(unittest.TestCase):
        self.assertIn("must be an integer", context.exception.detail)


+class TestProcessToolCallsWithRequiredToolChoice(unittest.TestCase):
+    """Test _process_tool_calls with tool_choice='required' uses model-specific parser."""
+
+    def setUp(self):
+        tm = _MockTokenizerManager()
+        tm.server_args.tool_call_parser = "kimi_k2"
+        self.chat = OpenAIServingChat(tm, _MockTemplateManager())
+
+    def test_required_with_parser_uses_function_call_parser(self):
+        """tool_choice='required' should use FunctionCallParser when tool_call_parser is set."""
+        with patch(
+            "sglang.srt.entrypoints.openai.serving_chat.FunctionCallParser"
+        ) as ParserMock:
+            call_info = Mock()
+            call_info.name = "get_weather"
+            call_info.parameters = '{"location":"Tokyo"}'
+            call_info.tool_index = 0
+
+            parser_instance = ParserMock.return_value
+            parser_instance.has_tool_call.return_value = True
+            parser_instance.parse_non_stream.return_value = ("", [call_info])
+
+            finish_reason = {"type": "stop", "matched": None}
+            tools = [{"type": "function", "function": {"name": "get_weather"}}]
+
+            tool_calls, text, fr = self.chat._process_tool_calls(
+                text="<|tool_calls_section_begin|>...<|tool_calls_section_end|>",
+                tools=tools,
+                finish_reason=finish_reason,
+                tool_choice="required",
+            )
+
+            self.assertIsNotNone(tool_calls)
+            self.assertEqual(len(tool_calls), 1)
+            self.assertEqual(tool_calls[0].function.name, "get_weather")
+            self.assertEqual(fr["type"], "tool_calls")
+
+    def test_required_without_parser_falls_back_to_json(self):
+        """tool_choice='required' without parser should parse as JSON array."""
+        self.chat.tool_call_parser = None
+
+        finish_reason = {"type": "stop", "matched": None}
+        tools = [{"type": "function", "function": {"name": "get_weather"}}]
+
+        tool_calls, text, fr = self.chat._process_tool_calls(
+            text='[{"name":"get_weather","parameters":{"location":"Tokyo"}}]',
+            tools=tools,
+            finish_reason=finish_reason,
+            tool_choice="required",
+        )
+
+        self.assertIsNotNone(tool_calls)
+        self.assertEqual(len(tool_calls), 1)
+        self.assertEqual(tool_calls[0].function.name, "get_weather")
+
+    def test_required_without_parser_invalid_json_returns_none(self):
+        """tool_choice='required' without parser and invalid JSON returns tool_calls=None."""
+        self.chat.tool_call_parser = None
+
+        finish_reason = {"type": "stop", "matched": None}
+        tools = [{"type": "function", "function": {"name": "get_weather"}}]
+
+        tool_calls, text, fr = self.chat._process_tool_calls(
+            text="<|tool_calls_section_begin|>not json",
+            tools=tools,
+            finish_reason=finish_reason,
+            tool_choice="required",
+        )
+
+        self.assertIsNone(tool_calls)
+
+
 if __name__ == "__main__":
    unittest.main(verbosity=2)
@@ -348,8 +348,12 @@ class TestToolChoiceLlama32(CustomTestCase):
        self.assertEqual(found_name, "get_weather")

    def test_required_streaming_arguments_chunks_json(self):
-        """In streaming required mode, complete tool call arguments should be valid JSON when all chunks are combined"""
+        """In streaming required mode, complete tool call arguments should be valid JSON when all chunks are combined.
+        Uses strict=True so the grammar enforces the parameter schema."""
        tools = self.get_test_tools()
+        # Add strict=True so arguments are schema-constrained
+        for tool in tools:
+            tool["function"]["strict"] = True
        messages = self.get_test_messages()

        response = self.client.chat.completions.create(
@@ -406,13 +410,15 @@ class TestToolChoiceLlama32(CustomTestCase):
                )

    def test_complex_parameters_required_non_streaming(self):
-        """Validate complex nested parameter schemas in non-streaming required mode"""
+        """Validate complex nested parameter schemas in non-streaming required mode.
+        Uses strict=True so the grammar enforces the parameter schema."""
        complex_tools = [
            {
                "type": "function",
                "function": {
                    "name": "analyze_data",
                    "description": "Analyze complex data structures",
+                    "strict": True,
                    "parameters": {
                        "type": "object",
                        "properties": {
@@ -3925,6 +3925,119 @@ function call<|role_sep|>
        self.assertEqual(params["city"], "Rome")


+class TestGetStructureConstraint(unittest.TestCase):
+    """Tests for FunctionCallParser.get_structure_constraint() logic.
+
+    Verifies that detectors supporting structural_tag use it for required/named
+    tool_choice, and that the generic json_schema fallback is used otherwise.
+    """
+
+    def _make_tools(self, strict=False):
+        return [
+            Tool(
+                type="function",
+                function=Function(
+                    name="get_weather",
+                    description="Get weather",
+                    parameters={
+                        "type": "object",
+                        "properties": {"city": {"type": "string"}},
+                        "required": ["city"],
+                    },
+                    strict=strict,
+                ),
+            ),
+        ]
+
+    def _make_parser(self, parser_name, strict=False):
+        from sglang.srt.function_call.function_call_parser import FunctionCallParser
+
+        return FunctionCallParser(self._make_tools(strict=strict), parser_name)
+
+    # --- structural_tag detectors (kimi_k2, deepseekv3, qwen25, etc.) ---
+
+    def test_kimi_required_strict_returns_structural_tag(self):
+        parser = self._make_parser("kimi_k2", strict=True)
+        result = parser.get_structure_constraint("required")
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+        self.assertTrue(result[1].at_least_one)
+
+    def test_kimi_required_no_strict_returns_structural_tag(self):
+        """required should use structural_tag even without strict, to preserve native format."""
+        parser = self._make_parser("kimi_k2", strict=False)
+        result = parser.get_structure_constraint("required")
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+        self.assertTrue(result[1].at_least_one)
+
+    def test_kimi_auto_strict_returns_structural_tag(self):
+        parser = self._make_parser("kimi_k2", strict=True)
+        result = parser.get_structure_constraint("auto")
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+        self.assertFalse(result[1].at_least_one)
+
+    def test_kimi_auto_no_strict_returns_none(self):
+        """auto without strict should not constrain."""
+        parser = self._make_parser("kimi_k2", strict=False)
+        result = parser.get_structure_constraint("auto")
+        self.assertIsNone(result)
+
+    def test_kimi_named_tool_choice_returns_structural_tag(self):
+        from sglang.srt.entrypoints.openai.protocol import (
+            ToolChoice,
+            ToolChoiceFuncName,
+        )
+
+        parser = self._make_parser("kimi_k2", strict=False)
+        tool_choice = ToolChoice(function=ToolChoiceFuncName(name="get_weather"))
+        result = parser.get_structure_constraint(tool_choice)
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+
+    def test_deepseekv3_required_no_strict_returns_structural_tag(self):
+        parser = self._make_parser("deepseekv3", strict=False)
+        result = parser.get_structure_constraint("required")
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+
+    def test_qwen25_required_no_strict_returns_structural_tag(self):
+        parser = self._make_parser("qwen25", strict=False)
+        result = parser.get_structure_constraint("required")
+        self.assertIsNotNone(result)
+        self.assertEqual(result[0], "structural_tag")
+
+    # --- structural_tag content verification ---
+
+    def test_kimi_structural_tag_has_kimi_tokens(self):
+        """Verify structural_tag contains kimi-specific special tokens."""
+        parser = self._make_parser("kimi_k2", strict=True)
+        result = parser.get_structure_constraint("required")
+        tag = result[1]
+        structures = tag.structures
+        self.assertTrue(len(structures) > 0)
+        self.assertIn("<|tool_calls_section_begin|>", structures[0].begin)
+        self.assertIn("<|tool_call_end|>", structures[0].end)
+
+    def test_kimi_required_no_strict_uses_empty_schema(self):
+        """Without strict, structural_tag should use empty schema per OpenAI
+        protocol: strict=False means no parameter schema enforcement."""
+        parser = self._make_parser("kimi_k2", strict=False)
+        result = parser.get_structure_constraint("required")
+        tag = result[1]
+        self.assertEqual(tag.structures[0].schema_, {})
+
+    def test_kimi_required_strict_uses_tool_schema(self):
+        """With strict, structural_tag should include the tool's parameter schema."""
+        parser = self._make_parser("kimi_k2", strict=True)
+        result = parser.get_structure_constraint("required")
+        tag = result[1]
+        schema = tag.structures[0].schema_
+        self.assertIn("properties", schema)
+        self.assertIn("city", schema["properties"])
+
+
 class TestQwen25Detector(unittest.TestCase):
    """Test Qwen25Detector streaming and non-streaming multi-tool-call parsing."""