fix: address PR feedback with improved validation, documentation, and tests

Co-Authored-By: Joe Moura <joao@crewai.com>

docs/how-to/ToolInstructions.md

@@ -32,6 +32,113 @@ agent = Agent(
 )
 ```
 
+## Real-World Examples
+
+### Example 1: Research Assistant with Web Search Tool
+
+```python
+from crewai import Agent
+from crewai_tools import SearchTool
+from crewai.tools import ToolWithInstruction
+
+search_tool = SearchTool()
+search_with_instructions = ToolWithInstruction(
+    tool=search_tool,
+    instructions="""
+    Use this tool ONLY for factual information that requires up-to-date data.
+    ALWAYS verify information by searching multiple sources.
+    DO NOT use this tool for speculative questions or opinions.
+    """
+)
+
+research_agent = Agent(
+    role="Research Analyst",
+    goal="Provide accurate and well-sourced information",
+    backstory="You are a meticulous research analyst with attention to detail and fact-checking.",
+    tools=[search_with_instructions],
+)
+```
+
+### Example 2: Data Scientist with Multiple Analysis Tools
+
+```python
+from crewai import Agent
+from crewai_tools import PythonTool, DataVisualizationTool
+from crewai.tools import ToolWithInstruction
+
+# Python tool for data processing
+python_tool = PythonTool()
+python_with_instructions = ToolWithInstruction(
+    tool=python_tool,
+    instructions="""
+    Use this tool for data cleaning, transformation, and statistical analysis.
+    ALWAYS include comments in your code.
+    DO NOT use this tool for creating visualizations.
+    """
+)
+
+# Visualization tool
+viz_tool = DataVisualizationTool()
+viz_with_instructions = ToolWithInstruction(
+    tool=viz_tool,
+    instructions="""
+    Use this tool ONLY for creating data visualizations.
+    ALWAYS label axes and include titles in your charts.
+    PREFER simple visualizations that clearly communicate the main insight.
+    """
+)
+
+data_scientist = Agent(
+    role="Data Scientist",
+    goal="Analyze data and create insightful visualizations",
+    backstory="You are an experienced data scientist who excels at finding patterns in data.",
+    tools=[python_with_instructions, viz_with_instructions],
+)
+```
+
+## How Instructions Are Presented to Agents
+
+When an agent considers using a tool, the instructions are included in the tool's description. For example, a tool with instructions might appear to the agent like this:
+
+```
+Tool: search_web
+Description: Search the web for information on a given topic.
+Instructions: Use this tool ONLY for factual information that requires up-to-date data. 
+ALWAYS verify information by searching multiple sources. 
+DO NOT use this tool for speculative questions or opinions.
+```
+
+This clear presentation helps the agent understand when and how to use the tool appropriately.
+
+## Dynamically Updating Instructions
+
+You can update tool instructions dynamically during execution:
+
+```python
+# Create a tool with initial instructions
+search_with_instructions = ToolWithInstruction(
+    tool=search_tool,
+    instructions="Initial instructions for tool usage"
+)
+
+# Later, update the instructions based on new requirements
+search_with_instructions.update_instructions("Updated instructions for tool usage")
+```
+
+## Error Handling and Best Practices
+
+### Validation
+
+The `ToolWithInstruction` class includes validation to ensure instructions are not empty and don't exceed a maximum length. If you provide invalid instructions, a `ValueError` will be raised.
+
+### Best Practices for Writing Instructions
+
+1. **Be specific and clear** about when to use and when not to use the tool
+2. **Use imperative language** like "ALWAYS", "NEVER", "USE", "DO NOT USE"
+3. **Keep instructions concise** but comprehensive
+4. **Include examples** of good and bad usage scenarios when possible
+5. **Format instructions** with line breaks for readability
+
 ## When to Use Tool Instructions
 
 Tool instructions are useful when:
@@ -40,5 +147,7 @@ Tool instructions are useful when:
 2. You have multiple similar tools that should be used in different situations
 3. You want to keep the agent's backstory focused on its role and personality, 
    not technical details about tools
+4. You need to provide technical guidance on how to format inputs or interpret outputs
+5. You want to enforce consistent tool usage across multiple agents
 
 Tool instructions are semantically more correct than putting tool usage guidelines in the agent's backstory.

src/crewai/tools/tool_with_instruction.py

@@ -1,6 +1,6 @@
-from typing import Any, List, Optional, Dict, Callable, Union
+from typing import Any, List, Optional, Dict, Callable, Union, ClassVar
 
-from pydantic import Field, model_validator
+from pydantic import Field, model_validator, field_validator, ConfigDict
 
 from crewai.tools.base_tool import BaseTool
 from crewai.tools.structured_tool import CrewStructuredTool
@@ -19,25 +19,90 @@ class ToolWithInstruction(BaseTool):
         description: Description of the tool (inherited from the wrapped tool with instructions)
     """
     
+    MAX_INSTRUCTION_LENGTH: ClassVar[int] = 2000
+    
     name: str = Field(default="", description="Name of the tool")
     description: str = Field(default="", description="Description of the tool")
     tool: BaseTool = Field(description="The tool to wrap")
     instructions: str = Field(description="Instructions about when and how to use this tool")
     
+    model_config = ConfigDict(arbitrary_types_allowed=True)
+    
+    @field_validator("instructions")
+    @classmethod
+    def validate_instructions(cls, value: str) -> str:
+        """Validate that instructions are not empty and not too long.
+        
+        Args:
+            value: The instructions string to validate
+            
+        Returns:
+            str: The validated and sanitized instructions
+            
+        Raises:
+            ValueError: If instructions are empty or exceed maximum length
+        """
+        if not value or not value.strip():
+            raise ValueError("Instructions cannot be empty")
+        
+        if len(value) > cls.MAX_INSTRUCTION_LENGTH:
+            raise ValueError(
+                f"Instructions exceed maximum length of {cls.MAX_INSTRUCTION_LENGTH} characters"
+            )
+        
+        return value.strip()
+    
     @model_validator(mode="after")
     def set_tool_attributes(self) -> "ToolWithInstruction":
-        """Set attributes from the wrapped tool."""
+        """Sets name, description, and args_schema from the wrapped tool.
+        
+        Returns:
+            ToolWithInstruction: The validated instance with updated attributes.
+        """
         self.name = self.tool.name
         self.description = f"{self.tool.description}\nInstructions: {self.instructions}"
         self.args_schema = self.tool.args_schema
         return self
     
+    def update_instructions(self, new_instructions: str) -> None:
+        """Updates the tool's usage instructions.
+        
+        Args:
+            new_instructions (str): New instructions for tool usage.
+            
+        Raises:
+            ValueError: If new instructions are empty or exceed maximum length
+        """
+        if not new_instructions or not new_instructions.strip():
+            raise ValueError("Instructions cannot be empty")
+        
+        if len(new_instructions) > self.MAX_INSTRUCTION_LENGTH:
+            raise ValueError(
+                f"Instructions exceed maximum length of {self.MAX_INSTRUCTION_LENGTH} characters"
+            )
+        
+        self.instructions = new_instructions.strip()
+        
+        self.description = f"{self.tool.description}\nInstructions: {self.instructions}"
+    
     def _run(self, *args: Any, **kwargs: Any) -> Any:
-        """Run the wrapped tool."""
+        """Run the wrapped tool.
+        
+        Args:
+            *args: Positional arguments to pass to the wrapped tool
+            **kwargs: Keyword arguments to pass to the wrapped tool
+            
+        Returns:
+            Any: The result from the wrapped tool's _run method
+        """
         return self.tool._run(*args, **kwargs)
     
     def to_structured_tool(self) -> CrewStructuredTool:
-        """Convert this tool to a CrewStructuredTool instance."""
+        """Convert this tool to a CrewStructuredTool instance.
+        
+        Returns:
+            CrewStructuredTool: A structured tool with instructions included in the description
+        """
         structured_tool = self.tool.to_structured_tool()
         
         structured_tool.description = f"{structured_tool.description}\nInstructions: {self.instructions}"

tests/tools/test_tool_with_instruction.py

@@ -65,3 +65,46 @@ class TestToolWithInstruction:
         
         assert wrapped_tool.name == tool.name
         assert "Instructions: Only use this tool for XYZ" in wrapped_tool.description
+    
+    def test_empty_instructions(self):
+        """Test that empty instructions raise ValueError."""
+        tool = MockTool()
+        
+        with pytest.raises(ValueError, match="Instructions cannot be empty"):
+            ToolWithInstruction(tool=tool, instructions="")
+        
+        with pytest.raises(ValueError, match="Instructions cannot be empty"):
+            ToolWithInstruction(tool=tool, instructions="   ")
+    
+    def test_too_long_instructions(self):
+        """Test that instructions exceeding maximum length raise ValueError."""
+        tool = MockTool()
+        long_instructions = "x" * (ToolWithInstruction.MAX_INSTRUCTION_LENGTH + 1)
+        
+        with pytest.raises(ValueError, match="Instructions exceed maximum length"):
+            ToolWithInstruction(tool=tool, instructions=long_instructions)
+    
+    def test_update_instructions(self):
+        """Test updating instructions dynamically."""
+        tool = MockTool()
+        initial_instructions = "Initial instructions"
+        new_instructions = "Updated instructions"
+        
+        wrapped_tool = ToolWithInstruction(tool=tool, instructions=initial_instructions)
+        assert "Instructions: Initial instructions" in wrapped_tool.description
+        
+        wrapped_tool.update_instructions(new_instructions)
+        assert "Instructions: Updated instructions" in wrapped_tool.description
+        assert wrapped_tool.instructions == new_instructions
+    
+    def test_update_instructions_validation(self):
+        """Test validation when updating instructions."""
+        tool = MockTool()
+        wrapped_tool = ToolWithInstruction(tool=tool, instructions="Valid instructions")
+        
+        with pytest.raises(ValueError, match="Instructions cannot be empty"):
+            wrapped_tool.update_instructions("")
+        
+        long_instructions = "x" * (ToolWithInstruction.MAX_INSTRUCTION_LENGTH + 1)
+        with pytest.raises(ValueError, match="Instructions exceed maximum length"):
+            wrapped_tool.update_instructions(long_instructions)