mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-09 08:08:32 +00:00
Fix lint errors and implement code review suggestions
- Remove unused imports (json, re) - Add validation for reasoning_interval parameter - Use deque for tools_used to prevent memory leaks - Add type hints to all new methods - Refactor adaptive reasoning logic for better readability - Centralize event handling logic - Expand test coverage with parametrized tests Co-Authored-By: Joe Moura <joao@crewai.com>
This commit is contained in:
@@ -2,7 +2,7 @@ import shutil
|
||||
import subprocess
|
||||
from typing import Any, Dict, List, Literal, Optional, Sequence, Type, Union
|
||||
|
||||
from pydantic import Field, InstanceOf, PrivateAttr, model_validator
|
||||
from pydantic import Field, InstanceOf, PrivateAttr, field_validator, model_validator
|
||||
|
||||
from crewai.agents import CacheHandler
|
||||
from crewai.agents.agent_builder.base_agent import BaseAgent
|
||||
@@ -139,6 +139,13 @@ class Agent(BaseAgent):
|
||||
default=None,
|
||||
description="Interval of steps after which the agent should reason again during execution. If None, reasoning only happens before execution.",
|
||||
)
|
||||
|
||||
@field_validator('reasoning_interval')
|
||||
@classmethod
|
||||
def validate_reasoning_interval(cls, v):
|
||||
if v is not None and v < 1:
|
||||
raise ValueError("reasoning_interval must be >= 1")
|
||||
return v
|
||||
adaptive_reasoning: bool = Field(
|
||||
default=False,
|
||||
description="Whether the agent should adaptively decide when to reason during execution based on context.",
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import json
|
||||
import re
|
||||
from collections import deque
|
||||
from typing import Any, Callable, Dict, List, Optional, Union
|
||||
|
||||
from crewai.agents.agent_builder.base_agent import BaseAgent
|
||||
@@ -85,7 +84,7 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
self.tool_name_to_tool_map: Dict[str, Union[CrewStructuredTool, BaseTool]] = {
|
||||
tool.name: tool for tool in self.tools
|
||||
}
|
||||
self.tools_used: List[str] = []
|
||||
self.tools_used: deque = deque(maxlen=100) # Limit history size
|
||||
self.steps_since_reasoning = 0
|
||||
existing_stop = self.llm.stop or []
|
||||
self.llm.stop = list(
|
||||
@@ -490,13 +489,24 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
Returns:
|
||||
bool: True if adaptive reasoning should be triggered, False otherwise.
|
||||
"""
|
||||
|
||||
if len(set(self.tools_used[-3:])) > 1 and len(self.tools_used) >= 3:
|
||||
return True
|
||||
|
||||
if self.iterations > self.max_iter // 2:
|
||||
return True
|
||||
|
||||
return (
|
||||
self._has_used_multiple_tools_recently() or
|
||||
self._is_taking_too_long() or
|
||||
self._has_recent_errors()
|
||||
)
|
||||
|
||||
def _has_used_multiple_tools_recently(self) -> bool:
|
||||
"""Check if multiple different tools were used in recent steps."""
|
||||
if len(self.tools_used) < 3:
|
||||
return False
|
||||
return len(set(list(self.tools_used)[-3:])) > 1
|
||||
|
||||
def _is_taking_too_long(self) -> bool:
|
||||
"""Check if iterations exceed expected duration."""
|
||||
return self.iterations > self.max_iter // 2
|
||||
|
||||
def _has_recent_errors(self) -> bool:
|
||||
"""Check for error indicators in recent messages."""
|
||||
error_indicators = ["error", "exception", "failed", "unable to", "couldn't"]
|
||||
recent_messages = self.messages[-3:] if len(self.messages) >= 3 else self.messages
|
||||
|
||||
@@ -504,7 +514,6 @@ class CrewAgentExecutor(CrewAgentExecutorMixin):
|
||||
content = message.get("content", "").lower()
|
||||
if any(indicator in content for indicator in error_indicators):
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
def _handle_mid_execution_reasoning(self) -> None:
|
||||
|
||||
@@ -386,6 +386,26 @@ class AgentReasoning:
|
||||
)
|
||||
return self.handle_agent_reasoning()
|
||||
|
||||
def _emit_reasoning_event(self, event_class, **kwargs):
|
||||
"""Centralized method for emitting reasoning events."""
|
||||
try:
|
||||
reasoning_trigger = "interval"
|
||||
if hasattr(self.agent, 'adaptive_reasoning') and self.agent.adaptive_reasoning:
|
||||
reasoning_trigger = "adaptive"
|
||||
|
||||
crewai_event_bus.emit(
|
||||
self.agent,
|
||||
event_class(
|
||||
agent_role=self.agent.role,
|
||||
task_id=str(self.task.id),
|
||||
reasoning_trigger=reasoning_trigger,
|
||||
**kwargs
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
# Ignore event bus errors to avoid breaking execution
|
||||
pass
|
||||
|
||||
def handle_mid_execution_reasoning(
|
||||
self,
|
||||
current_steps: int,
|
||||
@@ -405,69 +425,37 @@ class AgentReasoning:
|
||||
Returns:
|
||||
AgentReasoningOutput: Updated reasoning plan based on current context
|
||||
"""
|
||||
try:
|
||||
from crewai.utilities.events.reasoning_events import AgentMidExecutionReasoningStartedEvent
|
||||
|
||||
reasoning_trigger = "interval"
|
||||
if self.agent.adaptive_reasoning:
|
||||
reasoning_trigger = "adaptive"
|
||||
|
||||
crewai_event_bus.emit(
|
||||
self.agent,
|
||||
AgentMidExecutionReasoningStartedEvent(
|
||||
agent_role=self.agent.role,
|
||||
task_id=str(self.task.id),
|
||||
current_step=current_steps,
|
||||
reasoning_trigger=reasoning_trigger,
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
# Ignore event bus errors to avoid breaking execution
|
||||
pass
|
||||
from crewai.utilities.events.reasoning_events import AgentMidExecutionReasoningStartedEvent
|
||||
|
||||
self._emit_reasoning_event(
|
||||
AgentMidExecutionReasoningStartedEvent,
|
||||
current_step=current_steps
|
||||
)
|
||||
|
||||
try:
|
||||
output = self.__handle_mid_execution_reasoning(
|
||||
current_steps, tools_used, current_progress, iteration_messages
|
||||
)
|
||||
|
||||
# Emit reasoning completed event
|
||||
try:
|
||||
from crewai.utilities.events.reasoning_events import AgentMidExecutionReasoningCompletedEvent
|
||||
|
||||
reasoning_trigger = "interval"
|
||||
if self.agent.adaptive_reasoning:
|
||||
reasoning_trigger = "adaptive"
|
||||
|
||||
crewai_event_bus.emit(
|
||||
self.agent,
|
||||
AgentMidExecutionReasoningCompletedEvent(
|
||||
agent_role=self.agent.role,
|
||||
task_id=str(self.task.id),
|
||||
current_step=current_steps,
|
||||
updated_plan=output.plan.plan,
|
||||
reasoning_trigger=reasoning_trigger,
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
# Emit completed event
|
||||
from crewai.utilities.events.reasoning_events import AgentMidExecutionReasoningCompletedEvent
|
||||
|
||||
self._emit_reasoning_event(
|
||||
AgentMidExecutionReasoningCompletedEvent,
|
||||
current_step=current_steps,
|
||||
updated_plan=output.plan.plan
|
||||
)
|
||||
|
||||
return output
|
||||
except Exception as e:
|
||||
# Emit reasoning failed event
|
||||
try:
|
||||
from crewai.utilities.events.reasoning_events import AgentReasoningFailedEvent
|
||||
|
||||
crewai_event_bus.emit(
|
||||
self.agent,
|
||||
AgentReasoningFailedEvent(
|
||||
agent_role=self.agent.role,
|
||||
task_id=str(self.task.id),
|
||||
error=str(e),
|
||||
attempt=1,
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
# Emit failed event
|
||||
from crewai.utilities.events.reasoning_events import AgentReasoningFailedEvent
|
||||
|
||||
self._emit_reasoning_event(
|
||||
AgentReasoningFailedEvent,
|
||||
error=str(e),
|
||||
attempt=1
|
||||
)
|
||||
|
||||
raise
|
||||
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
"""Tests for reasoning interval and adaptive reasoning in agents."""
|
||||
|
||||
import json
|
||||
import pytest
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
@@ -211,3 +210,35 @@ def test_should_trigger_adaptive_reasoning():
|
||||
{"role": "assistant", "content": "Let me continue with the next step."}
|
||||
]
|
||||
assert executor._should_adaptive_reason() is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize("interval,steps,should_reason", [
|
||||
(None, 5, False),
|
||||
(3, 2, False),
|
||||
(3, 3, True),
|
||||
(1, 1, True),
|
||||
(5, 10, True),
|
||||
])
|
||||
def test_reasoning_interval_scenarios(interval, steps, should_reason):
|
||||
"""Test various reasoning interval scenarios."""
|
||||
agent = MagicMock()
|
||||
agent.reasoning = True
|
||||
agent.reasoning_interval = interval
|
||||
agent.adaptive_reasoning = False
|
||||
|
||||
executor = CrewAgentExecutor(
|
||||
llm=MagicMock(),
|
||||
task=MagicMock(),
|
||||
crew=MagicMock(),
|
||||
agent=agent,
|
||||
prompt={},
|
||||
max_iter=10,
|
||||
tools=[],
|
||||
tools_names="",
|
||||
stop_words=[],
|
||||
tools_description="",
|
||||
tools_handler=MagicMock()
|
||||
)
|
||||
|
||||
executor.steps_since_reasoning = steps
|
||||
assert executor._should_trigger_reasoning() is should_reason
|
||||
|
||||
Reference in New Issue
Block a user