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

backend: Tool config major refactoring #822

Merged
merged 26 commits into from
Nov 8, 2024
Merged

backend: Tool config major refactoring #822

merged 26 commits into from
Nov 8, 2024

Conversation

tianjing-li
Copy link
Collaborator

@tianjing-li tianjing-li commented Oct 28, 2024

Big tool refactor that removes a lot of friction/steps when creating or modifying a tool.

Notably:

  • Change several tool schemas to better reflect their purpose
  • Made tool config definition live in Tool's class
  • Remove the need to maintain dictionary manually, only have to add tool implementation class to the Tool enum
  • Above means that tool ID will need to be fetched with Tool.Google_Drive.value.ID for example
  • Remove config settings to enable tools (in practice, this was not useful and just added a layer of confusion)

AI Description

This PR introduces a new Tool class and a ToolDefinition class, replacing the previous ToolName and ManagedTool classes. The Tool class is an enum that maps to the tool's main implementation class, while the ToolDefinition class provides a definition for each tool, including its name, display name, implementation, parameter definitions, and other attributes.

The PR also updates the get_available_tools function to return a dictionary of ToolDefinition objects, and modifies various files to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.

  • The ToolName class is replaced with the Tool class, which is an enum that maps to the tool's main implementation class.
  • The ManagedTool class is replaced with the ToolDefinition class, which provides a definition for each tool, including its name, display name, implementation, parameter definitions, and other attributes.
  • The get_available_tools function is updated to return a dictionary of ToolDefinition objects.
  • Various files are modified to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/config/configuration.template.yaml file is updated to remove the enabled_tools list and replace it with a dictionary of tool definitions.
  • The src/backend/config/settings.py file is updated to remove the enabled_tools list and replace it with a dictionary of tool definitions.
  • The src/backend/config/tools.py file is updated to remove the ToolName class and replace it with the Tool class, and to remove the ManagedTool class and replace it with the ToolDefinition class.
  • The src/backend/main.py file is updated to import the logging module and set the log level for the pydantic module to ERROR.
  • The src/backend/pytest.ini file is updated to add a filterwarnings line to ignore UserWarnings from the pydantic module.
  • The src/backend/routers/auth.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/routers/organization.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/routers/tool.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/schemas/tool.py file is updated to remove the Category class and replace it with the ToolCategory class, and to remove the ManagedTool class and replace it with the ToolDefinition class.
  • The src/backend/services/auth/strategies/base.py file is updated to add a @classmethod decorator to the _post_init_check method.
  • The src/backend/services/chat.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/services/file.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/services/request_validators.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/tests/integration/routers/test_agent.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/chat/test_tool_calls.py file is updated to use the new Tool and ToolDefinition classes instead of the old ToolName and ManagedTool classes.
  • The src/backend/tests/unit/config/test_tools.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/configuration.yaml file is updated to remove the enabled_tools list and replace it with a dictionary of tool definitions.
  • The src/backend/tests/unit/crud/test_agent.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/crud/test_agent_tool_metadata.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/crud/test_tool_auth.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/factories/agent.py file is updated to use the new Tool class instead of the old ToolName class.
  • The src/backend/tests/unit/factories/agent_tool_metadata.py file is updated to use the new Tool class instead of the old ToolName class.

@codecov-commenter
Copy link

codecov-commenter commented Oct 28, 2024

Codecov Report

Attention: Patch coverage is 87.77293% with 28 lines in your changes missing coverage. Please review.

Project coverage is 79.19%. Comparing base (e0af79f) to head (da0f42f).

Files with missing lines Patch % Lines
src/backend/tools/tavily_search.py 35.71% 9 Missing ⚠️
src/backend/routers/auth.py 16.66% 5 Missing ⚠️
src/backend/tests/unit/routers/test_chat.py 20.00% 4 Missing ⚠️
src/backend/chat/custom/custom.py 75.00% 1 Missing ⚠️
src/backend/cli/main.py 50.00% 1 Missing ⚠️
src/backend/services/auth/strategies/base.py 66.66% 1 Missing ⚠️
src/backend/services/chat.py 50.00% 1 Missing ⚠️
src/backend/services/file.py 0.00% 1 Missing ⚠️
src/backend/services/request_validators.py 75.00% 1 Missing ⚠️
src/backend/tools/base.py 94.44% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #822      +/-   ##
==========================================
+ Coverage   79.14%   79.19%   +0.04%     
==========================================
  Files         250      250              
  Lines       10675    10723      +48     
==========================================
+ Hits         8449     8492      +43     
- Misses       2226     2231       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tianjing-li tianjing-li changed the title WIP backend: Initial tool refactoring - more to come Oct 28, 2024
@tianjing-li tianjing-li changed the title backend: Initial tool refactoring - more to come backend: Tool config improvements - WIP Oct 30, 2024
@tianjing-li tianjing-li changed the title backend: Tool config improvements - WIP backend: Tool config improvements Nov 1, 2024
@tianjing-li tianjing-li changed the title backend: Tool config improvements backend: Tool config major refactoring Nov 4, 2024
Copy link
Collaborator

@EugeneLightsOn EugeneLightsOn left a comment

Choose a reason for hiding this comment

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

Looks good to me, there is just one small thing to change.

src/backend/config/tools.py Outdated Show resolved Hide resolved
@tianjing-li tianjing-li added this pull request to the merge queue Nov 6, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 6, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 6, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 6, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 6, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 6, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 6, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 6, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 7, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 7, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 7, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 7, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 7, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 7, 2024
@tianjing-li tianjing-li added this pull request to the merge queue Nov 8, 2024
@tianjing-li tianjing-li removed this pull request from the merge queue due to a manual request Nov 8, 2024
@tianjing-li tianjing-li merged commit 4b1b8cd into main Nov 8, 2024
7 checks passed
@tianjing-li tianjing-li deleted the tool-refactor branch November 8, 2024 15:18
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