Skip to content

feat: migrate spring-ai to v2 - #1006

Open
vladimir-a-sap wants to merge 13 commits into
mainfrom
spring-ai-v2
Open

feat: migrate spring-ai to v2#1006
vladimir-a-sap wants to merge 13 commits into
mainfrom
spring-ai-v2

Conversation

@vladimir-a-sap

@vladimir-a-sap vladimir-a-sap commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Context

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

Updated Spring AI glue to version 2.0.1

Documentation pr - SAP/ai-sdk#687

Definition of Done

  • Code is tested (Unit, E2E)
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Only Public APIs are allowed to be used in documentation/tutorials/sample code
  • (Optional) Aligned changes with the JS SDK (no spring ai for JS SDK)
  • (Optional) Release notes updated
  • revert version fix for dependabot

@vladimir-a-sap vladimir-a-sap changed the title migrate spring-ai to v2 feat: migrate spring-ai to v2 Aug 27, 2026
@vladimir-a-sap
vladimir-a-sap marked this pull request as ready for review August 28, 2026 13:10
@vladimir-a-sap
vladimir-a-sap enabled auto-merge (squash) August 28, 2026 13:10
<dependency>
<groupId>com.github.victools</groupId>
<artifactId>jsonschema-module-swagger-2</artifactId>
<version>5.0.0</version>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. Why do you need to update the version? if it doesn't work with 4.38.0, that would mean it doesn't work for our customers out of the box, unless you pin the version in parent pom or openai pom.
  2. Isn't this dependency already at version 5 in the parent pom? If not I would at least reuse the version number property jsonschema-generator.version, or create one in the spring app if it's unrelated.

<dependency>
<groupId>com.networknt</groupId>
<artifactId>json-schema-validator</artifactId>
<version>3.0.1</version>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again

  1. Why do you need to update the version? That would mean it doesn't work for our customers out of the box, unless you pin the version in parent pom or another released pom.

Comment thread .github/dependabot.yaml
@@ -22,8 +22,6 @@ updates:
versions: [ ">=5.0.0" ]
- dependency-name: "com.github.victools:jsonschema-module-jackson"
versions: [ ">=5.0.0" ]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why about the other dependencies that are ignored here?

Comment thread docs/release_notes.md
### 🔧 Compatibility Notes

-
-[Orchestration] Spring AI support was upgraded to version `2.0.1`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could make a mini migration guide, SpringAiOpenAiService received some breaking changes, they could be documented. Also newer dependency versions might be required.

Comment thread orchestration/pom.xml
<dependency>
<groupId>tools.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. Why?
  2. Also if this is Spring AI only shouldn't it be optional? (Not sure about the impacts of that)

@Jonas-Isr Jonas-Isr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there are some breaking changes for our users hidden in the code changes. While I think it is correct to change it that way and I don't think we can do anything better, we need to discuss if we are okay with a breaking change in convenience code in a minor version release or if we need to push for AI SDK major version 2.

This is a discussion we have to have with Junjie once we are certain how many and what breaking changes would be necessary. (At the least, we needs to add migration steps to the Compatibility Notes section of the release notes.)

Comment on lines +203 to 205
public Boolean isInternalToolExecutionEnabled() {
return this.internalToolExecutionEnabled;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Breaking Change)

This will be a breaking change for users. I think it is correct to change the name as it is done similarly in Spring AI v2 if I am not mistaking.

Comment on lines -160 to 161
@Override
public <T extends ChatOptions> T copy() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Breaking Change?)

Loosing the @Override here means a potential breaking change for our users I think. Users who relied on ToolCallingChatOptions.copy() will have problems.

Comment on lines -182 to 188
@Override
/**
* Setter method
*
* @param toolCallbacks tool callbacks to set int template config
*/
public void setToolCallbacks(@Nonnull final List<ToolCallback> toolCallbacks) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Breaking Change?)

Same problem as with .copy() above.

return call(new Prompt(toolExecutionResult.conversationHistory(), options));
}
return response;
return new ChatResponse(toGenerations(result));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Question, Breaking change?)

Is it correct to skip this entire logic here? It looks to me that now there is no way anymore to directly execute the tool calls during the call. What is the migration guide for isInternalToolExecutionEnabled(options)? This might result in a breaking of usage behaviour for our users, right?

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.

3 participants