Refactor options for strict immutability and centralized default values#6218
Closed
sdeleuze wants to merge 15 commits into
Closed
Refactor options for strict immutability and centralized default values#6218sdeleuze wants to merge 15 commits into
sdeleuze wants to merge 15 commits into
Conversation
- Use protected ctor - Make fields final - Remove Jackson annotations not needed anymore - Stop using options classes directly as `@NestedConfigurationProperty` in Spring Boot `@ConfigurationProperties`. - Introduce dedicated nested options classes for property binding. - Update auto-configurations and tests to use the new properties structure. Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Not needed anymore since options are now immutable Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Not needed since should be consistent with options level ones. Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Make collections (lists, sets, maps) in options classes strictly immutable by wrapping them in List.copyOf(), Set.copyOf(), Map.copyOf(), and Collectors.toUnmodifiableMap() instead of instantiating mutable collections. Update combineWith() logic in options builders to properly merge collections when overridden, rather than blindly overwriting the base collections with the override ones. Also optimize builder clone() methods to take advantage of this immutability. Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Refactor model options to use null values for undefined collections (like tool callbacks, names, and headers) instead of defaulting to empty collections. This ensures that explicitly empty collections are not ignored during option merging, properly distinguishing between an unset option and an intentionally empty one. Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Contributor
|
@sdeleuze: I like this refactor! I have added a few comments that could be applied to other AI vendors as well. |
ericbottard
reviewed
Jun 1, 2026
ericbottard
reviewed
Jun 1, 2026
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Removed because options are now strictly immutable. See spring-projects#6218
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Contributor
Author
|
I applied the suggested changes. |
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
Removed because options are now strictly immutable. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
658d2ce to
9789d54
Compare
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
- Use protected ctor - Make fields final - Remove Jackson annotations not needed anymore - Stop using options classes directly as `@NestedConfigurationProperty` in Spring Boot `@ConfigurationProperties`. - Introduce dedicated nested options classes for property binding. - Update auto-configurations and tests to use the new properties structure. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Not needed anymore since options are now immutable Seee spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Not needed since should be consistent with options level ones. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Make collections (lists, sets, maps) in options classes strictly immutable by wrapping them in List.copyOf(), Set.copyOf(), Map.copyOf(), and Collectors.toUnmodifiableMap() instead of instantiating mutable collections. Update combineWith() logic in options builders to properly merge collections when overridden, rather than blindly overwriting the base collections with the override ones. Also optimize builder clone() methods to take advantage of this immutability. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Refactor model options to use null values for undefined collections (like tool callbacks, names, and headers) instead of defaulting to empty collections. This ensures that explicitly empty collections are not ignored during option merging, properly distinguishing between an unset option and an intentionally empty one. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
Removed because options are now strictly immutable. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 1, 2026
See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
nicolaskrier
added a commit
to nicolaskrier/spring-ai
that referenced
this pull request
Jun 1, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor - Rename with methods from N to n See spring-projects#6218 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com>
nicolaskrier
added a commit
to nicolaskrier/spring-ai
that referenced
this pull request
Jun 2, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor - Rename with methods from N to n See spring-projects#6218 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com>
nicolaskrier
added a commit
to nicolaskrier/spring-ai
that referenced
this pull request
Jun 2, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor - Rename with methods from N to n See spring-projects#6218 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com>
sdeleuze
pushed a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 4, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor See spring-projects#6218 Closes spring-projects#6248 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com> Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
pushed a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 4, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor See spring-projects#6218 Closes spring-projects#6248 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com> Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
sdeleuze
pushed a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 4, 2026
…rties - Remove unnecessary null cheks - Make some with methods nullable for option builder - Handle default values in options constructor See spring-projects#6218 Closes spring-projects#6248 Signed-off-by: Nicolas Krier <7557886+nicolaskrier@users.noreply.github.com>
sdeleuze
added a commit
to sdeleuze/spring-ai
that referenced
this pull request
Jun 5, 2026
They should now be managed and exposed at options level. See spring-projects#6218 Signed-off-by: Sébastien Deleuze <sdeleuze@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the various
Optionsclasses (such asChatOptions,EmbeddingOptions, etc.) across the framework to enforce strict immutability, improve null-safety representation, and centralize where default values are defined.Previously, default values were scattered across
Modelimplementations and Auto-configuration*Propertiesclasses, and options collections were sometimes mutable or defaulted to empty collections (which caused issues when merging). This PR streamlines the options architecture to be safer and more predictable.Key Changes
toolCallbacks,stopSequences,customHeaders) by using unmodifiable copies.@Nullable) to accurately represent the absence of a value, which fixes and improves the behavior of options merging.*Propertiesconfiguration classes directly into the options constructors.*Propertiesclasses since they are now consistently handled at the options level.ChatModel#getDefaultOptions()toChatModel#getOptions()(the former is now deprecated).ChatOptions#copy()since options are now strictly immutable (users should usemutate().build()instead).@NullMarkedannotations, and fixed integration tests to align with the new options behavior.upgrade-notes.adocfor2.0.0-RC1to detail the breaking changes and migration paths for these options updates.Breaking Changes & Migration
ChatOptions#copy()and*Options#fromOptionshave been removed. Usemutate().build()to create a modified copy of an options instance.UnsupportedOperationException.ChatModel#getDefaultOptions()is deprecated in favor ofChatModel#getOptions().*Propertiesclasses, which may affect code that relied on reading those default properties directly.@tzolov @ilayaperumalg I will take care of merging it, please just provide your review.
@ThomasVitale @nicolaskrier Feel free to provide your feedback as well.
I recommend reviewing commit by commit, each has a meaningful description.