sashirestela / simple-openai

A Java library to use the OpenAI Api in the simplest possible way.
MIT License
162 stars 16 forks source link

Inconsistency in the use of Tool and AssistantTool #134

Closed sashirestela closed 5 months ago

sashirestela commented 5 months ago

Discussed in https://github.com/sashirestela/simple-openai/discussions/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: ``` public class AssistantRequest { ... private List tools; ... } ``` While AssistantModifyRequest requires AssistantTool ``` public class AssistantModifyRequest { ... private List 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 commented 5 months ago

I will implement your proposal.