mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-30 02:28:13 +00:00
fix(manager_llm): improve error handling and add debug logging
- Add debug logging for better observability - Add sanitize_agent_name helper method - Enhance error messages with more context - Add parameterized tests for edge cases: - Embedded quotes - Trailing newlines - Multiple whitespace - Case variations - None values - Improve error handling with specific exceptions Co-Authored-By: Joe Moura <joao@crewai.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
import logging
|
||||||
from typing import Optional, Union
|
from typing import Optional, Union
|
||||||
|
|
||||||
from pydantic import Field
|
from pydantic import Field
|
||||||
@@ -8,6 +9,9 @@ from crewai.tools.base_tool import BaseTool
|
|||||||
from crewai.utilities import I18N
|
from crewai.utilities import I18N
|
||||||
|
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class BaseAgentTool(BaseTool):
|
class BaseAgentTool(BaseTool):
|
||||||
"""Base class for agent-related tools"""
|
"""Base class for agent-related tools"""
|
||||||
|
|
||||||
@@ -16,6 +20,20 @@ class BaseAgentTool(BaseTool):
|
|||||||
default_factory=I18N, description="Internationalization settings"
|
default_factory=I18N, description="Internationalization settings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def sanitize_agent_name(self, name: str) -> str:
|
||||||
|
"""
|
||||||
|
Sanitize agent role name by trimming whitespace and setting to lowercase.
|
||||||
|
Removes quotes and newlines for consistent matching.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
name (str): The agent role name to sanitize
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
str: The sanitized agent role name, with whitespace trimmed,
|
||||||
|
converted to lowercase, and quotes/newlines removed
|
||||||
|
"""
|
||||||
|
return name.strip().casefold().replace('"', "").replace("\n", "")
|
||||||
|
|
||||||
def _get_coworker(self, coworker: Optional[str], **kwargs) -> Optional[str]:
|
def _get_coworker(self, coworker: Optional[str], **kwargs) -> Optional[str]:
|
||||||
coworker = coworker or kwargs.get("co_worker") or kwargs.get("coworker")
|
coworker = coworker or kwargs.get("co_worker") or kwargs.get("coworker")
|
||||||
if coworker:
|
if coworker:
|
||||||
@@ -25,11 +43,27 @@ class BaseAgentTool(BaseTool):
|
|||||||
return coworker
|
return coworker
|
||||||
|
|
||||||
def _execute(
|
def _execute(
|
||||||
self, agent_name: Union[str, None], task: str, context: Union[str, None]
|
self,
|
||||||
|
agent_name: Optional[str],
|
||||||
|
task: str,
|
||||||
|
context: Optional[str] = None
|
||||||
) -> str:
|
) -> str:
|
||||||
|
"""
|
||||||
|
Execute delegation to an agent with case-insensitive and whitespace-tolerant matching.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
agent_name: Name/role of the agent to delegate to (case-insensitive)
|
||||||
|
task: The specific question or task to delegate
|
||||||
|
context: Optional additional context for the task execution
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
str: The execution result from the delegated agent or an error message
|
||||||
|
if the agent cannot be found
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
if agent_name is None:
|
if agent_name is None:
|
||||||
agent_name = ""
|
agent_name = ""
|
||||||
|
logger.debug("No agent name provided, using empty string")
|
||||||
|
|
||||||
# It is important to remove the quotes from the agent name.
|
# It is important to remove the quotes from the agent name.
|
||||||
# The reason we have to do this is because less-powerful LLM's
|
# The reason we have to do this is because less-powerful LLM's
|
||||||
@@ -38,31 +72,49 @@ class BaseAgentTool(BaseTool):
|
|||||||
# {"task": "....", "coworker": "....
|
# {"task": "....", "coworker": "....
|
||||||
# when it should look like this:
|
# when it should look like this:
|
||||||
# {"task": "....", "coworker": "...."}
|
# {"task": "....", "coworker": "...."}
|
||||||
agent_name = agent_name.strip().casefold().replace('"', "").replace("\n", "")
|
sanitized_name = self.sanitize_agent_name(agent_name)
|
||||||
|
logger.debug(f"Sanitized agent name from '{agent_name}' to '{sanitized_name}'")
|
||||||
|
|
||||||
|
available_agents = [agent.role for agent in self.agents]
|
||||||
|
logger.debug(f"Available agents: {available_agents}")
|
||||||
|
|
||||||
agent = [ # type: ignore # Incompatible types in assignment (expression has type "list[BaseAgent]", variable has type "str | None")
|
agent = [ # type: ignore # Incompatible types in assignment (expression has type "list[BaseAgent]", variable has type "str | None")
|
||||||
available_agent
|
available_agent
|
||||||
for available_agent in self.agents
|
for available_agent in self.agents
|
||||||
if available_agent.role.strip().casefold().replace("\n", "") == agent_name
|
if self.sanitize_agent_name(available_agent.role) == sanitized_name
|
||||||
]
|
]
|
||||||
except Exception as _:
|
logger.debug(f"Found {len(agent)} matching agents for role '{sanitized_name}'")
|
||||||
|
except (AttributeError, ValueError) as e:
|
||||||
|
# Handle specific exceptions that might occur during role name processing
|
||||||
return self.i18n.errors("agent_tool_unexisting_coworker").format(
|
return self.i18n.errors("agent_tool_unexisting_coworker").format(
|
||||||
coworkers="\n".join(
|
coworkers="\n".join(
|
||||||
[f"- {agent.role.casefold()}" for agent in self.agents]
|
[f"- {self.sanitize_agent_name(agent.role)}" for agent in self.agents]
|
||||||
)
|
),
|
||||||
|
error=str(e)
|
||||||
)
|
)
|
||||||
|
|
||||||
if not agent:
|
if not agent:
|
||||||
|
# No matching agent found after sanitization
|
||||||
return self.i18n.errors("agent_tool_unexisting_coworker").format(
|
return self.i18n.errors("agent_tool_unexisting_coworker").format(
|
||||||
coworkers="\n".join(
|
coworkers="\n".join(
|
||||||
[f"- {agent.role.casefold()}" for agent in self.agents]
|
[f"- {self.sanitize_agent_name(agent.role)}" for agent in self.agents]
|
||||||
)
|
),
|
||||||
|
error=f"No agent found with role '{sanitized_name}'"
|
||||||
)
|
)
|
||||||
|
|
||||||
agent = agent[0]
|
agent = agent[0]
|
||||||
task_with_assigned_agent = Task( # type: ignore # Incompatible types in assignment (expression has type "Task", variable has type "str")
|
try:
|
||||||
description=task,
|
task_with_assigned_agent = Task(
|
||||||
agent=agent,
|
description=task,
|
||||||
expected_output=agent.i18n.slice("manager_request"),
|
agent=agent,
|
||||||
i18n=agent.i18n,
|
expected_output=agent.i18n.slice("manager_request"),
|
||||||
)
|
i18n=agent.i18n,
|
||||||
return agent.execute_task(task_with_assigned_agent, context)
|
)
|
||||||
|
logger.debug(f"Created task for agent '{self.sanitize_agent_name(agent.role)}': {task}")
|
||||||
|
return agent.execute_task(task_with_assigned_agent, context)
|
||||||
|
except Exception as e:
|
||||||
|
# Handle task creation or execution errors
|
||||||
|
return self.i18n.errors("agent_tool_execution_error").format(
|
||||||
|
agent_role=self.sanitize_agent_name(agent.role),
|
||||||
|
error=str(e)
|
||||||
|
)
|
||||||
|
|||||||
52
tests/test_manager_llm_delegation.py
Normal file
52
tests/test_manager_llm_delegation.py
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
import pytest
|
||||||
|
from crewai import Agent, Task
|
||||||
|
from crewai.tools.agent_tools.base_agent_tools import BaseAgentTool
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
class TestAgentTool(BaseAgentTool):
|
||||||
|
"""Concrete implementation of BaseAgentTool for testing."""
|
||||||
|
def _run(self, *args, **kwargs):
|
||||||
|
"""Implement required _run method."""
|
||||||
|
return "Test response"
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("role_name,should_match", [
|
||||||
|
('Futel Official Infopoint', True), # exact match
|
||||||
|
(' "Futel Official Infopoint" ', True), # extra quotes and spaces
|
||||||
|
('Futel Official Infopoint\n', True), # trailing newline
|
||||||
|
('"Futel Official Infopoint"', True), # embedded quotes
|
||||||
|
(' FUTEL\nOFFICIAL INFOPOINT ', True), # multiple whitespace and newline
|
||||||
|
('futel official infopoint', True), # lowercase
|
||||||
|
('FUTEL OFFICIAL INFOPOINT', True), # uppercase
|
||||||
|
('Non Existent Agent', False), # non-existent agent
|
||||||
|
(None, False), # None agent name
|
||||||
|
])
|
||||||
|
def test_agent_tool_role_matching(role_name, should_match):
|
||||||
|
"""Test that agent tools can match roles regardless of case, whitespace, and special characters."""
|
||||||
|
# Create test agent
|
||||||
|
test_agent = Agent(
|
||||||
|
role='Futel Official Infopoint',
|
||||||
|
goal='Answer questions about Futel',
|
||||||
|
backstory='Futel Football Club info',
|
||||||
|
allow_delegation=False
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create test agent tool
|
||||||
|
agent_tool = TestAgentTool(
|
||||||
|
name="test_tool",
|
||||||
|
description="Test tool",
|
||||||
|
agents=[test_agent]
|
||||||
|
)
|
||||||
|
|
||||||
|
# Test role matching
|
||||||
|
result = agent_tool._execute(
|
||||||
|
agent_name=role_name,
|
||||||
|
task='Test task',
|
||||||
|
context=None
|
||||||
|
)
|
||||||
|
|
||||||
|
if should_match:
|
||||||
|
assert "coworker mentioned not found" not in result.lower(), \
|
||||||
|
f"Should find agent with role name: {role_name}"
|
||||||
|
else:
|
||||||
|
assert "coworker mentioned not found" in result.lower(), \
|
||||||
|
f"Should not find agent with role name: {role_name}"
|
||||||
Reference in New Issue
Block a user