Skip to content

fix: possible fix for Thinking stuck #2996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pythonbyte
Copy link
Collaborator

No description provided.

@pythonbyte pythonbyte requested a review from lucasgomide June 11, 2025 16:58
@joaomdmoura
Copy link
Collaborator

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

Code Review for PR #2996: "fix: possible fix for Thinking stuck"

Overview

This pull request comprehensively addresses issues with the "Thinking" status display in the console formatter. The primary focus is on improving how LLM call events are handled and visually represented in the console tree, which is crucial for maintaining user engagement and understanding during processing times.

Modified Files

  1. src/crewai/utilities/events/event_listener.py
  2. src/crewai/utilities/events/utils/console_formatter.py

Detailed Analysis & Suggestions for Improvement

1. event_listener.py Observations

Improvements

  • The recent changes introduce proper tracking of the thinking branch state, which ensures that user interactions reflect ongoing processing accurately.
  • Enhanced error handling mechanisms that improve robustness during LLM call events.

Issues Explained

  • Potential Memory Leak: The handling of the thinking_branch could lead to references not being cleaned up during error scenarios, which may result in increased memory usage over time.

  • Missing Type Hints: The new variable thinking_branch lacks type hints which are essential for maintaining code clarity and promoting type safety across the codebase.

Suggested Code Improvement

def on_llm_call_started(source, event: LLMCallStartedEvent) -> None:
    thinking_branch: Optional[Tree] = self.formatter.handle_llm_call_started(
        self.formatter.current_agent_branch,
        self.formatter.current_crew_tree,
    )
    if thinking_branch is not None:
        self.formatter.current_tool_branch = thinking_branch

2. console_formatter.py Observations

Improvements

  • The code now robustly handles thinking status nodes, preventing the creation of duplicate nodes, which can confuse users.

Identified Issues

  • Complex Conditional Logic: The logic to determine if a "Thinking" node should be added could be simplified, making it easier to read and maintain.

  • Code Duplication: The repeated search logic across methods like handle_llm_call_completed and handle_llm_call_failed points to a need for refactoring to improve maintainability.

Suggested Code Improvements

  1. Simplified Thinking Status Check
def _is_thinking_node(branch: Optional[Tree]) -> bool:
    return branch is not None and "Thinking" in str(branch.label)

should_add_thinking = not _is_thinking_node(self.current_tool_branch)
  1. Extracted Common Search Logic
def _find_thinking_branch(self, parents: List[Optional[Tree]]) -> Optional[Tree]:
    for parent in parents:
        if not isinstance(parent, Tree):
            continue
        for child in parent.children:
            if self._is_thinking_node(child):
                return child
    return None
  1. Improved Error Handling
def handle_llm_call_failed(
    self,
    tool_branch: Optional[Tree],
    crew_tree: Optional[Tree],
) -> None:
    if not self.verbose:
        return

    tree_to_use = self.current_crew_tree or crew_tree or self.current_task_branch
    if tree_to_use is None:
        logger.warning("No valid tree found for LLM failure display")
        return

    thinking_branch_to_update = (
        tool_branch if self._is_thinking_node(tool_branch)
        else self._find_thinking_branch(self._get_parent_branches())
    )
    
    if thinking_branch_to_update:
        self._update_failure_status(thinking_branch_to_update)
        self._render_tree(tree_to_use)

General Recommendations

  1. Add Unit Tests: Unit tests covering the new thinking branch handling logic should be implemented alongside cases that test the edge scenarios within tree structures.

  2. Documentation: It’s essential to document all new methods and clarify the lifecycle of the thinking node, supporting maintainability for future developers.

  3. Code Organization: Consider isolating tree manipulation logic in a dedicated helper class or module to clarify intent and reduce coupling within the current classes.

  4. Performance Optimization: Investigate caching strategies for parent branch searches and optimize tree traversal operations to enhance performance, especially in deep/nested structures.

Summary of Benefits

  • The changes enhance reliability when displaying "Thinking" status, improving user experience significantly.
  • Error state management is improved, reducing potential disruptions during LLM call failures.
  • Code maintainability is enhanced by reducing duplication and improving structure through abstraction.

The proposed changes present a positive step towards improving the code quality and user experience. By addressing the suggested improvements, the overall robustness and maintainability of the system could be greatly enhanced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants