Inconsistency in the use of Tool and AssistantTool

[sashirestela]

@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).

[sashirestela]

@the-gigi Thanks for detecting that inconsistency between AssistantRequest and AssistantModifyRequest classes.

In fact, both classes should be using the AssistantTool. I understand your concern, but I prefer not to impact many parts (thirteen classes) of the library, at least not for now. Moreover, I think it is more intuitive to use all the available tools (code interpreter, file search and functions) from the same place, AssistantTool in this case.

In my change proposal, I just need to impact two classes:

• Move the static method functions() from Tool to AssistantTool.
• Overload the static method function() on AssistantTool to cast appropriately.

[the-gigi]

Sure. That's another way to do it. I still think it's more in the spirit of the recent refactoring to use the common Tool everywhere. This will also be useful if you keep a list of Tools that you want to use both for chat requests and assistant request. With the separation you propose users will need to convert from one to another or keep two list of Tool and AssistantTool with the same underlying functions.

[hlzhangxt]

Yeah, it's a problem.

[sashirestela]

A new issue #134 has been created from this discussion.