From bca79fa4858959a02f5a752597f5c0ccce96d839 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 17 Mar 2025 20:24:15 +0000 Subject: [PATCH] Implement PR review suggestions: improve type hints, documentation, and tests Co-Authored-By: Joe Moura --- src/crewai/task.py | 16 ++- src/crewai/tasks/conditional_task.py | 8 +- tests/test_conditional_task_copy.py | 143 ++++++++++++++------------- 3 files changed, 96 insertions(+), 71 deletions(-) diff --git a/src/crewai/task.py b/src/crewai/task.py index cb697953e..5733aaec7 100644 --- a/src/crewai/task.py +++ b/src/crewai/task.py @@ -33,6 +33,9 @@ from pydantic_core import PydanticCustomError from crewai.agents.agent_builder.base_agent import BaseAgent from crewai.security import Fingerprint, SecurityConfig +from typing import TYPE_CHECKING +if TYPE_CHECKING: + from crewai.tasks.conditional_task import ConditionalTask from crewai.tasks.guardrail_result import GuardrailResult from crewai.tasks.output_format import OutputFormat from crewai.tasks.task_output import TaskOutput @@ -617,8 +620,17 @@ class Task(BaseModel): def copy( self, agents: List["BaseAgent"], task_mapping: Dict[str, "Task"] - ) -> "Task": - """Create a deep copy of the Task.""" + ) -> Union["Task", "ConditionalTask"]: + """ + Creates a deep copy of the task while preserving its specific type (Task or ConditionalTask). + + Args: + agents: List of agents to search for the agent by role + task_mapping: Dictionary mapping task keys to tasks + + Returns: + Union[Task, ConditionalTask]: A copy of the task maintaining its original type. + """ exclude = { "id", "agent", diff --git a/src/crewai/tasks/conditional_task.py b/src/crewai/tasks/conditional_task.py index d2edee12f..083072160 100644 --- a/src/crewai/tasks/conditional_task.py +++ b/src/crewai/tasks/conditional_task.py @@ -1,8 +1,12 @@ -from typing import Any, Callable +from typing import Any, Callable, TYPE_CHECKING from pydantic import Field -from crewai.task import Task +if TYPE_CHECKING: + from crewai.task import Task +else: + # Import the base class at runtime + from crewai.task import Task from crewai.tasks.output_format import OutputFormat from crewai.tasks.task_output import TaskOutput diff --git a/tests/test_conditional_task_copy.py b/tests/test_conditional_task_copy.py index f72faa785..46727d3d7 100644 --- a/tests/test_conditional_task_copy.py +++ b/tests/test_conditional_task_copy.py @@ -5,33 +5,94 @@ from crewai.tasks.conditional_task import ConditionalTask from crewai.tasks.task_output import TaskOutput -def test_conditional_task_preserved_in_copy(): - """Test that ConditionalTask objects are preserved when copying a Crew.""" - agent = Agent( +@pytest.fixture +def test_agent(): + """Fixture for creating a test agent.""" + return Agent( role="Researcher", goal="Research topics", backstory="You are a researcher." ) - - # Create a regular task - task1 = Task( + +@pytest.fixture +def test_task(test_agent): + """Fixture for creating a regular task.""" + return Task( description="Research topic A", expected_output="Research results for topic A", - agent=agent + agent=test_agent ) - - # Create a conditional task - conditional_task = ConditionalTask( + +@pytest.fixture +def test_conditional_task(test_agent): + """Fixture for creating a conditional task.""" + return ConditionalTask( description="Research topic B if topic A was successful", expected_output="Research results for topic B", - agent=agent, + agent=test_agent, condition=lambda output: "success" in output.raw.lower() ) + +@pytest.fixture +def test_crew(test_agent, test_task, test_conditional_task): + """Fixture for creating a crew with both regular and conditional tasks.""" + return Crew( + agents=[test_agent], + tasks=[test_task, test_conditional_task] + ) + + +def test_conditional_task_preserved_in_copy(test_crew): + """Test that ConditionalTask objects are preserved when copying a Crew.""" + # Create a copy of the crew + crew_copy = test_crew.copy() - # Create a crew with both tasks + # Check that the conditional task is still a ConditionalTask in the copied crew + assert isinstance(crew_copy.tasks[1], ConditionalTask) + assert hasattr(crew_copy.tasks[1], "should_execute") + +def test_conditional_task_preserved_in_kickoff_for_each(test_crew, test_agent): + """Test that ConditionalTask objects are preserved when using kickoff_for_each.""" + from unittest.mock import patch + + # Mock the kickoff method to avoid actual execution + with patch.object(Crew, "kickoff") as mock_kickoff: + # Set up the mock to return a TaskOutput + mock_output = TaskOutput( + description="Mock task output", + raw="Success with topic", + agent=test_agent.role + ) + mock_kickoff.return_value = mock_output + + # Call kickoff_for_each with test inputs + inputs = [{"topic": "test1"}, {"topic": "test2"}] + test_crew.kickoff_for_each(inputs=inputs) + + # Verify the mock was called with the expected inputs + assert mock_kickoff.call_count == len(inputs) + + # Create a copy of the crew to verify the type preservation + # (since we can't directly access the crews created inside kickoff_for_each) + crew_copy = test_crew.copy() + assert isinstance(crew_copy.tasks[1], ConditionalTask) + + +def test_conditional_task_copy_with_none_values(test_agent, test_task): + """Test that ConditionalTask objects are preserved when copying with optional fields.""" + # Create a conditional task with optional fields + conditional_task = ConditionalTask( + description="Research topic B if topic A was successful", + expected_output="Research results for topic B", # Required field + agent=test_agent, + condition=lambda output: "success" in output.raw.lower(), + context=None # Optional field that can be None + ) + + # Create a crew with both a regular task and the conditional task crew = Crew( - agents=[agent], - tasks=[task1, conditional_task] + agents=[test_agent], + tasks=[test_task, conditional_task] ) # Create a copy of the crew @@ -40,56 +101,4 @@ def test_conditional_task_preserved_in_copy(): # Check that the conditional task is still a ConditionalTask in the copied crew assert isinstance(crew_copy.tasks[1], ConditionalTask) assert hasattr(crew_copy.tasks[1], "should_execute") - -def test_conditional_task_preserved_in_kickoff_for_each(): - """Test that ConditionalTask objects are preserved when using kickoff_for_each.""" - from unittest.mock import patch - - agent = Agent( - role="Researcher", - goal="Research topics", - backstory="You are a researcher." - ) - - # Create a regular task - task1 = Task( - description="Research topic A", - expected_output="Research results for topic A", - agent=agent - ) - - # Create a conditional task - conditional_task = ConditionalTask( - description="Research topic B if topic A was successful", - expected_output="Research results for topic B", - agent=agent, - condition=lambda output: "success" in output.raw.lower() - ) - - # Create a crew with both tasks - crew = Crew( - agents=[agent], - tasks=[task1, conditional_task] - ) - - # Mock the kickoff method to avoid actual execution - with patch.object(Crew, "kickoff") as mock_kickoff: - # Set up the mock to return a TaskOutput - mock_output = TaskOutput( - description="Mock task output", - raw="Success with topic", - agent=agent.role - ) - mock_kickoff.return_value = mock_output - - # Call kickoff_for_each with test inputs - inputs = [{"topic": "test1"}, {"topic": "test2"}] - crew.kickoff_for_each(inputs=inputs) - - # Verify the mock was called with the expected inputs - assert mock_kickoff.call_count == len(inputs) - - # Create a copy of the crew to verify the type preservation - # (since we can't directly access the crews created inside kickoff_for_each) - crew_copy = crew.copy() - assert isinstance(crew_copy.tasks[1], ConditionalTask) + assert crew_copy.tasks[1].context is None # Verify None value is preserved