feat: [OpenAI] Full Tool call Convenience by rpanackal · Pull Request #396 · SAP/ai-sdk-java

rpanackal · 2025-03-26T15:34:02Z

Context

AI/ai-sdk-java-backlog#214.

Previously, ~70 lines of code needed to send tool message to OpenAi models.

Non-nullability guarantee of `OpenAiAssistantMessage().content()`

Motivation
According to the spec, content in the LLM response is nullable. But our messaging convenience promise non-nullability.
The spec file contains a comment that informs content maybe null when tool calls are not (no mutual exclusivity).
Proposal: ~~Abstract away toolCalls as a content item~~ content will be OpenAiMessageContent with an empty.

Feature scope:

Get assistant message from chat response (OpenAiChatCompletionResponse) with getMessage()
New class to capture Function type tool calls suggested by LLM - OpenAiFunctionCall
Assistant messages has a new field for toolCalls
OpenAiChatCompletionRequest(List<OpenAiMessage>) added for better request creation with externally managed message list

Definition of Done

Functionality scope stated & covered
Tests cover the scope above
~~Error handling created / updated & covered by the tests above~~
~~Aligned changes with the JavaScript SDK~~
Documentation updated
Release notes updated

…nai/tool-call-execute

- Mostly tested - API design complete - Function Call as message content item - Non nullability of `Message.content()`

…nai/tool-call-execute

...dels/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiAssistantMessage.java

...openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionRequest.java

...penai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionResponse.java

sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java

sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OpenAiV2Test.java

…nai/tool-call-execute # Conflicts: # sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java

- OpenAiAssistantMessage().content() may contain empty list

- final keyword - remove redundant assertion - variable naming

…nai/tool-call-execute # Conflicts: # sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java

- final keyword - variable naming

...penai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionResponse.java

...on-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiToolMessage.java

...on-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAIMessageTest.java

...ion-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java

...dels/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiAssistantMessage.java

...penai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionResponse.java

...n-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiFunctionCall.java

...dation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiMessage.java

...ation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiToolCall.java

…onmodels/openai/OpenAiChatCompletionResponse.java Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>

javadoc and naming Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>

...s/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientGeneratedTest.java

…P/ai-sdk-java into feat/openai/tool-call-execute

newtork · 2025-04-03T08:38:43Z

...n-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiFunctionCall.java

+  @Nonnull String name;
+
+  /** The arguments for the function call, encoded as a JSON string. */
+  @Nonnull String arguments;


(Question)

I'm not sure. Is this supposed to be a convenience class? If yes, how is "String" convenient to access arguments "encoded as a JSON"? Or is it not expected to be consumed by users?

I had only considered deserializing directly into the corresponding user provided model class. I ruled out this convenience because we don't have guarantees on their deserializability.

But, it is a possibility to parse it into a json object. We could provide this convenience. Is that what you are pointing at?

As discussed, we would provide additional convenience in a separate PR.

rpanackal and others added 10 commits March 14, 2025 16:26

Tool call full e2e unverified

1662ae7

CI

9ef8e6f

Reuse tool

ef9bdc4

Remove unnecessary line inserts

fd6acdc

Create schema with jackson

a6a209b

Merge remote-tracking branch 'refs/remotes/origin/main' into test/ope…

9ad7225

…nai/tool-call-execute

First version

9a5aec3

- Mostly tested - API design complete - Function Call as message content item - Non nullability of `Message.content()`

Formatting

275eee5

Testing and javadocs

ad9ef65

improve test clarity

e70eb29

rpanackal self-assigned this Mar 27, 2025

rpanackal changed the base branch from main to test/openai/tool-call-execute March 27, 2025 14:25

rpanackal added 2 commits March 27, 2025 15:30

Merge remote-tracking branch 'refs/remotes/origin/main' into test/ope…

a13b183

…nai/tool-call-execute

Merge branch 'refs/heads/test/openai/tool-call-execute' into feat/ope…

03a14a4

…nai/tool-call-execute

rpanackal mentioned this pull request Mar 27, 2025

test: [OpenAI] E2E Tool Call Execution #386

Merged

6 tasks

rpanackal added 2 commits March 27, 2025 15:49

Remove strict tool invocation config

bf2ce11

Merge branch 'refs/heads/test/openai/tool-call-execute' into feat/ope…

f74a0a7

…nai/tool-call-execute

CharlesDuboisSAP reviewed Mar 28, 2025

View reviewed changes

sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/controllers/OpenAiV2Test.java Outdated Show resolved Hide resolved

rpanackal added 10 commits March 28, 2025 14:19

Fix maven dependency scope issue

698eab8

Fix maven dependency scope issue + 1

f694f40

Merge branch 'refs/heads/test/openai/tool-call-execute' into feat/ope…

4e388e2

…nai/tool-call-execute # Conflicts: # sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java

Refactor. toolCalls get dedicated field in assistant message

78aa10e

- OpenAiAssistantMessage().content() may contain empty list

minor fixes

49b8be4

- final keyword - remove redundant assertion - variable naming

Merge branch 'refs/heads/test/openai/tool-call-execute' into feat/ope…

78c2909

…nai/tool-call-execute # Conflicts: # sample-code/spring-app/src/test/java/com/sap/ai/sdk/app/services/OpenAiServiceV2.java

minor fixes

05d6218

- final keyword - variable naming

Test message list in request externally unmodifiable

23a247a

Test message list in request externally unmodifiable

4642de0

minor javadoc update

96647f5

rpanackal added the please-review Request to review a pull-request label Mar 31, 2025

@beta annotation

26b7dae

CharlesDuboisSAP requested changes Apr 1, 2025

View reviewed changes

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...ion-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...dels/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiAssistantMessage.java Outdated Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...dels/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiAssistantMessage.java Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...penai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiChatCompletionResponse.java Outdated Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...n-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiFunctionCall.java Outdated Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...dation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiMessage.java Outdated Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...ation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiToolCall.java Outdated Show resolved Hide resolved

rpanackal and others added 2 commits April 1, 2025 10:02

Update foundation-models/openai/src/main/java/com/sap/ai/sdk/foundati…

f616263

…onmodels/openai/OpenAiChatCompletionResponse.java Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>

Apply suggestions from code review

cd11f37

javadoc and naming Co-authored-by: Charles Dubois <103174266+CharlesDuboisSAP@users.noreply.github.com>

rpanackal removed the please-review Request to review a pull-request label Apr 1, 2025

rpanackal added 3 commits April 1, 2025 13:33

update getMessage method impl readability

b95b960

Move test to generated client test class

9062e3a

Extend tests

69b49ea

rpanackal added the please-review Request to review a pull-request label Apr 1, 2025

Reduce test

d26ecbf

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...s/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientGeneratedTest.java Outdated Show resolved Hide resolved

CharlesDuboisSAP reviewed Apr 1, 2025

View reviewed changes

...s/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientGeneratedTest.java Outdated Show resolved Hide resolved

rpanackal and others added 5 commits April 1, 2025 15:20

Refactor test

40f1e22

Formatting

bd61ce2

import statement

d440c28

Merge branch 'refs/heads/main' into feat/openai/tool-call-execute

c6969f4

Merge branch 'feat/openai/tool-call-execute' of https://github.com/SA…

a36bef0

…P/ai-sdk-java into feat/openai/tool-call-execute

rpanackal added please-review Request to review a pull-request and removed please-review Request to review a pull-request labels Apr 1, 2025

CharlesDuboisSAP approved these changes Apr 3, 2025

View reviewed changes

newtork reviewed Apr 3, 2025

View reviewed changes

rpanackal merged commit f7ebcad into main Apr 3, 2025
6 checks passed

rpanackal deleted the feat/openai/tool-call-execute branch April 3, 2025 11:18

Conversation

rpanackal commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Non-nullability guarantee of OpenAiAssistantMessage().content()

Feature scope:

Definition of Done

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

newtork Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpanackal Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpanackal Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rpanackal commented Mar 26, 2025 •

edited

Loading

Non-nullability guarantee of `OpenAiAssistantMessage().content()`

newtork Apr 3, 2025 •

edited

Loading

rpanackal Apr 3, 2025 •

edited

Loading

rpanackal Apr 3, 2025 •

edited

Loading