Skip to content
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

Fix issue #2383: Add invoke method to BaseTool for models without function calling support #2384

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

devin-ai-integration[bot]
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Mar 17, 2025

Fix issue #2383: Add invoke method to BaseTool for models without function calling support

This PR fixes issue #2383 where custom tools' _run method is not being called when using models like QwQ-32B-Preview and deepseek-chat that don't support function calling.

Problem

Some models (specifically QwQ-32B-Preview and deepseek-chat) don't support function calling, and the tool execution mechanism in CrewAI relies on the invoke() method which is only implemented in CrewStructuredTool but not in BaseTool.

Solution

The fix adds an invoke() method to BaseTool that:

  1. Handles string inputs by parsing them as JSON
  2. Handles dictionary inputs by filtering them based on the args_schema
  3. Properly calls the _run method with the appropriate arguments

This ensures that tools can be executed even when using models that don't support function calling.

Security Enhancements

Based on code review feedback, the implementation includes several security features:

  1. Input sanitization to prevent potential exploits through JSON parsing
  2. Input size limits (100KB) to mitigate risks associated with excessive data processing
  3. Nested dictionary depth validation (max depth of 5) to avoid recursion issues
  4. Comprehensive logging for debugging and monitoring
  5. Robust error handling with specific error messages

Testing

Added comprehensive tests to verify the fix works correctly with:

  • Dictionary inputs
  • JSON string inputs
  • Raw string inputs
  • Empty dictionary inputs
  • Inputs with extra arguments
  • Invalid input types
  • Large inputs exceeding size limits
  • Deeply nested dictionaries

Link to Devin run: https://app.devin.ai/sessions/32a7fbb5786649ca817046f78ec4b9f1
Requested by: User

…ction calling support

Co-Authored-By: Joe Moura <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 #2384

Overview

This pull request (PR) introduces an invoke method in the BaseTool class to enhance compatibility with models that lack native function calling support. The primary goal of this change is to address issue #2383, allowing tools that utilize the _run method to function properly for models like QwQ-32B-Preview and deepseek-chat.

Key Findings

Positive Aspects

  • Error Handling: Effective error handling is evident, particularly with the fallback mechanisms for JSON parsing.
  • Type Hints and Documentation: The use of clear type hints (with Union and Optional) and method documentation greatly aids in code readability and maintainability.
  • Comprehensive Testing: The new test cases demonstrate a commendable approach to testing various input scenarios, ensuring robust coverage.

Specific Code Improvements

  1. Error Handling Enhancement:

    except json.JSONDecodeError as e:
        logger.debug(f"Input string is not JSON format: {e}")
  2. Import Optimization: Move JSON import to the top of the file for clarity and efficiency.

    import json
  3. Type Validation:
    Enhance the input validation logic in the invoke method:

    if not isinstance(input, (str, dict)):
        raise ValueError(f"Input must be string or dict, got {type(input)}")
  4. Schema Validation Improvement:
    Improve argument filtering in the schema validation:

    filtered_args = {}
    for k in input.keys():
        if k in arg_names:
            filtered_args[k] = input[k]
        else:
            logger.warning(f"Ignoring unexpected argument: {k}")

Testing Recommendations

  1. Add Edge Cases:
    Implement tests for invalid input types and malformed JSON, which are critical for ensuring resilience.

    def test_invoke_with_invalid_type():
        with pytest.raises(ValueError, match="Input must be string or dict"):
            tool.invoke(input=123)
  2. Configuration Testing:
    Verify the invoke method's behavior when handling configuration dictionaries to extend test coverage:

    def test_invoke_with_config():
        tool.invoke(input={"param": "test value"}, config={"timeout": 30})

General Recommendations

  • Logging: Increase logging throughout the code to enhance debugging capabilities.
  • Input Validation: Include more robust validation for configuration parameters.
  • Return Type Hints: Add return type hints to the invoke method for clarity.
  • Documentation: Provide usage examples in docstrings to assist other developers.

Security Considerations

  • Implement input sanitization to prevent potential exploits through JSON parsing.
  • Enforce limits on input size to mitigate risks associated with excessive data processing.
  • Establish validation norms for nested dictionary depths to avoid recursion issues.

Related PR History

Reviewing similar PRs that modified BaseTool could provide insights into common patterns and design decisions that inform this change. This could also highlight previous discussions on input handling and parser reliability.

Implementing the suggested improvements would reinforce the BaseTool's functionality, increasing its robustness, maintainability, and security posture, thereby enhancing the overall user experience in the CrewAI ecosystem.

devin-ai-integration bot and others added 2 commits March 17, 2025 02:51
…validation

Co-Authored-By: Joe Moura <joao@crewai.com>
…d dictionary depth validation

Co-Authored-By: Joe Moura <joao@crewai.com>
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.

1 participant