Skip to content

Fix Pydantic validation error in LLMCallStartedEvent when TokenCalcHandler in tools list #3044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Fix Pydantic validation error in LLMCallStartedEvent when TokenCalcHandler in tools list

Problem Description

This PR fixes issue #3043 where a Pydantic ValidationError occurs in LLMCallStartedEvent when TokenCalcHandler objects are passed in the tools list instead of dict objects.

Error Details

  • Error: Input should be a valid dictionary [type=dict_type, input_value=<crewai.utilities.token_counter_callback.TokenCalcHandler object>, input_type=TokenCalcHandler]
  • Root Cause: The LLMCallStartedEvent schema expects tools: Optional[List[dict]] = None but receives TokenCalcHandler objects
  • Environment: Python 3.12, Pydantic v2.11.7, CrewAI 0.130.0

Reproduction Case

from crewai.utilities.events.llm_events import LLMCallStartedEvent
from crewai.utilities.token_counter_callback import TokenCalcHandler
from crewai.agents.agent_builder.utilities.base_token_process import TokenProcess

token_handler = TokenCalcHandler(TokenProcess())

# This would cause ValidationError before the fix
event = LLMCallStartedEvent(
    messages=[{"role": "user", "content": "test message"}],
    tools=[{"name": "tool1"}, token_handler],  # Mixed dict and TokenCalcHandler
    callbacks=None
)

Solution Approach

The fix implements input sanitization using Pydantic's @model_validator(mode='before') decorator to filter out non-dict objects from the tools list before validation occurs.

Key Changes

  1. Added model validator: sanitize_tools method that filters the tools list
  2. Preserves dict tools: Only removes non-dict objects, keeping valid tool dictionaries
  3. Maintains backward compatibility: No changes to the public API or existing functionality
  4. Follows Pydantic v2 best practices: Uses proper validation patterns

Implementation Details

@model_validator(mode='before')
@classmethod
def sanitize_tools(cls, values):
    """Sanitize tools list to only include dict objects, filtering out non-dict objects like TokenCalcHandler"""
    if isinstance(values, dict) and 'tools' in values and values['tools'] is not None:
        if isinstance(values['tools'], list):
            values['tools'] = [tool for tool in values['tools'] if isinstance(tool, dict)]
    return values

Testing and Verification

Comprehensive Test Coverage

  • Original bug reproduction: Confirms the ValidationError is fixed
  • Normal functionality: Dict tools continue to work correctly
  • Edge cases: Empty lists, None values, mixed object types
  • TokenCalcHandler filtering: Non-dict objects properly removed
  • Callbacks preservation: TokenCalcHandler in callbacks still works
  • All test scenarios: 10 comprehensive test cases covering various scenarios

Test Results

Running comprehensive test suite for LLMCallStartedEvent validation fix...
======================================================================
✅ test_normal_dict_tools_work: PASSED
✅ test_token_calc_handler_in_tools_filtered_out: PASSED
✅ test_mixed_objects_in_tools_only_dicts_preserved: PASSED
✅ test_empty_tools_list_handled: PASSED
✅ test_none_tools_handled: PASSED
✅ test_all_non_dict_tools_results_in_empty_list: PASSED
✅ test_reproduction_case_from_issue_3043: PASSED
✅ test_callbacks_with_token_handler_still_work: PASSED
✅ test_string_messages_work: PASSED
✅ test_available_functions_preserved: PASSED
======================================================================
RESULTS: 10 passed, 0 failed
✅ ALL TESTS PASSED!

Before/After Comparison

Before Fix:

❌ ValidationError reproduced: 1 validation error for LLMCallStartedEvent
tools.1
  Input should be a valid dictionary [type=dict_type, input_value=<crewai.utilities.token_counter_callback.TokenCalcHandler object>, input_type=TokenCalcHandler]

After Fix:

✅ SUCCESS: No ValidationError occurred!
   - Event created successfully
   - Tools list sanitized: [{'name': 'tool1'}]
   - Only dict tools preserved: 1 tools

Files Changed

  • src/crewai/utilities/events/llm_events.py: Added model validator for input sanitization
  • tests/utilities/events/test_llm_events_validation.py: Comprehensive test coverage

Backward Compatibility

  • ✅ No breaking changes to public API
  • ✅ Existing functionality preserved
  • ✅ Dict tools continue to work as expected
  • ✅ TokenCalcHandler in callbacks still functions normally

Link to Devin run

https://app.devin.ai/sessions/062f30b8b91241a4a0af88a6f7c73934

Requested by

João (joao@crewai.com)

Resolves #3043

…ndler in tools list

- Add model_validator to sanitize tools list before validation
- Filter out non-dict objects like TokenCalcHandler from tools list
- Preserve dict tools while removing problematic objects
- Add comprehensive test coverage for the fix and edge cases
- Resolves issue #3043

Co-Authored-By: João <joao@crewai.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #3044

Summary

This pull request addresses a critical Pydantic validation issue in the LLMCallStartedEvent when invalid TokenCalcHandler objects are present within the tools list. The implementation introduces a robust model validator that effectively filtrates the tools, ensuring only valid dictionary objects are retained. The comprehensive test coverage further solidifies the reliability of the changes made.

Code Quality Findings

Positive Aspects

  • The addition of the @model_validator decorator enhances the Pydantic validation process, effectively sanitizing the tools list prior to evaluation.
  • Type hints are used effectively throughout the code, promoting better readability and understanding of expected input data types.
  • The methods have clear and descriptive naming conventions, such as sanitize_tools, which aids in quick comprehension of their functionality.

Areas for Improvement

  1. Documentation Enhancement:

    • Improve the documentation for the sanitize_tools method to describe parameters and return values clearly. For example:
    def sanitize_tools(cls, values):
        """Sanitize tools list to only include dict objects; filters out non-dict objects like TokenCalcHandler.
        
        Args:
            values (dict): Input values dictionary containing tools.
    
        Returns:
            dict: Sanitized values with filtered tools.
        
        Example:
            >>> tools = [{"name": "tool1"}, TokenCalcHandler(), {"name": "tool2"}]
            >>> sanitized = sanitize_tools({"tools": tools})
            >>> # Expected: {"tools": [{"name": "tool1"}, {"name": "tool2"}]}
        """
  2. Error Handling Improvement:

    • Enhance error handling within sanitize_tools to log and manage unexpected input scenarios:
    def sanitize_tools(cls, values):
        try:
            if ...
        except Exception as e:
            logger.warning(f"Error during tools sanitization: {e}")
            return values
  3. Type Validation Strengthening:

    • Increase the type validation process by ensuring all values within the dictionary conform to expected data structures:
    def sanitize_tools(cls, values):
        if isinstance(values, dict) and 'tools' in values and values['tools'] is not None:
            values['tools'] = [
                tool for tool in values['tools'] 
                if isinstance(tool, dict) and all(isinstance(v, (str, int, float, bool, dict, list)) for v in tool.values())
            ]
        return values

Testing Improvements

  • The test suite is well-structured but could benefit from parameterized tests to enhance the coverage and organization:
@pytest.mark.parametrize("tools_input,expected", [
    ([{"name": "tool1"}, TokenCalcHandler()], [{"name": "tool1"}]),
    ...
])
def test_tools_sanitization(tools_input, expected):
    ...
  • Consider adding performance tests for large datasets to ensure the sanitizer scales efficiently under load:
def test_sanitize_tools_performance():
    ...

Historical Context

Given the previous issues with the validation of tools (referenced in issue #3043), this PR represents a significant step towards improving the stability and functionality of LLMCallStartedEvent. Maintaining robust validation and sanitization logic is crucial for preventing runtime errors and ensuring data integrity across the application.

Security Considerations

  • The code appears to handle input validation adequately, with no immediate vulnerabilities noted. However, consider implementing deeper input validation to protect against unforeseen cases where dictionary values might cause issues.

Performance Impact

  • The validation processes implemented possess O(n) complexity concerning the tools list, which is efficient, and memory usage is optimized by employing list comprehensions.

Conclusion

The changes in this PR significantly enhance the handling of event validation while preserving the integrity of the application. The proposed recommendations aim to further strengthen the overall quality, performance, and security of the implementation. The continuous improvement based on lessons learned from previous PRs and issues will contribute to a more robust codebase moving forward.

@mplachta
Copy link
Contributor

Disclaimer: This review was made by a crew of AI Agents.

This PR #3044 presents a focused and well-engineered fix for the Pydantic validation error reported in issue #3043 related to the LLMCallStartedEvent model’s tools field. Below is a detailed code review covering code quality, historical context, implications, and improvement suggestions:


Summary of Key Findings

  • Core Fix:
    Introduces a Pydantic @model_validator(mode='before') method sanitize_tools on LLMCallStartedEvent which cleans the tools list before validation by filtering out any non-dict objects. This prevents validation errors caused by objects like TokenCalcHandler appearing in the tools field.

  • Testing:
    Comprehensive tests in a new test file cover typical cases, edge cases, and the exact reproduction case from the reported GitHub issue [BUG] Pydantic Schema Validation in CrewAI Events #3043. Tests verify that only dicts remain in tools, while allowing TokenCalcHandler and other objects in callbacks to remain unaffected. Other fields (messages, available_functions) are verified to be preserved correctly.

  • Code Quality:
    The validator method is concise with a clear docstring. It correctly handles None and empty lists, ensuring no side effects. The patch adheres well to the repository’s style and uses the current Pydantic v2 validation API appropriately.

  • Backward Compatibility:
    The use of Pydantic v2’s model_validator means this fix requires Pydantic v2 compatibility in your environment. This is a notable breaking change if the project has not yet transitioned.


Specific Code Improvements & Suggestions

  1. Add Type Annotations to Validator
    For better maintainability and IDE support, add type hints to sanitize_tools:

    from typing import Any, Type
    
    @model_validator(mode='before')
    @classmethod
    def sanitize_tools(cls: Type["LLMCallStartedEvent"], values: Any) -> Any:
        ...
  2. Optional Logging When Filtering Occurs
    To improve observability and debug unexpected filtering, consider logging when non-dict tools are removed:

    import logging
    logger = logging.getLogger("crewai.events")
    
    @model_validator(mode='before')
    @classmethod
    def sanitize_tools(cls, values):
        if isinstance(values, dict) and 'tools' in values and values['tools'] is not None:
            if isinstance(values['tools'], list):
                original_len = len(values['tools'])
                values['tools'] = [tool for tool in values['tools'] if isinstance(tool, dict)]
                if len(values['tools']) < original_len:
                    logger.warning("LLMCallStartedEvent: Filtered non-dict objects from tools list")
        return values
  3. Consider Extending Tests for Negative Scenarios
    While existing tests are thorough, adding tests to confirm errors or exceptions if tools contain dicts with invalid structure can further harden validation.

  4. Parametrize Some Tests for Conciseness (Optional)
    Using pytest.mark.parametrize for similar tests could reduce boilerplate, improving readability.


Historical Context & Related PRs

  • Issue [BUG] Pydantic Schema Validation in CrewAI Events #3043 identified a critical Pydantic validation failure due to non-dict objects like TokenCalcHandler appearing in tools.
  • This PR directly addresses that with a sanitizer, preventing such objects from causing errors.
  • The team’s choice of Pydantic v2's model_validator indicates an intentional alignment with updated validation architecture, reflecting modern best practices.

Impact on Related Files and Modules

  • The fix only modifies LLMCallStartedEvent in the event model.
  • The extensive new test suite under tests/utilities/events/test_llm_events_validation.py improves stability in this area.
  • No other event models or unrelated files are impacted.
  • Usage of TokenCalcHandler remains valid in callbacks, ensuring unrelated workflows are unaffected.

Final Code Snippet Example (with Suggested Improvements)

from typing import Any, Type
import logging

logger = logging.getLogger("crewai.events")

class LLMCallStartedEvent(BaseEvent):
    # existing fields ...

    @model_validator(mode='before')
    @classmethod
    def sanitize_tools(cls: Type["LLMCallStartedEvent"], values: Any) -> Any:
        """
        Sanitize the 'tools' list to only include dict objects,
        filtering out non-dict objects like TokenCalcHandler.
        """
        if isinstance(values, dict) and 'tools' in values and values['tools'] is not None:
            if isinstance(values['tools'], list):
                original_len = len(values['tools'])
                values['tools'] = [tool for tool in values['tools'] if isinstance(tool, dict)]
                if len(values['tools']) < original_len:
                    logger.warning("Filtered non-dict objects from tools list in LLMCallStartedEvent")
        return values

Conclusion

This PR is a solid, well-tested fix that improves data validation robustness for a key event model. Approval is recommended with suggested minor enhancements for type hints and optional debug logging to facilitate future maintenance. Ensure the project environment requires Pydantic v2 as a baseline.

Excellent job on addressing the reported issue comprehensively and safeguarding against similar future bugs.


If further refinements or assistance with integrating this into the mainline are desired, feel free to request a follow-up review.

Thank you for the thoughtful implementation and thorough tests!

…mentation, error handling, and type validation

- Add comprehensive docstring with Args, Returns, and Example sections
- Implement try-catch error handling with logging for unexpected scenarios
- Add stronger type validation for dictionary values
- Include logging for debugging when non-dict objects are filtered
- Add type annotations for better maintainability and IDE support
- Add parameterized tests for better coverage and organization
- Add performance tests for large datasets
- Add tests for invalid dict value types and error handling scenarios

Addresses feedback from joaomdmoura and mplachta on PR #3044

Co-Authored-By: João <joao@crewai.com>
Copy link
Contributor Author

Closing due to inactivity for more than 7 days. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Pydantic Schema Validation in CrewAI Events
2 participants