test(e2e): replace vacuous assertions with contract checks

e2e_test/bindings_go/test_go_oai_server.py

@@ -103,17 +103,17 @@ def test_streaming_multiple_messages(self, go_openai_client, go_oai_server):
 @pytest.mark.engine("sglang")
 @pytest.mark.gpu(1)
 @pytest.mark.model("meta-llama/Llama-3.2-1B-Instruct")
-@pytest.mark.xfail(
-    reason="Llama-3.2-1B-Instruct doesn't reliably support tool calling with tool_choice=required",
-    strict=False,  # Allow tests to pass if model happens to work
-)
 class TestGoOAIServerFunctionCalling:
     """Tests for function calling through Go OAI server.
 
-    Note: Function calling requires the model to support tool use.
-    The meta-llama/Llama-3.2-1B-Instruct model may not reliably support tool_choice='required'.
-    These tests are marked as xfail to allow CI to pass while still
-    testing the Go OAI server's tool calling proxy functionality.
+    Note: Function calling requires the model to support tool use, and
+    meta-llama/Llama-3.2-1B-Instruct may not reliably emit a tool call even
+    with tool_choice='required'. Only that model-dependent step is allowed to
+    xfail (via an imperative ``pytest.xfail`` when the model produces no tool
+    call). Structural assertions — response shape, streamed index/argument
+    accumulation, JSON validity of any tool call the server *does* parse, and
+    tool_choice='none' suppression — are unconditional: they verify the Go OAI
+    server's proxy logic, not the model.
     """
 
     TOOLS = [
@@ -162,17 +162,31 @@ def test_function_calling_non_streaming(self, go_openai_client, go_oai_server):
             stream=False,
         )
 
+        # Structural: response shape (unconditional).
         assert response.choices is not None
+        assert len(response.choices) > 0
         tool_calls = response.choices[0].message.tool_calls
 
-        assert tool_calls is not None, "Expected tool calls"
-        assert len(tool_calls) > 0, "Expected at least one tool call"
+        # Model-dependent: whether the 1B model emits a tool call at all.
+        if not tool_calls:
+            pytest.xfail(
+                "Llama-3.2-1B-Instruct did not emit a tool call despite "
+                "tool_choice='required' (known model limitation)"
+            )
 
+        # Structural: any parsed tool call must be well-formed (unconditional).
         tool_call = tool_calls[0]
         assert tool_call.function.name == "get_weather"
 
         args = json.loads(tool_call.function.arguments)
-        assert "location" in args
+        assert isinstance(args, dict)
+
+        # Model-dependent: argument quality.
+        if "location" not in args:
+            pytest.xfail(
+                f"Llama-3.2-1B-Instruct produced schema-incomplete arguments {args} "
+                "(known model limitation)"
+            )
         logger.info(f"Tool call: {tool_call.function.name}({args})")
 
     def test_function_calling_streaming(self, go_openai_client, go_oai_server):
@@ -191,6 +205,7 @@ def test_function_calling_streaming(self, go_openai_client, go_oai_server):
             stream=True,
         )
 
+        # Structural: the stream itself must be well-formed (unconditional).
         chunks = list(response_stream)
         assert len(chunks) > 0
 
@@ -213,18 +228,37 @@ def test_function_calling_streaming(self, go_openai_client, go_oai_server):
                     if tc.function.arguments:
                         tool_calls_by_index[idx]["arguments"] += tc.function.arguments
 
-        assert len(tool_calls_by_index) > 0, "Expected tool calls in stream"
+        # Model-dependent: whether the 1B model emits a tool call at all.
+        if not tool_calls_by_index:
+            pytest.xfail(
+                "Llama-3.2-1B-Instruct streamed no tool call despite "
+                "tool_choice='required' (known model limitation)"
+            )
 
-        # Verify first tool call
+        # Structural: index math and argument accumulation (unconditional).
+        assert 0 in tool_calls_by_index, (
+            f"Streamed tool call indices must start at 0, got {sorted(tool_calls_by_index)}"
+        )
         first_tc = tool_calls_by_index[0]
         assert first_tc["name"] == "get_weather"
 
         args = json.loads(first_tc["arguments"])
-        assert "location" in args
+        assert isinstance(args, dict)
+
+        # Model-dependent: argument quality.
+        if "location" not in args:
+            pytest.xfail(
+                f"Llama-3.2-1B-Instruct produced schema-incomplete arguments {args} "
+                "(known model limitation)"
+            )
         logger.info(f"Streamed tool call: {first_tc['name']}({args})")
 
     def test_function_calling_tool_choice_none(self, go_openai_client, go_oai_server):
-        """Test that tool_choice='none' prevents function calls."""
+        """Test that tool_choice='none' prevents function calls.
+
+        This is a server-side suppression contract, not a model capability:
+        a weak tool-caller cannot make it fail, so nothing here may xfail.
+        """
         _, _, model_path = go_oai_server
 
         response = go_openai_client.chat.completions.create(
@@ -631,14 +665,15 @@ def test_system_message_only(self, go_openai_client, go_oai_server):
 @pytest.mark.model("meta-llama/Llama-3.2-1B-Instruct")
 @pytest.mark.xfail(
     reason="Go OAI server does not currently support n > 1 (multiple choices)",
-    strict=False,
+    strict=True,  # deterministic capability gap: XPASS must fail so the marker is removed when n>1 lands
 )
 class TestGoOAIServerMultipleChoices:
     """Tests for n parameter (multiple choices).
 
     Note: The Go OAI server currently does not support generating multiple
-    choices (n > 1). These tests are marked as xfail to document the expected
-    behavior and will pass when support is added.
+    choices (n > 1) — a deterministic server capability gap, not model
+    flakiness — so these tests are strict xfails: when support is added they
+    will XPASS and fail CI, forcing removal of the marker.
     """
 
     def test_n_parameter_non_streaming(self, go_openai_client, go_oai_server):

e2e_test/chat_completions/test_function_calling.py

@@ -976,52 +976,184 @@ def _is_flaky_test(self, test_name):
         """Check if the current test is marked as flaky for this class."""
         return test_name in self.FLAKY_TESTS
 
+    # Prompt that clearly needs the declared ``get_weather`` tool.
+    AUTO_TOOL_NEEDED_MESSAGES = [{"role": "user", "content": "What's the weather in Tokyo?"}]
+    # Prompt that clearly needs no tool at all.
+    AUTO_NO_TOOL_MESSAGES = [
+        {"role": "user", "content": "What is 2+2? Answer with just the number."}
+    ]
+
+    def _assert_auto_tool_calls_valid(self, tool_calls, tools):
+        """Every produced tool call must name a declared tool with JSON-dict args."""
+        declared_names = {tool["function"]["name"] for tool in tools}
+        for tool_call in tool_calls:
+            assert tool_call.function.name in declared_names, (
+                f"Tool call names undeclared function {tool_call.function.name!r}; "
+                f"declared: {sorted(declared_names)}"
+            )
+            args = json.loads(tool_call.function.arguments)
+            assert isinstance(args, dict), f"Arguments should parse to a dict, got {type(args)}"
+
     def test_tool_choice_auto_non_streaming(self, model, api_client):
-        """Test tool_choice='auto' in non-streaming mode."""
+        """Test tool_choice='auto' semantics in non-streaming mode.
+
+        - A prompt that clearly needs the declared weather tool should yield a
+          tool call for a declared tool, with JSON args mentioning the location.
+        - A prompt that clearly needs no tool should yield non-empty text,
+          no tool calls, and finish_reason == 'stop'.
+
+        For models registered in FLAKY_TESTS (weak tool-callers), the
+        *decision* (call vs. answer) is relaxed but every produced tool call
+        must still be structurally valid.
+        """
 
         tools = get_test_tools()
-        messages = get_test_messages()
+        flaky = self._is_flaky_test("test_tool_choice_auto_non_streaming")
 
+        # --- Scenario 1: prompt that needs the weather tool ---
         response = api_client.chat.completions.create(
             model=model,
-            messages=messages,
+            messages=self.AUTO_TOOL_NEEDED_MESSAGES,
             max_tokens=2048,
+            temperature=0.2,
             tools=tools,
             tool_choice="auto",
             stream=False,
         )
 
-        assert response.choices[0].message is not None
-        # With auto, tool calls are optional
-
-    def test_tool_choice_auto_streaming(self, model, api_client):
-        """Test tool_choice='auto' in streaming mode."""
-
-        tools = get_test_tools()
-        messages = get_test_messages()
+        choice = response.choices[0]
+        tool_calls = choice.message.tool_calls
+        if tool_calls:
+            self._assert_auto_tool_calls_valid(tool_calls, tools)
+            assert any("tokyo" in (tc.function.arguments or "").lower() for tc in tool_calls), (
+                f"Expected the location (Tokyo) in tool call args, got: {tool_calls}"
+            )
+            assert choice.finish_reason == "tool_calls", (
+                f"Expected finish_reason 'tool_calls', got {choice.finish_reason!r}"
+            )
+        else:
+            assert flaky, (
+                "tool_choice='auto' produced no tool call for a prompt that "
+                "clearly needs the declared weather tool"
+            )
+            # Weak model answered directly; it must at least answer with text.
+            assert choice.message.content and choice.message.content.strip(), (
+                "Expected non-empty text content when no tool call was made"
+            )
 
+        # --- Scenario 2: prompt that needs no tool ---
         response = api_client.chat.completions.create(
             model=model,
-            messages=messages,
-            max_tokens=2048,
+            messages=self.AUTO_NO_TOOL_MESSAGES,
+            max_tokens=256,
+            temperature=0.2,
             tools=tools,
             tool_choice="auto",
-            stream=True,
+            stream=False,
         )
 
-        # Collect streaming response
-        content_chunks = []
-        tool_call_chunks = []
+        choice = response.choices[0]
+        if flaky and choice.message.tool_calls:
+            # Weak model spuriously called a tool; it must still be declared/valid.
+            self._assert_auto_tool_calls_valid(choice.message.tool_calls, tools)
+        else:
+            assert not choice.message.tool_calls, (
+                f"Expected no tool calls for a trivial arithmetic prompt, "
+                f"got: {choice.message.tool_calls}"
+            )
+            assert choice.message.content and choice.message.content.strip(), (
+                "Expected non-empty text content for a trivial arithmetic prompt"
+            )
+            assert choice.finish_reason == "stop", (
+                f"Expected finish_reason 'stop', got {choice.finish_reason!r}"
+            )
 
-        for chunk in response:
-            if chunk.choices[0].delta.content:
-                content_chunks.append(chunk.choices[0].delta.content)
-            elif chunk.choices[0].delta.tool_calls:
-                tool_call_chunks.extend(chunk.choices[0].delta.tool_calls)
+    def test_tool_choice_auto_streaming(self, model, api_client):
+        """Test tool_choice='auto' semantics in streaming mode.
 
-        # Should complete without errors
-        assert isinstance(content_chunks, list)
-        assert isinstance(tool_call_chunks, list)
+        Same contract as the non-streaming variant, verified over accumulated
+        stream deltas (chunk collection mirrors
+        test_required_streaming_arguments_chunks_json).
+        """
+
+        tools = get_test_tools()
+        flaky = self._is_flaky_test("test_tool_choice_auto_streaming")
+
+        def collect_stream(messages, max_tokens):
+            response = api_client.chat.completions.create(
+                model=model,
+                messages=messages,
+                max_tokens=max_tokens,
+                temperature=0.2,
+                tools=tools,
+                tool_choice="auto",
+                stream=True,
+            )
+            content_parts = []
+            tool_calls_by_index = {}
+            finish_reason = None
+            for chunk in response:
+                choice = chunk.choices[0]
+                if choice.finish_reason:
+                    finish_reason = choice.finish_reason
+                if choice.delta.content:
+                    content_parts.append(choice.delta.content)
+                if choice.delta.tool_calls:
+                    for tc_delta in choice.delta.tool_calls:
+                        tool_call = tool_calls_by_index.setdefault(
+                            tc_delta.index, {"name": "", "arguments": ""}
+                        )
+                        if tc_delta.function:
+                            if tc_delta.function.name:
+                                tool_call["name"] = tc_delta.function.name
+                            if tc_delta.function.arguments:
+                                tool_call["arguments"] += tc_delta.function.arguments
+            return "".join(content_parts), list(tool_calls_by_index.values()), finish_reason
+
+        declared_names = {tool["function"]["name"] for tool in tools}
+
+        def assert_streamed_calls_valid(tool_calls):
+            for tool_call in tool_calls:
+                assert tool_call["name"] in declared_names, (
+                    f"Tool call names undeclared function {tool_call['name']!r}"
+                )
+                args = json.loads(tool_call["arguments"])
+                assert isinstance(args, dict)
+
+        # --- Scenario 1: prompt that needs the weather tool ---
+        content, tool_calls, finish_reason = collect_stream(
+            self.AUTO_TOOL_NEEDED_MESSAGES, max_tokens=2048
+        )
+        if tool_calls:
+            assert_streamed_calls_valid(tool_calls)
+            assert any("tokyo" in tc["arguments"].lower() for tc in tool_calls), (
+                f"Expected the location (Tokyo) in streamed tool call args, got: {tool_calls}"
+            )
+            assert finish_reason == "tool_calls", (
+                f"Expected finish_reason 'tool_calls', got {finish_reason!r}"
+            )
+        else:
+            assert flaky, (
+                "tool_choice='auto' streamed no tool call for a prompt that "
+                "clearly needs the declared weather tool"
+            )
+            assert content.strip(), "Expected non-empty streamed text when no tool call was made"
+
+        # --- Scenario 2: prompt that needs no tool ---
+        content, tool_calls, finish_reason = collect_stream(
+            self.AUTO_NO_TOOL_MESSAGES, max_tokens=256
+        )
+        if flaky and tool_calls:
+            assert_streamed_calls_valid(tool_calls)
+        else:
+            assert not tool_calls, (
+                f"Expected no streamed tool calls for a trivial arithmetic prompt, "
+                f"got: {tool_calls}"
+            )
+            assert content.strip(), (
+                "Expected non-empty streamed text for a trivial arithmetic prompt"
+            )
+            assert finish_reason == "stop", f"Expected finish_reason 'stop', got {finish_reason!r}"
 
     def test_tool_choice_required_non_streaming(self, model, api_client):
         """Test tool_choice='required' in non-streaming mode."""
@@ -1508,10 +1640,13 @@ def test_conflicting_defs_required_tool_choice(self, model, api_client):
 class TestToolChoiceLlama(_TestToolChoiceBase):
     """Tests for tool_choice functionality with Llama model."""
 
-    # Mark flaky tests for this model
+    # Mark flaky tests for this model — Llama-3.2-1B does not reliably
+    # decide correctly with tool_choice='auto'.
     FLAKY_TESTS = {
         "test_multi_tool_scenario_auto",
         "test_multi_tool_scenario_required",
+        "test_tool_choice_auto_non_streaming",
+        "test_tool_choice_auto_streaming",
     }
 
 
@@ -1547,10 +1682,13 @@ class TestToolChoiceQwen(_TestToolChoiceBase):
 class TestToolChoiceMistral(_TestToolChoiceBase):
     """Tests for tool_choice functionality with Mistral model."""
 
-    # Mark flaky tests for this model
+    # Mark flaky tests for this model — Mistral-7B-v0.3 is not a reliable
+    # tool_choice='auto' decision-maker.
     FLAKY_TESTS = {
         "test_multi_tool_scenario_auto",
         "test_multi_tool_scenario_required",
+        "test_tool_choice_auto_non_streaming",
+        "test_tool_choice_auto_streaming",
     }
 
     def test_complex_parameters_required_non_streaming(self, model, api_client):