You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Originally posted by sashirestela May 15, 2024 @the-gigi I've copied your comment to this new discussion thread:
One more issue came up as a result of the refactoring. There is inconsistency in the use of Tool and AssistantTool.
For example, AssistantRequest requires Tool:
public class AssistantRequest {
...
private List<Tool> tools;
...
}
While AssistantModifyRequest requires AssistantTool
public class AssistantModifyRequest {
...
private List<AssistantTool> tools;
...
}
AssistantTool extends Tool. Looking at the code it seems that all AssistantTool adds is the pre-defined tools CODE_INTERPRETER and FILE_SEARCH. Is it necessary to actually instantiate AssistantTool objects?
The idea of the refactor to a common tool is that the Tool functionality is the same between chat and assistants. The fact that assistants have pre-defined tool doesn't require a new type of AssstantTool.
Consider replacing all references to AssistantTool with just Tool. The AssistantTool class itself will still provide the CODE_INTEPRETER and FILE_SEARCH tools, but it will provide them as Tool instances.
This will simplify the code, make it consistent and utilize just the common Tool for all use cases of chat and assistant APIs.
I tried it locally and it and it works just fine (all the tests pass and all the demos).
The text was updated successfully, but these errors were encountered:
Discussed in #131
Originally posted by sashirestela May 15, 2024
@the-gigi I've copied your comment to this new discussion thread:
One more issue came up as a result of the refactoring. There is inconsistency in the use of Tool and AssistantTool.
For example, AssistantRequest requires Tool:
While AssistantModifyRequest requires AssistantTool
AssistantTool extends Tool. Looking at the code it seems that all AssistantTool adds is the pre-defined tools CODE_INTERPRETER and FILE_SEARCH. Is it necessary to actually instantiate AssistantTool objects?
The idea of the refactor to a common tool is that the Tool functionality is the same between chat and assistants. The fact that assistants have pre-defined tool doesn't require a new type of AssstantTool.
Consider replacing all references to AssistantTool with just Tool. The AssistantTool class itself will still provide the CODE_INTEPRETER and FILE_SEARCH tools, but it will provide them as Tool instances.
This will simplify the code, make it consistent and utilize just the common Tool for all use cases of chat and assistant APIs.
I tried it locally and it and it works just fine (all the tests pass and all the demos).
The text was updated successfully, but these errors were encountered: