feat: migrate spring-ai to v2 - #1006
Conversation
| <dependency> | ||
| <groupId>com.github.victools</groupId> | ||
| <artifactId>jsonschema-module-swagger-2</artifactId> | ||
| <version>5.0.0</version> |
There was a problem hiding this comment.
- 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. - 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> |
There was a problem hiding this comment.
Again
- 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.
| @@ -22,8 +22,6 @@ updates: | |||
| versions: [ ">=5.0.0" ] | |||
| - dependency-name: "com.github.victools:jsonschema-module-jackson" | |||
| versions: [ ">=5.0.0" ] | |||
There was a problem hiding this comment.
Why about the other dependencies that are ignored here?
| ### 🔧 Compatibility Notes | ||
|
|
||
| - | ||
| -[Orchestration] Spring AI support was upgraded to version `2.0.1` |
There was a problem hiding this comment.
You could make a mini migration guide, SpringAiOpenAiService received some breaking changes, they could be documented. Also newer dependency versions might be required.
| <dependency> | ||
| <groupId>tools.jackson.core</groupId> | ||
| <artifactId>jackson-databind</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
- Why?
- Also if this is Spring AI only shouldn't it be optional? (Not sure about the impacts of that)
Jonas-Isr
left a comment
There was a problem hiding this comment.
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.)
| public Boolean isInternalToolExecutionEnabled() { | ||
| return this.internalToolExecutionEnabled; | ||
| } |
There was a problem hiding this comment.
(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.
| @Override | ||
| public <T extends ChatOptions> T copy() { |
There was a problem hiding this comment.
(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.
| @Override | ||
| /** | ||
| * Setter method | ||
| * | ||
| * @param toolCallbacks tool callbacks to set int template config | ||
| */ | ||
| public void setToolCallbacks(@Nonnull final List<ToolCallback> toolCallbacks) { |
There was a problem hiding this comment.
(Breaking Change?)
Same problem as with .copy() above.
| return call(new Prompt(toolExecutionResult.conversationHistory(), options)); | ||
| } | ||
| return response; | ||
| return new ChatResponse(toGenerations(result)); |
There was a problem hiding this comment.
(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?
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
(Optional) Aligned changes with the JS SDK(no spring ai for JS SDK)