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

Add validation on agent instantiation for multi-input tools #3681

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

vowelparrot
Copy link
Contributor

@vowelparrot vowelparrot commented Apr 27, 2023

Tradeoffs here:

  • No lint-time checking for compatibility
  • Differs from JS package
  • The signature inference, etc. in the base tool isn't simple
  • The args_schema is optional

Pros:

  • Forwards compatibility retained
  • Doesn't break backwards compatibility
  • User doesn't have to think about which class to subclass (single base tool or dynamic Tool interface regardless of input)
  • No need to change the load_tools, etc. interfaces

@vowelparrot vowelparrot force-pushed the vwp/single_input_idea branch 2 times, most recently from c07695e to 18afc9c Compare April 27, 2023 22:14
@vowelparrot vowelparrot requested review from hwchase17 and agola11 and removed request for hwchase17 April 27, 2023 22:19
Reuse existing tool implementation but add validation on agent
instantiation
@vowelparrot vowelparrot force-pushed the vwp/single_input_idea branch from 18afc9c to 6542b3b Compare April 27, 2023 22:32
@vowelparrot vowelparrot merged commit 4654c58 into master Apr 27, 2023
@vowelparrot vowelparrot deleted the vwp/single_input_idea branch April 27, 2023 22:36
vowelparrot added a commit that referenced this pull request Apr 27, 2023
Tradeoffs here:
- No lint-time checking for compatibility
- Differs from JS package
- The signature inference, etc. in the base tool isn't simple
- The `args_schema` is optional

Pros:
- Forwards compatibility retained
- Doesn't break backwards compatibility
- User doesn't have to think about which class to subclass (single base
tool or dynamic `Tool` interface regardless of input)
-  No need to change the load_tools, etc. interfaces

Co-authored-by: Hasan Patel <[email protected]>
@eavanvalkenburg
Copy link
Contributor

This broke the powerbi toolkit...

vowelparrot added a commit that referenced this pull request Apr 28, 2023
Tradeoffs here:
- No lint-time checking for compatibility
- Differs from JS package
- The signature inference, etc. in the base tool isn't simple
- The `args_schema` is optional

Pros:
- Forwards compatibility retained
- Doesn't break backwards compatibility
- User doesn't have to think about which class to subclass (single base
tool or dynamic `Tool` interface regardless of input)
-  No need to change the load_tools, etc. interfaces

Co-authored-by: Hasan Patel <[email protected]>
vowelparrot added a commit that referenced this pull request Apr 28, 2023
Tradeoffs here:
- No lint-time checking for compatibility
- Differs from JS package
- The signature inference, etc. in the base tool isn't simple
- The `args_schema` is optional 

Pros:
- Forwards compatibility retained
- Doesn't break backwards compatibility
- User doesn't have to think about which class to subclass (single base
tool or dynamic `Tool` interface regardless of input)
-  No need to change the load_tools, etc. interfaces

Co-authored-by: Hasan Patel <[email protected]>
samching pushed a commit to samching/langchain that referenced this pull request May 1, 2023
…n-ai#3681)

Tradeoffs here:
- No lint-time checking for compatibility
- Differs from JS package
- The signature inference, etc. in the base tool isn't simple
- The `args_schema` is optional 

Pros:
- Forwards compatibility retained
- Doesn't break backwards compatibility
- User doesn't have to think about which class to subclass (single base
tool or dynamic `Tool` interface regardless of input)
-  No need to change the load_tools, etc. interfaces

Co-authored-by: Hasan Patel <[email protected]>
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.

4 participants