Skip to content
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

refactor: Move BaseTool to main package and centralize tool description generation #1514

Merged
merged 16 commits into from
Nov 1, 2024

Conversation

c0dezli
Copy link
Contributor

@c0dezli c0dezli commented Oct 26, 2024

Description:

Problem

Currently, we have:

  1. The BaseTool definition in crewai-tools package, which should focus on tool implementations rather than core definitions
  2. Duplicated tool description generation logic across three locations:
    • agent.py: _render_text_description_and_args()
    • tool_usage.py: _render()
    • base_tool.py: _generate_description()

Proposed Changes

  1. Move BaseTool class from crewai-tools to the main crewai package

    • This establishes crewai as the source of core tool abstractions
    • Allows crewai-tools to focus purely on tool implementations
  2. Create a centralized tool description generator in the moved base_tool.py:

    def _generate_description(self):
        args_schema = {
            name: {
                "description": field.description,
                "type": BaseTool._get_arg_annotations(field.annotation),
            }
            for name, field in self.args_schema.model_fields.items()
        }

        description = self.description.replace("\n", " ")
        self.description = f"Tool Name: {self.name}\nTool Arguments: {args_schema}\nTool Description: {description}"

    @staticmethod
    def _get_arg_annotations(annotation: type[Any] | None) -> str:
        origin = get_origin(annotation)
        args = get_args(annotation)

        if origin is None:
            return (
                annotation.__name__
                if hasattr(annotation, "__name__")
                else str(annotation)
            )

        if args:
            args_str = ", ".join(BaseTool._get_arg_annotations(arg) for arg in args)
            return f"{origin.__name__}[{args_str}]"

        return origin.__name__
  1. Update all existing implementations to use this centralized function
    • Replace _render_text_description_and_args() in agent.py
    • Replace _render() in tool_usage.py
    • Remove duplicated code from crewai-tools

Benefits

  • Proper separation of concerns between packages
    • crewai: core abstractions and interfaces
    • crewai-tools: implementations and utilities
  • Single source of truth for tool descriptions
  • Improved maintainability
  • Consistent tool description format across the system

Breaking Changes

  • Projects directly importing BaseTool from crewai-tools will need to update their imports
  • Will require a major version bump for crewai-tools

Migration Guide

For users importing from crewai-tools:

# Old
from crewai_tools import BaseTool

# New
from crewai.tools import BaseTool

Related Issues: #1511

@theCyberTech theCyberTech added the Under review Feature requests under review label Oct 26, 2024
@bhancockio bhancockio self-assigned this Oct 28, 2024
@classmethod
def from_langchain(cls, tool: StructuredTool) -> "BaseTool":
if cls == Tool:
return cls(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was getting some errors with func not existing on cls.

Let's change this to something like this unless you have another suggestion:

 @classmethod
  def from_langchain(cls, tool: StructuredTool) -> "BaseTool":
      if cls == Tool:
          if tool.func is None:
              raise ValueError("StructuredTool must have a callable 'func'")
          return Tool(
              name=tool.name,
              description=tool.description,
              args_schema=tool.args_schema,
              func=tool.func,
          )
      raise NotImplementedError(f"from_langchain not implemented for {cls.__name__}")
      ```

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an amazing PR @c0dezli !!!

Thank you so much. There was only one linting on the from_langchain function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sweet! just made the change. thanks for reviewing!

@c0dezli
Copy link
Contributor Author

c0dezli commented Oct 29, 2024

PR for crewAI-tools linked here: crewAIInc/crewAI-tools#119

@bhancockio bhancockio merged commit e66a135 into crewAIInc:main Nov 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Under review Feature requests under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants