diff --git a/src/crewai/utilities/converter.py b/src/crewai/utilities/converter.py index 8aea317c1..18d1f393c 100644 --- a/src/crewai/utilities/converter.py +++ b/src/crewai/utilities/converter.py @@ -1,6 +1,7 @@ import json +import logging import re -from typing import Any, Optional, Type, Union, get_args, get_origin +from typing import Any, ClassVar, Optional, Type, Union, get_args, get_origin from pydantic import BaseModel, ValidationError @@ -17,8 +18,18 @@ class ConverterError(Exception): self.message = message +class InstructorToolCallError(Exception): + """Error raised when Instructor does not support multiple tool calls.""" + + def __init__(self, message: str, *args: object) -> None: + super().__init__(message, *args) + self.message = message + + class Converter(OutputConverter): """Class that converts text into either pydantic or json.""" + + logger: ClassVar[logging.Logger] = logging.getLogger(__name__) def to_pydantic(self, current_attempt=1) -> BaseModel: """Convert text to pydantic.""" @@ -68,43 +79,71 @@ class Converter(OutputConverter): f"Failed to convert text into a Pydantic model due to error: {e}" ) - def to_json(self, current_attempt=1): - """Convert text to json.""" + def to_json(self, current_attempt: int = 1) -> Union[str, ConverterError]: + """ + Convert text to JSON. + + Args: + current_attempt: The current attempt number for retries. + + Returns: + A JSON string or a ConverterError if conversion fails. + """ try: if self.llm.supports_function_calling(): try: + self.logger.debug("Using Instructor for JSON conversion") return self._create_instructor().to_json() except Exception as e: # Check if this is the specific Instructor error for multiple tool calls if "Instructor does not support multiple tool calls, use List[Model] instead" in str(e): - # Fall back to non-function calling approach for custom OpenAI backends - return json.dumps( - self.llm.call( - [ - {"role": "system", "content": self.instructions}, - {"role": "user", "content": self.text}, - ] - ) + self.logger.warning( + "Instructor does not support multiple tool calls, falling back to simple JSON conversion" ) + return self._fallback_json_conversion() raise e else: - return json.dumps( - self.llm.call( - [ - {"role": "system", "content": self.instructions}, - {"role": "user", "content": self.text}, - ] - ) - ) + self.logger.debug("Using simple JSON conversion (no function calling support)") + return self._fallback_json_conversion() except Exception as e: if current_attempt < self.max_attempts: + self.logger.warning(f"JSON conversion failed, retrying (attempt {current_attempt})") return self.to_json(current_attempt + 1) + self.logger.error(f"JSON conversion failed after {self.max_attempts} attempts: {e}") return ConverterError(f"Failed to convert text into JSON, error: {e}.") + def _fallback_json_conversion(self) -> str: + """Convert text to JSON using a simple approach without Instructor.""" + self.logger.debug("Using fallback JSON conversion method") + response = self.llm.call( + [ + {"role": "system", "content": self.instructions}, + {"role": "user", "content": self.text}, + ] + ) + + # Try to parse the response as JSON to ensure it's valid + try: + # If it's already a valid JSON string, just return it + if isinstance(response, str): + json.loads(response) + return response + # If it's a dict or other JSON-serializable object, convert it to a JSON string + return json.dumps(response) + except json.JSONDecodeError as e: + self.logger.error(f"Invalid JSON in fallback conversion: {e}") + raise ConverterError(f"Failed to convert text into JSON, error: {e}.") + def _create_instructor(self): - """Create an instructor.""" + """ + Create an instructor instance for JSON conversion. + + Returns: + An InternalInstructor instance. + """ from crewai.utilities import InternalInstructor + self.logger.debug("Creating InternalInstructor instance") inst = InternalInstructor( llm=self.llm, model=self.model, diff --git a/tests/utilities/test_custom_openai_json.py b/tests/utilities/test_custom_openai_json.py index be380f20f..f32274776 100644 --- a/tests/utilities/test_custom_openai_json.py +++ b/tests/utilities/test_custom_openai_json.py @@ -1,10 +1,11 @@ import json -import pytest -from pydantic import BaseModel from unittest.mock import Mock, patch +import pytest +from pydantic import BaseModel + from crewai.llm import LLM -from crewai.utilities.converter import Converter +from crewai.utilities.converter import Converter, ConverterError class SimpleModel(BaseModel): @@ -12,52 +13,72 @@ class SimpleModel(BaseModel): age: int +@pytest.fixture +def mock_llm_with_function_calling(): + """Create a mock LLM that supports function calling.""" + llm = Mock(spec=LLM) + llm.supports_function_calling.return_value = True + llm.call.return_value = '{"name": "John", "age": 30}' + return llm + + +@pytest.fixture +def mock_instructor_with_error(): + """Create a mock Instructor that raises the specific error.""" + mock_instructor = Mock() + mock_instructor.to_json.side_effect = Exception( + "Instructor does not support multiple tool calls, use List[Model] instead" + ) + return mock_instructor + + class TestCustomOpenAIJson: - def test_custom_openai_json_conversion_with_instructor_error(self): + def test_custom_openai_json_conversion_with_instructor_error(self, mock_llm_with_function_calling, mock_instructor_with_error): """Test that JSON conversion works with custom OpenAI backends when Instructor raises an error.""" - # Mock LLM that supports function calling - llm = Mock(spec=LLM) - llm.supports_function_calling.return_value = True - llm.call.return_value = '{"name": "John", "age": 30}' - - # Mock Instructor that raises the specific error - mock_instructor = Mock() - mock_instructor.to_json.side_effect = Exception( - "Instructor does not support multiple tool calls, use List[Model] instead" - ) - # Create converter with mocked dependencies converter = Converter( - llm=llm, + llm=mock_llm_with_function_calling, text="Convert this to JSON", model=SimpleModel, instructions="Convert to JSON", ) # Mock the _create_instructor method to return our mocked instructor - with patch.object(converter, '_create_instructor', return_value=mock_instructor): + with patch.object(converter, '_create_instructor', return_value=mock_instructor_with_error): # Call to_json method result = converter.to_json() # Verify that the fallback mechanism was used - llm.call.assert_called_once() - # The result is a JSON string, so we need to parse it - parsed_result = json.loads(result) - assert parsed_result == '{"name": "John", "age": 30}' or parsed_result == {"name": "John", "age": 30} + mock_llm_with_function_calling.call.assert_called_once() + + # The result should be a JSON string + assert isinstance(result, str) + + # The result might be a string representation of a JSON string + # Try to parse it directly first, and if that fails, try to parse it as a string representation + try: + parsed_result = json.loads(result) + except json.JSONDecodeError: + # If it's a string representation of a JSON string, it will be surrounded by quotes + # and have escaped quotes inside + if result.startswith('"') and result.endswith('"'): + # Remove the surrounding quotes and unescape the string + unescaped = result[1:-1].replace('\\"', '"') + parsed_result = json.loads(unescaped) + + assert isinstance(parsed_result, dict) + assert parsed_result.get("name") == "John" + assert parsed_result.get("age") == 30 - def test_custom_openai_json_conversion_without_error(self): + def test_custom_openai_json_conversion_without_error(self, mock_llm_with_function_calling): """Test that JSON conversion works normally when Instructor doesn't raise an error.""" - # Mock LLM that supports function calling - llm = Mock(spec=LLM) - llm.supports_function_calling.return_value = True - # Mock Instructor that returns JSON without error mock_instructor = Mock() mock_instructor.to_json.return_value = '{"name": "John", "age": 30}' # Create converter with mocked dependencies converter = Converter( - llm=llm, + llm=mock_llm_with_function_calling, text="Convert this to JSON", model=SimpleModel, instructions="Convert to JSON", @@ -69,5 +90,36 @@ class TestCustomOpenAIJson: result = converter.to_json() # Verify that the normal path was used (no fallback) - llm.call.assert_not_called() - assert json.loads(result) == {"name": "John", "age": 30} + mock_llm_with_function_calling.call.assert_not_called() + + # Verify the result matches the expected output + assert result == '{"name": "John", "age": 30}' + + def test_custom_openai_json_conversion_with_invalid_json(self, mock_llm_with_function_calling): + """Test that JSON conversion handles invalid JSON gracefully.""" + # Mock LLM to return invalid JSON + mock_llm_with_function_calling.call.return_value = 'invalid json' + + # Mock Instructor that raises the specific error + mock_instructor = Mock() + mock_instructor.to_json.side_effect = Exception( + "Instructor does not support multiple tool calls, use List[Model] instead" + ) + + # Create converter with mocked dependencies + converter = Converter( + llm=mock_llm_with_function_calling, + text="Convert this to JSON", + model=SimpleModel, + instructions="Convert to JSON", + max_attempts=1, # Set max_attempts to 1 to avoid retries + ) + + # Mock the _create_instructor method to return our mocked instructor + with patch.object(converter, '_create_instructor', return_value=mock_instructor): + # Call to_json method + result = converter.to_json() + + # The result should be a ConverterError instance + assert isinstance(result, ConverterError) + assert "invalid json" in str(result).lower() or "expecting value" in str(result).lower()