Skip to content

Add docstrings to utility methods in agent.py #3035

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 22 commits into
base: main
Choose a base branch
from

Conversation

dhyeyinf
Copy link

What this PR does

This pull request adds missing docstrings to several utility methods in src/crewai/agent.py to improve code readability and developer understanding.

Updated Methods

  • get_delegation_tools(self, agents)
  • get_multimodal_tools(self)
  • get_code_execution_tools(self)
  • get_output_converter(self, llm, text, model, instructions)

These docstrings follow PEP 257 standards and are aimed at helping contributors and users better understand the codebase.

Why this is important

Improved documentation supports open-source maintainability and makes it easier for new contributors to onboard. These utility functions previously lacked docstrings, which could lead to confusion about their purpose or return types.

Additional Notes

No functionality was changed. The file passes ruff and black formatting checks.

Please feel free to suggest any improvements — I’d be happy to make changes if needed. 🙂

@mplachta
Copy link
Contributor

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

Code Review for crewAIInc/crewAI PR #3035 — Adding Missing Docstrings to Utility Methods in agent.py

Summary of Key Findings

This PR enhances the code quality of src/crewai/agent.py by adding detailed docstrings to several utility methods that previously lacked inline documentation. The added docstrings improve readability, maintainability, and clarify the purpose, inputs, and outputs of the following methods:

  • get_delegation_tools
  • get_multimodal_tools
  • get_code_execution_tools
  • get_output_converter

No changes were made to logic or functionality, so the impact is strictly on improving internal documentation and developer experience.

Code Quality Findings and Specific Improvement Suggestions

1. Docstring Adequacy and Style

  • The docstrings provide clear summaries, argument descriptions, and return values, following a consistent style using triple-quoted strings.
  • They align well with existing type hints, enhancing clarity.
  • Suggest adopting a consistent docstring style throughout the module (e.g., Google style as used here) for uniformity and to support automated documentation tools.
  • Include a one-line summary followed by a blank line before elaborating arguments and return types to comply fully with PEP 257 conventions.

2. Parameter and Return Type Descriptions

  • For get_delegation_tools, consider using Sequence[BaseTool] as the return type for greater flexibility and to match the style seen in other methods.
  • Document potential exceptions or failure modes if instantiating AgentTools can fail or raise exceptions.
  • For get_code_execution_tools, replace List[BaseTool] | None in docstrings with the PEP-compliant Optional[List[BaseTool]].
  • Explicitly mention in get_code_execution_tools that None is returned only if the optional dependency crewai_tools is not installed, helping users understand conditional availability.
  • In get_output_converter, describe llm more explicitly as an interface or class expected, and provide the fully qualified name for Converter to reduce ambiguity for future maintainers.

3. Clarity on Dependency Handling

  • get_code_execution_tools gracefully handles the absence of the crewai_tools dependency by returning None. This pattern is good, but documenting this conditional behavior explicitly in the method docstring is recommended to inform users of optional features.
  • Optionally, consider logging a warning (if not done already) when the dependency is missing to aid debugging.

4. Maintainability and Consistency

  • The added docstrings enable easier onboarding and code navigation.
  • Review related files (AgentTools, AddImageTool, Converter, etc.) for similar documentation consistency.
  • Encourage adopting a uniform docstring standard and applying it throughout the codebase to maximize documentation utility.

Historical Context and Related PR Lessons

  • This PR fits into a common pattern of incremental documentation improvements seen in similar repositories and likely in past crewAI PRs focused on code quality.
  • Prior related PRs typically emphasize aligning docstrings with type hints and describing optional or conditional behaviors clearly—particularly useful for optional dependencies.
  • Lessons from related changes suggest prioritizing consistent style, using standard typing forms (Optional, Sequence), and fully qualifying project-specific classes in docstrings.

Implications for Related Files

  • The AgentTools class used in get_delegation_tools should be reviewed to confirm its documentation and interface stability.
  • AddImageTool in get_multimodal_tools is a singleton example of multimodal support; if extended, docs should be updated accordingly.
  • The converter.py or relevant module where Converter is defined should have well-maintained docstrings describing usage, expected behavior, and integration points.
  • Code execution tooling depends on optional package crewai_tools, so any integration or fallback mechanisms related to this package deserve testing and documentation.

Example of Refined Docstring for get_code_execution_tools

def get_code_execution_tools(self) -> Optional[List[BaseTool]]:
    """
    Retrieve tools to enable code execution capabilities for the agent.

    Returns:
        Optional[List[BaseTool]]: A list of code execution tools if the
        'crewai_tools' package is installed; otherwise, returns None.
    """

Conclusion

This PR positively contributes to the maintainability and clarity of the agent.py module by adding missing documentation to key utility methods involved in multi-agent tooling, multimodal support, code execution, and output conversion. While no functional changes were introduced, the added docstrings lay a stronger foundation for future contributors and facilitate smoother integrations with optional dependencies.

Consider adopting the improvement suggestions above to further refine docstrings and ensure consistent documentation across related components.


If there is interest, the project might benefit from a documentation style guide to standardize docstrings and typing annotations for all agents and tools-related modules going forward. This would streamline future docstring additions and reviews.

Thank you for this valuable documentation enhancement contribution!

@joaomdmoura
Copy link
Collaborator

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

Code Review Comments for PR #3035

Overview

This pull request makes significant improvements to the documentation of utility methods in agent.py, specifically for the following methods:

  • get_delegation_tools()
  • get_multimodal_tools()
  • get_code_execution_tools()
  • get_output_converter()

Positive Aspects

  1. Documentation Enhancements: The addition of docstrings follows the Google-style format and provides a clear understanding of the methods' purposes, parameters, and return types.
  2. Type Hints: The preservation of type hints alongside docstrings ensures better code readability and IDE support.
  3. Clarity on Exceptions: The inclusion of exception information in the docstrings enhances understanding of potential issues during method calls.

Areas for Improvement

While the pull request makes commendable improvements, there are some areas that could be enhanced further:

1. get_delegation_tools()

Improvement Suggestions:

  • Enhance the clarity and specificity of the return type and detail any potential exceptions that may occur.

Example:

def get_delegation_tools(self, agents: List[BaseAgent]) -> List[BaseTool]:
    """
    Retrieve delegation tools configured with the provided agents.

    Args:
        agents (List[BaseAgent]): A list of agents to be used for delegation.

    Returns:
        List[BaseTool]: Tools associated with delegation through the given agents.

    Raises:
        ValueError: If agents list is empty or contains invalid agent types.
    """

2. get_multimodal_tools()

Improvement Suggestions:

  • Add details regarding limitations and dependencies associated with the multimodal tools.

Example:

def get_multimodal_tools(self) -> Sequence[BaseTool]:
    """
    Retrieve tools used for handling multimodal inputs like images.

    Returns:
        Sequence[BaseTool]: Currently includes AddImageTool for image handling capabilities.

    Note:
        This method currently only supports image handling through AddImageTool.
    """

3. get_code_execution_tools()

Improvement Suggestions:

  • Ensure that exception handling is thoroughly documented to elucidate potential failures.

Example:

def get_code_execution_tools(self) -> Optional[List[BaseTool]]:
    """
    Retrieve tools for code execution.

    Returns:
        List[BaseTool] | None: List of code execution tools if available,
        otherwise None if crewai_tools is not installed.

    Raises:
        ImportError: When crewai_tools package is not installed.

    Note:
        Requires crewai_tools package to be installed for functionality.
    """

4. get_output_converter()

Improvement Suggestions:

  • Provide more precise typing for parameters and returns and include usage examples if applicable.

Example:

def get_output_converter(
    self,
    llm: BaseLLM,
    text: str,
    model: str,
    instructions: str
) -> Converter:
    """
    Create a Converter object for processing model outputs.

    Args:
        llm (BaseLLM): The language model instance to be used for conversion.

    Returns:
        Converter: Configured Converter object.
    """

Historical Context from Related PRs

Previous discussions in pull requests regarding documentation have highlighted the necessity for:

  • Consistency in adhering to Google-style docstrings.
  • The practice of adding usage examples for complex methods.

In particular, PRs that have focused on documenting return types and error handling consistently underscore the value this brings to code maintainability and comprehension.

General Recommendations

  1. Continue to ensure all methods have specific and clear documentation with relevant examples.
  2. Standardize the inclusion of "Raises" sections to document potential exceptions in methods.
  3. Enhance the overall usability of documentation by including more comprehensive examples in the docstrings.

Summary

The improvements in this PR are a solid step toward establishing clearer documentation standards within the codebase. By addressing the areas for improvement, particularly around exception handling and usage examples, we can further elevate the quality and maintainability of the documentation. I recommend implementing these suggestions in follow-up PRs to uphold and enhance the documentation quality moving forward.

@dhyeyinf
Copy link
Author

Thank you so much for the detailed and constructive reviews! I’m glad the improvements were helpful.

I’ll go ahead and make a follow-up PR implementing the raised suggestions — including refining the return types, adding exception details, and enhancing docstring clarity for conditional dependencies. That way, this PR can stay clean and focused.

Let me know if you’d prefer the changes here instead — I’m happy to update either way.

@dhyeyinf
Copy link
Author

Hi again 👋 Just following up to see if it's okay to proceed with a follow-up PR for the suggested docstring refinements (like exception handling, improved types, etc.).

Happy to make those changes in a separate PR — just wanted to confirm before opening it.

Thanks again for your time and feedback! 🙏

Copy link
Contributor

@lucasgomide lucasgomide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhyeyinf great changes!

However can you add the type hints also?

@dhyeyinf
Copy link
Author

Thank you so much for the feedback! 🙏
I've now made the requested updates by adding the missing type hints to the three methods.

Please let me know if there’s anything else you'd like me to adjust — happy to make further changes if needed.
Otherwise, if everything looks good, feel free to merge the PR whenever convenient.

@dhyeyinf
Copy link
Author

Thanks again for the review and guidance! 🙌
I've added the missing return None to address the mypy check, and pushed the updated changes.

Let me know if there's anything else you'd like me to improve — happy to iterate further.
Otherwise, it's ready for merge whenever you feel it's good to go. 😊

@dhyeyinf
Copy link
Author

dhyeyinf commented Jul 1, 2025

Hi @lucasgomide 👋 Just following up on this — I’ve addressed the mypy check and all requested changes have been pushed.

Let me know if there's anything else you'd like me to update. Otherwise, if everything looks good, this should be ready for merge. Thanks again! 😊

@lucasgomide
Copy link
Contributor

@dhyeyinf I'm sorry about delay to reply but now you have to resolve some conflicts.

Please tag me when you did it

danielfsbarreto and others added 19 commits July 3, 2025 11:41
…Inc#3030)

Co-authored-by: Tony Kipkemboi <iamtonykipkemboi@gmail.com>
* Added Union of List of Task, None, NotSpecified

* Seems like a flaky test

* Fixed run time issue

* Fixed Linting issues

* fix pydantic error

* aesthetic changes

---------

Co-authored-by: Lucas Gomide <lucaslg200@gmail.com>
…AIInc#3023)

* feat: support to initialize a tool from defined Tool attributes

* fix: ensure Agent is able to load a list of Tools dynamically
* Add Oxylabs tools

* Review updates

* Review updates

---------

Co-authored-by: Tony Kipkemboi <iamtonykipkemboi@gmail.com>
)

* test: add tests to test get_crews

* feat: improve Crew search while resetting their memories

Some memories couldn't be reset due to their reliance on relative external sources like `PDFKnowledge`. This was caused by the need to run the reset memories command from the `src` directory, which could break when external files weren't accessible from that path.

This commit allows the reset command to be executed from the root of the project — the same location typically used to run a crew — improving compatibility and reducing friction.

* feat: skip cli/templates folder while looking for Crew

* refactor: use console.print instead of print
* docs: added Maxim support for Agent Observability

* enhanced the maxim integration doc page as per the github PR reviewer bot suggestions

* Update maxim-observability.mdx

* Update maxim-observability.mdx

- Fixed Python version, >=3.10
- added expected_output field in Task
- Removed marketing links and added github link

* added maxim in observability

* updated the maxim docs page

* fixed image paths

* removed demo link

---------

Co-authored-by: Tony Kipkemboi <iamtonykipkemboi@gmail.com>
Co-authored-by: Lucas Gomide <lucaslg200@gmail.com>
* docs: add pt-br translations

Powered by a CrewAI Flow https://github.com/danielfsbarreto/docs_translator

* Update mcp/overview.mdx brazilian docs

Its en-US counterpart was updated after I did a pass,
so now it includes the new section about @crewbase
…tools per agent (crewAIInc#3065)

* feat: enhance CrewBase MCP tools support to allow selecting multiple tools per agent

* docs: clarify how to access MCP tools

* build: upgrade crewai-tools
This change aims to be more generic, so we don’t have to constantly reflect all available LLM options suggested by the CLI when creating a crew.
Co-authored-by: Lucas Gomide <lucaslg200@gmail.com>
…eation (crewAIInc#3060)

* fix: normalize project names by stripping trailing slashes in crew creation

- Strip trailing slashes from project names in create_folder_structure
- Add comprehensive tests for trailing slash scenarios
- Fixes crewAIInc#3059

The issue occurred because trailing slashes in project names like 'hello/'
were directly incorporated into pyproject.toml, creating invalid package
names and script entries. This fix silently normalizes project names by
stripping trailing slashes before processing, maintaining backward
compatibility while fixing the invalid template generation.

Co-Authored-By: João <joao@crewai.com>

* trigger CI re-run to check for flaky test issue

Co-Authored-By: João <joao@crewai.com>

* fix: resolve circular import in CLI authentication module

- Move ToolCommand import to be local inside _poll_for_token method
- Update test mock to patch ToolCommand at correct location
- Resolves Python 3.11 test collection failure in CI

Co-Authored-By: João <joao@crewai.com>

* feat: add comprehensive class name validation for Python identifiers

- Ensure generated class names are always valid Python identifiers
- Handle edge cases: names starting with numbers, special characters, keywords, built-ins
- Add sanitization logic to remove invalid characters and prefix with 'Crew' when needed
- Add comprehensive test coverage for class name validation edge cases
- Addresses GitHub PR comment from lucasgomide about class name validity

Fixes include:
- Names starting with numbers: '123project' -> 'Crew123Project'
- Python built-ins: 'True' -> 'TrueCrew', 'False' -> 'FalseCrew'
- Special characters: 'hello@world' -> 'HelloWorld'
- Empty/whitespace: '   ' -> 'DefaultCrew'
- All generated class names pass isidentifier() and keyword checks

Co-Authored-By: João <joao@crewai.com>

* refactor: change class name validation to raise errors instead of generating defaults

- Remove default value generation (Crew prefix/suffix, DefaultCrew fallback)
- Raise ValueError with descriptive messages for invalid class names
- Update tests to expect validation errors instead of default corrections
- Addresses GitHub comment feedback from lucasgomide about strict validation

Co-Authored-By: João <joao@crewai.com>

* fix: add working directory safety checks to prevent test interference

Co-Authored-By: João <joao@crewai.com>

* fix: standardize working directory handling in tests to prevent corruption

Co-Authored-By: João <joao@crewai.com>

* fix: eliminate os.chdir() usage in tests to prevent working directory corruption

- Replace os.chdir() with parent_folder parameter for create_folder_structure tests
- Mock create_folder_structure directly for create_crew tests to avoid directory changes
- All 12 tests now pass locally without working directory corruption
- Should resolve the 103 failing tests in Python 3.12 CI

Co-Authored-By: João <joao@crewai.com>

* fix: remove unused os import to resolve lint failure

- Remove unused 'import os' statement from test_create_crew.py
- All tests still pass locally after removing unused import
- Should resolve F401 lint error in CI

Co-Authored-By: João <joao@crewai.com>

* feat: add folder name validation for Python module names

- Implement validation to ensure folder_name is valid Python identifier
- Check that folder names don't start with digits
- Validate folder names are not Python keywords
- Sanitize invalid characters from folder names
- Raise ValueError with descriptive messages for invalid cases
- Update tests to validate both folder and class name requirements
- Addresses GitHub comment requiring folder names to be valid Python module names

Co-Authored-By: João <joao@crewai.com>

* fix: correct folder name validation logic to match test expectations

- Fix validation regex to catch names starting with invalid characters like '@#/'
- Ensure validation properly raises ValueError for cases expected by tests
- Maintain support for valid cases like 'my.project/' -> 'myproject'
- Address lucasgomide's comment about valid Python module names

Co-Authored-By: João <joao@crewai.com>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: João <joao@crewai.com>
Co-authored-by: Lucas Gomide <lucaslg200@gmail.com>
* Adding Nebius to docs

Submitting this PR on behalf of Nebius AI Studio to add Nebius models to the CrewAI documentation.

I tested with the latest CrewAI + Nebius setup to ensure compatibility.

cc @tonykipkemboi

* updated LiteLLM page

---------

Co-authored-by: Lucas Gomide <lucaslg200@gmail.com>
When creating a Crew via the CLI and selecting the Azure provider, the generated .env file had environment variables in lowercase.
This commit ensures that all environment variables are written in uppercase.
StuporHero and others added 2 commits July 3, 2025 11:41
* Updated LiteLLM dependency.

This moves to the latest stable release. Critically, this includes a fix
from BerriAI/litellm#11563 which is required to
use grok-3-mini with crewAI.

* Ran `uv sync` as requested.
@dhyeyinf dhyeyinf force-pushed the docs/add-missing-docstrings branch from a2bd276 to 4ccdc6f Compare July 3, 2025 06:18
@dhyeyinf
Copy link
Author

dhyeyinf commented Jul 3, 2025

Hi @lucasgomide, the branch is now successfully rebased and up to date with main — no conflicts remaining ✅

Let me know if there's anything else you'd like me to adjust. Thanks again for all your help! 🙏

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.