Skip to content

feat: Implementing minimal feature scope for Spring AI integration in OpenAI #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 56 commits into
base: main
Choose a base branch
from

Conversation

n-o-u-r-h-a-n
Copy link
Contributor

@n-o-u-r-h-a-n n-o-u-r-h-a-n commented Aug 5, 2025

Context

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

Integrating SpringAi with our OpenAiClient.

Feature scope:

  • Implementation of Chat Completion, Stream Chat Completion, Tool Calling and Chat Memory in SpringAiOpenAiService.
  • Making OpenAiChatOptions accept a config parameter using a new approach.
    • Creation of a new class : OpenAiChatCompletionConfig.
    • Updating OpenAiChatCompletionRequest for integration with OpenAiChatCompletionConfig class.
    • Updating OpenAiChatOptions accordingly as well(aligned with orchestration).

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

Jonas-Isr and others added 29 commits June 16, 2025 17:44
…AiService + their corresponding passed tests in SpringAiOpenAiTest class.

Regarding the OpenAiChatModel class it was just formatting, nothing changed.
--> 4 methods in OpenAiChatCompletionRequest.java return only this now and other constructors other than main are removed for now.
…ringopenai

# Conflicts:
#	core/src/main/java/com/sap/ai/sdk/core/common/ClientResponseHandler.java
@n-o-u-r-h-a-n n-o-u-r-h-a-n requested a review from rpanackal August 5, 2025 15:29
@n-o-u-r-h-a-n n-o-u-r-h-a-n removed the request for review from rpanackal August 7, 2025 17:03
Copy link
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, please have another reviewer- because I'm one of the contributors.

Please find my PR to reduce constructor visibility again in your branch:
#531

val options = new OpenAiChatOptions();
val prompt =
new Prompt("Can you give me the first 100 numbers of the Fibonacci sequence?", options);
return chatClient.call(prompt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return chatClient.call(prompt);
return chatClient.stream(prompt);

You would also need to change the return type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I do stream , I have an exception (streaming is not supported as stream is not overridden at all in OpenAi so currently implementing it)

Comment on lines +122 to +130
public <T extends ChatOptions> T copy() {
final OpenAiChatOptions copy = new OpenAiChatOptions();
copy.setToolCallbacks(this.toolCallbacks);
copy.setInternalToolExecutionEnabled(this.internalToolExecutionEnabled);
copy.setTools(this.tools);
copy.setToolNames(this.toolNames);
copy.setToolContext(this.toolContext);
return (T) copy;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Minor)

Please also copy the config and any other fields that was missed

newtork and others added 3 commits August 12, 2025 17:49
* Reduce constructors

* Update thresholds

* Update javadoc and factory name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants