feat: [OpenAI] Tool Definition and Call Parsing#412
Conversation
…quest with tool handling
…nai/tool-definition-and-call-parsing
There was a problem hiding this comment.
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Files not reviewed (1)
- foundation-models/openai/pom.xml: Language not supported
Comments suppressed due to low confidence (2)
sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java:100
- The new functionality for defining and using OpenAiFunctionTool is introduced here, but there are no corresponding tests to verify its integration into the chat completion request. Please add test cases that exercise this functionality.
final var openAiTool = new OpenAiFunctionTool("weather", WeatherMethod.Request.class)
foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionRequest.java:308
- [nitpick] The Javadoc comment includes extraneous text ('tool.getClass().getName()); }') that should be removed to improve clarity and professionalism.
tool.getClass().getName()); } Converts the request to a generated model class CreateChatCompletionRequest.
…verage for OpenAiFunctionCall and OpenAiFunctionTool
…ve redundant methods from OpenAiUtils
…sts for argument methods
…nhance type safety - OpenAiFunctionTool getter package private - introduce getArgumentsAsObject(OpenAiFunctionTool)
| // 3. Execute the function | ||
| final OpenAiToolCall toolCall = assistantMessage.toolCalls().get(0); | ||
| if (!(toolCall instanceof OpenAiFunctionCall functionCall)) { | ||
| throw new IllegalArgumentException( | ||
| "Expected a function call, but got: %s".formatted(assistantMessage)); | ||
| } | ||
| final WeatherMethod.Request arguments = functionCall.getArgumentsAsObject(weatherFunction); | ||
| final WeatherMethod.Response currentWeather = WeatherMethod.getCurrentWeather(arguments); |
There was a problem hiding this comment.
You could put all of this into a convenience function of OpenAiChatCompletionResponse.
Because the client already has the reference to the OpenAiFunctionTool
| // Send back the results, and the model will incorporate them into its final response. | ||
| // 4. Send back the results, and the model will incorporate them into its final response. | ||
| messages.add(assistantMessage); | ||
| messages.add(OpenAiMessage.tool(currentWeather.toString(), functionCall.getId())); |
There was a problem hiding this comment.
OpenAiMessage.tool(currentWeather.toString(), functionCall.getId()) could also be a convenience so th customer doesn't have to build it.
| // 1. Define the function | ||
| final var weatherFunction = | ||
| new OpenAiFunctionTool("weather", WeatherMethod.Request.class) | ||
| .withDescription("Get the weather for the given location"); |
There was a problem hiding this comment.
The is no use-case for OpenAiFunctionTool to be immutable.
Also, is there ever going to be another OpenAiTool that is not OpenAiFunctionTool?
…nai/tool-definition-and-call-parsing
rpanackal
force-pushed
the
feat/openai/tool-definition-and-call-parsing
branch
from
36b0189 to
88aa9de
Compare
|
closed in favor of #418 |
Context
AI/ai-sdk-java-backlog#214.
Feature scope:
OpenAiFunctionToolOpenAiFunctionCall-getArgumentsAsObject()and more...withOpenAiToolsDefinition of Done
Aligned changes with the JavaScript SDK