Fix tool input type conversion for nested dictionaries

[devin-ai-integration]

Fix Tool Input Type Conversion for Nested Dictionaries

This PR fixes issue #2826 where tool inputs were being modified when using different LLM providers.

Issue Description

When using custom tools with string input parameters, different LLM providers (like Ollama and Groq) format the tool inputs differently. Some providers wrap string inputs in a nested dictionary with a 'value' field, which causes validation errors when the tool expects a string but receives a dictionary.

For example, a tool expecting a string input like:

ai_query: str = Field(description="A string parameter")

Would receive input in different formats:

• From some providers: {"ai_query": "test query"}
• From others: {"ai_query": {"description": "A string parameter", "value": "test query"}}

The second format was causing validation errors because the tool was receiving a dictionary instead of a string.

Fix

The fix modifies the _parse_args method in CrewStructuredTool to handle nested dictionaries with 'value' fields for all parameter types. When a tool input is a dictionary with a 'value' field, we extract the value and use it directly.

# Handle nested dictionaries with 'value' field for all parameter types
if isinstance(raw_args, dict):
    schema_fields = self.args_schema.model_fields
    
    for field_name, field_value in list(raw_args.items()):
        # Check if this field exists in the schema
        if field_name in schema_fields:
            # If we got a dict with 'value' key for any field type
            if (isinstance(field_value, dict) and 'value' in field_value):
                raw_args[field_name] = field_value['value']

Testing

Added comprehensive unit tests that verify the fix works for:

1. String inputs with nested dictionaries
2. Integer inputs with nested dictionaries
3. Complex inputs with multiple field types
4. Direct invocation of tools with nested dictionary inputs
5. Error handling for invalid inputs

All tests pass, confirming that the fix correctly handles different input formats from various LLM providers.

Link to Devin run

https://app.devin.ai/sessions/7fb4ebc8824f4139a761dd316cc5eae8

Requested by: Joe Moura ([email protected])

[devin-ai-integration]

🤖 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]

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

Code Review Comment for PR #2827

Overview

This PR aims to enhance the structured tool implementation to manage nested dictionaries containing 'value' fields. This effort is in response to issue #2826 and showcases significant improvements in handling input across various LLM providers.

Files Reviewed

• src/crewai/tools/structured_tool.py
• tests/tools/test_structured_tool_nested_dict.py

Detailed Analysis

1. Modifications in structured_tool.py

Positive Aspects

• The code clearly outlines the handling of nested dictionaries, demonstrating a solid understanding of the problem space.
• Backward compatibility is preserved, ensuring there are no disruptions in existing functionality.
• Validation against schema fields is appropriately implemented, which is critical to maintaining data integrity.

Issues and Suggestions

1. Enhancement of Error Handling

   • Current Implementation:

     if isinstance(field_value, dict) and 'value' in field_value:
         raw_args[field_name] = field_value['value']

   • Improved Suggestion:

     if isinstance(field_value, dict):
         if 'value' in field_value:
             raw_args[field_name] = field_value['value']
         else:
             logger.warning(f"Nested dictionary for {field_name} missing 'value' key")

2. Type Validation Requirement

   • It's important to implement strict type validation to mitigate runtime errors:

     if isinstance(field_value, dict) and 'value' in field_value:
         value = field_value['value']
         expected_type = schema_fields[field_name].annotation
         if isinstance(value, expected_type):
             raw_args[field_name] = value
         else:
             raise ValueError(f"Invalid type for {field_name}: expected {expected_type}, got {type(value)}")

3. Documentation Improvement

   • Comprehensive docstrings can drastically improve code readability:

     def _parse_args(self, raw_args: Union[str, dict]) -> dict:
         """
         Parse and validate tool arguments.
         
         Args:
             raw_args: Arguments as string or dictionary, supporting nested dictionaries
                      with 'value' field for LLM provider compatibility.
         
         Returns:
             dict: Validated and processed arguments
         
         Raises:
             ValueError: If argument parsing or validation fails
         """

2. Insights on test_structured_tool_nested_dict.py

Positive Aspects

• The tests present thorough coverage, analyzing a broad array of input variations effectively.
• The organizational structure of test cases is clear, enhancing readability and maintainability.
• There's commendable testing diversity, including varying input types from simple strings to complex nested structures.

Suggestions for Improvement

1. Edge Cases Testing

   • Introducing edge cases tests is vital for identifying potential failure points:

     def test_edge_cases():
         # Test cases for empty nested dicts, null values, and deeply nested structures

2. Performance Testing Execution

   • Verifying performance with large inputs can ensure robust functionality:

     def test_performance():
         # Tests for performance with large nested structures

General Recommendations

1. Implement Logging: Proper logging mechanisms are essential for debugging and tracking.
2. Ensure Type Hints: Specific type hints can improve code safety and maintainability.
3. Documentation Enrichment: Examples in the docstrings illustrating nested dictionary use cases should be added.
4. Input Validation Policies: Establishing maximum nesting depth can prevent performance bottlenecks.
5. Performance Monitoring Metrics: Adding performance metrics related to argument parsing should be prioritized.

Security Considerations

1. Sanitization Needs: Nested dictionary values must be sanitized to avert security vulnerabilities.
2. Memory Usage Monitoring: It’s crucial to monitor memory usage when dealing with deep nesting.
3. Recursion Limits Introduction: Defining a maximum allowable nesting level can safeguard against stack overflow issues.

Overall, PR #2827 reflects thoughtful implementation and deserves commendation for its comprehensive test coverage. The proposed suggestions focus on enhancing error handling and robustness while ensuring core functionalities remain intact.

[devin-ai-integration]

Closing due to inactivity for more than 7 days.