diff --git a/tests/test_tool_env.py b/tests/test_tool_env.py new file mode 100644 index 0000000000..6aa60bc408 --- /dev/null +++ b/tests/test_tool_env.py @@ -0,0 +1,414 @@ +"""Tests for the ToolEnv class with multiple tool call support.""" + +import pytest +from unittest.mock import MagicMock +from verifiers import XMLParser +from verifiers.envs.tool_env import ToolEnv + + +class TestToolEnv: + """Test cases for the ToolEnv class.""" + + @pytest.fixture + def mock_tools(self): + """Create mock tools for testing.""" + def add_tool(a: int, b: int) -> int: + """Add two numbers together. + + Args: + a: First number + b: Second number + + Returns: + int: Sum of a and b + """ + return a + b + + def multiply_tool(x: int, y: int) -> int: + """Multiply two numbers. + + Args: + x: First number + y: Second number + + Returns: + int: Product of x and y + """ + return x * y + + def greet_tool(name: str = "World") -> str: + """Greet someone. + + Args: + name: Name to greet + + Returns: + str: Greeting message + """ + return f"Hello, {name}!" + + return [add_tool, multiply_tool, greet_tool] + + @pytest.fixture + def tool_env(self, mock_tools, sample_dataset): + """Create a ToolEnv instance with mock tools.""" + parser = XMLParser(fields=["think", ("tool", "answer")]) + return ToolEnv( + tools=mock_tools, + parser=parser, + system_prompt="Test system prompt with {tool_descriptions}", + max_turns=5, + dataset=sample_dataset + ) + + def test_single_tool_call(self, tool_env): + """Test calling a single tool.""" + messages = [ + {"role": "user", "content": "Calculate 2 + 3"}, + {"role": "assistant", "content": 'I need to add 2 and 3{"name": "add_tool", "args": {"a": 2, "b": 3}}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + assert "5" in response["content"] + + def test_multiple_tool_calls(self, tool_env): + """Test calling multiple tools in one message.""" + messages = [ + {"role": "user", "content": "Calculate 2 + 3 and then multiply 4 * 5"}, + {"role": "assistant", "content": '''I need to do two calculations +{"name": "add_tool", "args": {"a": 2, "b": 3}} +{"name": "multiply_tool", "args": {"x": 4, "y": 5}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "multiply_tool result:" in content + assert "5" in content # Result of 2 + 3 + assert "20" in content # Result of 4 * 5 + + def test_three_tool_calls(self, tool_env): + """Test calling three tools in one message.""" + messages = [ + {"role": "user", "content": "Do multiple operations"}, + {"role": "assistant", "content": '''Multiple operations +{"name": "add_tool", "args": {"a": 1, "b": 2}} +{"name": "multiply_tool", "args": {"x": 3, "y": 4}} +{"name": "greet_tool", "args": {"name": "Alice"}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "multiply_tool result:" in content + assert "greet_tool result:" in content + assert "3" in content # Result of 1 + 2 + assert "12" in content # Result of 3 * 4 + assert "Hello, Alice!" in content # Result of greet_tool + + def test_mixed_valid_invalid_tools(self, tool_env): + """Test mix of valid and invalid tool calls.""" + messages = [ + {"role": "user", "content": "Mix of valid and invalid"}, + {"role": "assistant", "content": '''Testing mixed calls +{"name": "add_tool", "args": {"a": 5, "b": 10}} +{"name": "invalid_tool", "args": {"x": 1}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "invalid_tool result:" in content # Tool name is extracted from JSON even if tool doesn't exist + assert "15" in content # Valid result from add_tool + assert "Error:" in content # Error from invalid_tool + + def test_invalid_json_in_multiple_tools(self, tool_env): + """Test handling of invalid JSON in multiple tool calls.""" + messages = [ + {"role": "user", "content": "Invalid JSON test"}, + {"role": "assistant", "content": '''Testing invalid JSON +{"name": "add_tool", "args": {"a": 1, "b": 2}} +{invalid json}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "unknown_tool result:" in content # Invalid JSON gets "unknown_tool" label + assert "3" in content # Valid result + assert "Error:" in content # Error from invalid JSON + + def test_backward_compatibility_single_tool(self, tool_env): + """Test that single tool calls still work (backward compatibility).""" + messages = [ + {"role": "user", "content": "Single tool test"}, + {"role": "assistant", "content": '{"name": "greet_tool", "args": {}}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + # Should not have tool name prefix for single tool + assert "greet_tool result:" not in response["content"] + assert "Hello, World!" in response["content"] + + def test_no_tools_fallback(self, tool_env): + """Test fallback when no tools are detected.""" + messages = [ + {"role": "user", "content": "No tools"}, + {"role": "assistant", "content": 'Just thinking, no tools'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + assert "Error:" in response["content"] + assert "Tool command not found" in response["content"] + + def test_empty_tool_results(self, tool_env): + """Test handling of tools that return empty results.""" + # Mock a tool that returns empty string + def empty_tool() -> str: + """Tool that returns empty string.""" + return "" + + tool_env.tools["empty_tool"] = empty_tool + tool_env.tool_schemas.append({ + "name": "empty_tool", + "description": "Returns empty string", + "args": {}, + "returns": "Empty string", + "examples": [] + }) + + messages = [ + {"role": "user", "content": "Empty tool test"}, + {"role": "assistant", "content": '''{"name": "add_tool", "args": {"a": 1, "b": 1}} +{"name": "empty_tool", "args": {}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "empty_tool result:" in content + assert "2" in content # Result from add_tool + + def test_tool_with_default_args(self, tool_env): + """Test calling tools with default arguments in multiple calls.""" + messages = [ + {"role": "user", "content": "Test default args"}, + {"role": "assistant", "content": '''Testing default arguments +{"name": "greet_tool", "args": {}} +{"name": "greet_tool", "args": {"name": "Bob"}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "greet_tool result:" in content + # Both calls use same tool name, so we'll see it twice + assert "Hello, World!" in content # Default name + assert "Hello, Bob!" in content # Specified name + + def test_tool_exception_handling(self, tool_env): + """Test handling of tool exceptions in multiple calls.""" + def error_tool() -> str: + """Tool that raises an exception.""" + raise ValueError("Tool error occurred") + + tool_env.tools["error_tool"] = error_tool + + messages = [ + {"role": "user", "content": "Test error handling"}, + {"role": "assistant", "content": '''Testing error handling +{"name": "add_tool", "args": {"a": 1, "b": 2}} +{"name": "error_tool", "args": {}} +{"name": "greet_tool", "args": {}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "add_tool result:" in content + assert "error_tool result:" in content + assert "greet_tool result:" in content + assert "3" in content # Successful result + assert "Error:" in content # Error from error_tool + assert "Hello, World!" in content # Successful result after error + + def test_empty_results_list_edge_case(self, tool_env): + """Test the edge case where results list is somehow empty.""" + # This is a bit contrived, but tests the specific condition on line 179 + original_call_tool = tool_env.call_tool + + def mock_call_tool(tool_json): + # Return None/empty to simulate empty results + return None + + tool_env.call_tool = mock_call_tool + + messages = [ + {"role": "user", "content": "Empty results test"}, + {"role": "assistant", "content": '{"name": "add_tool", "args": {"a": 1, "b": 2}}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + # Should handle the None result gracefully + assert response["role"] == "user" + + # Restore original method + tool_env.call_tool = original_call_tool + + def test_parse_all_exception_handling(self, tool_env): + """Test exception handling in parse_all.""" + # Mock parse_all to raise an exception + original_parse_all = tool_env.parser.parse_all + + def mock_parse_all(content): + raise Exception("Parse error") + + tool_env.parser.parse_all = mock_parse_all + + messages = [ + {"role": "user", "content": "Parse error test"}, + {"role": "assistant", "content": '{"name": "add_tool", "args": {"a": 1, "b": 2}}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + # Should fall back to error message + assert response["role"] == "user" + assert "Error:" in response["content"] + + # Restore original method + tool_env.parser.parse_all = original_parse_all + + def test_missing_tool_attribute(self, tool_env): + """Test when parsed_all doesn't have tool attribute.""" + # Mock parse_all to return object without tool attribute + original_parse_all = tool_env.parser.parse_all + + def mock_parse_all(content): + from types import SimpleNamespace + return SimpleNamespace(other_field=[]) + + tool_env.parser.parse_all = mock_parse_all + + messages = [ + {"role": "user", "content": "Missing tool attribute test"}, + {"role": "assistant", "content": '{"name": "add_tool", "args": {"a": 1, "b": 2}}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + # Should fall back to single parse + assert response["role"] == "user" + + # Restore original method + tool_env.parser.parse_all = original_parse_all + + def test_tool_results_with_special_characters(self, tool_env): + """Test tool results containing newlines and special characters.""" + def special_output_tool() -> str: + """Tool that returns output with special characters.""" + return "Line 1\nLine 2\n\nTab:\tHere\nSpecial: !@#$%^&*()" + + tool_env.tools["special_output_tool"] = special_output_tool + + messages = [ + {"role": "user", "content": "Special characters test"}, + {"role": "assistant", "content": '''Testing special characters +{"name": "special_output_tool", "args": {}} +{"name": "greet_tool", "args": {}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "special_output_tool result:" in content + assert "greet_tool result:" in content + assert "Line 1\nLine 2" in content + assert "Tab:\tHere" in content + assert "Special: !@#$%^&*()" in content + assert "Hello, World!" in content + + def test_many_tool_calls(self, tool_env): + """Test performance with many tool calls (10+).""" + # Create message with 15 tool calls + tools_content = "\n".join([ + f'{{"name": "add_tool", "args": {{"a": {i}, "b": {i+1}}}}}' + for i in range(15) + ]) + + messages = [ + {"role": "user", "content": "Many tools test"}, + {"role": "assistant", "content": f'Testing many tools\n{tools_content}'} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + + # Should have all 15 results with add_tool labels + assert content.count("add_tool result:") == 15 + + # Check some specific calculations + assert str(0 + 1) in content # First result: 0+1=1 + assert str(14 + 15) in content # Last result: 14+15=29 + + def test_max_chars_with_multiple_tools(self, tool_env): + """Test max_chars truncation with multiple tool calls.""" + def long_output_tool() -> str: + """Tool that returns very long output.""" + return "A" * 2000 # Very long string + + tool_env.tools["long_output_tool"] = long_output_tool + + messages = [ + {"role": "user", "content": "Long output test"}, + {"role": "assistant", "content": '''Testing long output +{"name": "long_output_tool", "args": {}} +{"name": "greet_tool", "args": {}}'''} + ] + state = {} + + response, new_state = tool_env.env_response(messages, state) + + assert response["role"] == "user" + content = response["content"] + assert "long_output_tool result:" in content + assert "greet_tool result:" in content + # Long output should be truncated if max_chars is set + assert "Hello, World!" in content # Second tool should still work diff --git a/tests/test_xml_parser.py b/tests/test_xml_parser.py index 4903278085..0369551e6f 100644 --- a/tests/test_xml_parser.py +++ b/tests/test_xml_parser.py @@ -162,4 +162,108 @@ def test_format_reward_function(self, xml_parser): {"role": "assistant", "content": "Just plain text without XML"} ] bad_reward = reward_func(bad_completion) - assert bad_reward == 0.2 # Gets 0.2 for proper spacing (no XML tags to mess up) \ No newline at end of file + assert bad_reward == 0.2 # Gets 0.2 for proper spacing (no XML tags to mess up) + + def test_parse_all_single_occurrence(self, xml_parser): + """Test parse_all with single occurrence of each field.""" + xml_text = """ + Single reasoning + Single answer + """ + result = xml_parser.parse_all(xml_text) + assert result.reasoning == ["Single reasoning"] + assert result.answer == ["Single answer"] + + def test_parse_all_multiple_occurrences(self, xml_parser): + """Test parse_all with multiple occurrences of the same field.""" + xml_text = """ + First reasoning + First answer + Second reasoning + Second answer + """ + result = xml_parser.parse_all(xml_text) + assert result.reasoning == ["First reasoning", "Second reasoning"] + assert result.answer == ["First answer", "Second answer"] + + def test_parse_all_no_occurrences(self, xml_parser): + """Test parse_all with no occurrences of fields.""" + xml_text = "Just plain text with no XML tags" + result = xml_parser.parse_all(xml_text) + assert result.reasoning == [] + assert result.answer == [] + + def test_parse_all_mixed_occurrences(self, xml_parser): + """Test parse_all with mixed occurrences (some fields present, others not).""" + xml_text = """ + Only reasoning here + More reasoning + """ + result = xml_parser.parse_all(xml_text) + assert result.reasoning == ["Only reasoning here", "More reasoning"] + assert result.answer == [] + + def test_parse_all_with_alternatives(self, xml_parser_with_alternatives): + """Test parse_all with alternative field names.""" + xml_text = """ + First reasoning + First code + Second reasoning + Alternative answer + """ + result = xml_parser_with_alternatives.parse_all(xml_text) + assert result.reasoning == ["First reasoning", "Second reasoning"] + assert result.code == ["First code"] + assert result.answer == ["Alternative answer"] + + def test_parse_all_no_strip(self, xml_parser): + """Test parse_all without stripping whitespace.""" + xml_text = """ + spaced reasoning + spaced answer + """ + result_strip = xml_parser.parse_all(xml_text, strip=True) + result_no_strip = xml_parser.parse_all(xml_text, strip=False) + + assert result_strip.reasoning == ["spaced reasoning"] + assert result_strip.answer == ["spaced answer"] + assert result_no_strip.reasoning == ["spaced reasoning"] # regex pattern strips + assert result_no_strip.answer == ["spaced answer"] + + def test_parse_all_malformed_xml(self, xml_parser): + """Test parse_all with malformed XML tags.""" + xml_text = """ + Good reasoning + Good answer + Unclosed reasoning without proper closing + Another good reasoning + """ + result = xml_parser.parse_all(xml_text) + # Regex will find content between properly matched opening/closing tags + # The unclosed reasoning tag will match with the next closing tag + assert "Good reasoning" in result.reasoning + assert result.answer == ["Good answer"] + # Check that we got some reasoning results (behavior depends on regex matching) + assert len(result.reasoning) >= 1 + + def test_parse_all_nested_tags(self, xml_parser): + """Test parse_all with nested tags (should not match nested).""" + xml_text = """ + + Outer reasoning with nested reasoning inside + + Simple answer + """ + result = xml_parser.parse_all(xml_text) + # Due to non-greedy matching, this should work correctly + assert len(result.reasoning) >= 1 + assert result.answer == ["Simple answer"] + + def test_parse_all_empty_xml_parser(self): + """Test parse_all with XMLParser that has no fields.""" + empty_parser = XMLParser([]) + xml_text = "Should be ignoredAlso ignored" + result = empty_parser.parse_all(xml_text) + # Should have no attributes since no fields defined + assert not hasattr(result, 'reasoning') + assert not hasattr(result, 'answer') diff --git a/verifiers/envs/tool_env.py b/verifiers/envs/tool_env.py index 5b2ba9cb7f..86c34174cb 100644 --- a/verifiers/envs/tool_env.py +++ b/verifiers/envs/tool_env.py @@ -153,8 +153,48 @@ def env_response(self, state: State, **kwargs) -> Tuple[Message, State]: try: - parsed = self.parser.parse(messages[-1]['content']) - # Check if we got a valid tool field (not just None from failed parsing) + content = messages[-1]['content'] + + # Parse all tool calls (supports multiple) + parsed_all = self.parser.parse_all(content) + + # Check if we have any tool calls + if hasattr(parsed_all, 'tool') and len(parsed_all.tool) > 0: + results = [] + tool_names = [] + + # Execute each tool call + for tool_json in parsed_all.tool: + result = self.call_tool(tool_json) + results.append(result) + + # Extract tool name for labeling + try: + import json + tool_data = json.loads(tool_json) + tool_name = tool_data.get("name", "unknown_tool") + tool_names.append(tool_name) + except: + tool_names.append("unknown_tool") + + # Combine all results + if results: + if len(results) == 1: + # Single tool - no label needed + combined_results = results[0] + else: + # Multiple tools - label with tool names + labeled_results = [] + for tool_name, result in zip(tool_names, results): + labeled_results.append(f"{tool_name} result:\n{result}") + combined_results = "\n\n".join(labeled_results) + + return {'role': 'user', 'content': self.env_parser.format(result=combined_results)}, state + else: + return {'role': 'user', 'content': "Error: Tool execution returned no output."}, state + + # If no tools found, check with single parse (for backward compatibility) + parsed = self.parser.parse(content) if hasattr(parsed, 'tool') and parsed.tool is not None: result = self.call_tool(parsed.tool) if len(result.strip()) > 0: diff --git a/verifiers/parsers/xml_parser.py b/verifiers/parsers/xml_parser.py index c2446c3659..28a028cf2d 100644 --- a/verifiers/parsers/xml_parser.py +++ b/verifiers/parsers/xml_parser.py @@ -69,6 +69,28 @@ def parse(self, text: str, strip: bool = True) -> Any: results[alt] = None return SimpleNamespace(**results) + def parse_all(self, text: str, strip: bool = True) -> Any: + """ + Parse the given XML string and return an object with attributes corresponding + to all allowed tags in the schema, but return lists for each field to handle + multiple occurrences. + + For each field defined: + - Returns a list of all occurrences of that field (empty list if none found) + """ + results: Dict[str, List[str]] = {} + for canonical, alternatives in self._fields: + # For each allowed alternative tag, search all occurrences + for alt in alternatives: + # Regex pattern to capture all contents between the tags + pattern = rf"<{alt}>\s*(.*?)\s*" + matches = re.findall(pattern, text, re.DOTALL) + if matches: + results[alt] = [m.strip() if strip else m for m in matches] + else: + results[alt] = [] + return SimpleNamespace(**results) + def parse_answer(self, completion: Messages) -> str | None: """Extract the last answer from a completion.""" if isinstance(completion, str):