Make Tools own model, add ToolKit Concept #1095
Conversation
- Migrate the Tool abstraction to a separate file - Add a Toolkit abstraction that can own the generation of tools around a shared concept or state Example would be a JSON Toolkit where each tool may define different functions an agent can run against the shared data object. We don't want the Agent to own the data object, and it makes more sense for now to keep the data object as a value that's managed by the tool/skill itself. --------- Co-authored-by: Harrison Chase <[email protected]> Co-authored-by: Francisco Ingham <[email protected]> Co-authored-by: Dhruv Anand <[email protected]> Co-authored-by: cragwolfe <[email protected]> Co-authored-by: Anton Troynikov <[email protected]> Co-authored-by: Oliver Klingefjord <[email protected]> Co-authored-by: William Fu-Hinthorn <[email protected]> Co-authored-by: Ankush Gola <[email protected]>
There was a problem hiding this comment.
my two big comments:
im not sure we need toolkits for google search/bing/wolfram alpha cause they only have one tool. would prefer not to overdo stuff, keep it light and basic until we have more certainty
tools loded in load_tools.py should use the Tool classes, right?
Makefile
Outdated
| @@ -32,9 +32,6 @@ lint: | |||
| test: | |||
| poetry run pytest tests/unit_tests | |||
|
|
|||
| tests: | |||
There was a problem hiding this comment.
@hwchase17 -- can move this to another PR but not sure why there is both make test and make tests
There was a problem hiding this comment.
make test is what CI expects
There was a problem hiding this comment.
someone added this because its more canonical, dont see harm in having both
Makefile
Outdated
| @@ -32,9 +32,6 @@ lint: | |||
| test: | |||
| poetry run pytest tests/unit_tests | |||
|
|
|||
| tests: | |||
There was a problem hiding this comment.
make test is what CI expects
|
|
||
| def run( | ||
| self, | ||
| action: AgentAction, |
There was a problem hiding this comment.
@hinthornw, @hwchase17 --> changed interface to take in AgentAction bc that's what callback manager expects, is backwards compatible
agola11
changed the title
Follow-up of @hinthornw's PR: - Migrate the Tool abstraction to a separate file (`BaseTool`). - `Tool` implementation of `BaseTool` takes in function and coroutine to more easily maintain backwards compatibility - Add a Toolkit abstraction that can own the generation of tools around a shared concept or state --------- Co-authored-by: William FH <[email protected]> Co-authored-by: Harrison Chase <[email protected]> Co-authored-by: Francisco Ingham <[email protected]> Co-authored-by: Dhruv Anand <[email protected]> Co-authored-by: cragwolfe <[email protected]> Co-authored-by: Anton Troynikov <[email protected]> Co-authored-by: Oliver Klingefjord <[email protected]> Co-authored-by: William Fu-Hinthorn <[email protected]> Co-authored-by: Bruno Bornsztein <[email protected]>
Follow-up of @hinthornw's PR:
BaseTool).Toolimplementation ofBaseTooltakes in function and coroutine to more easily maintain backwards compatibility