Skip to content

Fix model binding broken by Spring AI 2.0 options merge removal - #1818

Merged
alexheifetz merged 2 commits into
mainfrom
post-spring-ai-2-fixes
Jul 25, 2026
Merged

Fix model binding broken by Spring AI 2.0 options merge removal#1818
alexheifetz merged 2 commits into
mainfrom
post-spring-ai-2-fixes

Conversation

@igordayen

@igordayen igordayen commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

OVERVIEW


Problem

Spring AI 2.0 removed the implicit merge between per-request ChatOptions and the ChatModel bean's configured default options.
Every provider's OptionsConverter implementation omits .model() from the builder, relying on the old merge to stamp the correct model.

Example — Anthropic: AnthropicOptionsConverter.convertOptions() never calls .model(), so every call produces
AnthropicChatOptions with DEFAULT_MODEL = "claude-haiku-4-5" regardless of which model was selected. The
AnthropicChatModel bean had claude-sonnet-4-5 in its configured default properties options — in 1.x that won; in 2.0 it is never consulted.

Fix

Added buildChatOptions(llmOptions) to SpringAiLlmService that stamps this.name (the configured model ID) onto the
converted options after conversion:

  fun buildChatOptions(llmOptions: LlmOptions): ChatOptions =
      optionsConverter.convertOptions(llmOptions).mutate().model(name).build()

This is provider-agnostic — name is the model ID set at SpringAiLlmService construction time, independent of whatever default any converter bakes in.

Applied at all call sites that build per-request options:

  • SpringAiLlmService.createMessageSender and createMessageStreamer
  • ChatClientLlmOperations.createMessageSender (override — the actual Anthropic/non-streaming call path; the fix in
    SpringAiLlmService alone is bypassed here)
  • Two additional sites in ChatClientLlmOperations (lines 351, 510)
  • Two sites in StreamingChatClientOperations (lines 177, 349)

Tests

  • SpringAiLlmServiceTest.BuildChatOptionsTests — unit tests for model stamping and field preservation
  • ChatClientLlmOperationsTest.ModelBinding — proves model name travels through the ChatClientLlmOperations override
    path to ChatModel.call()
  • AnthropicModelBindingTest — Anthropic-specific regression: AnthropicOptionsConverter + buildChatOptions produces
    AnthropicChatOptions with the configured model, not DEFAULT_MODEL

Closes: #1815 , #1735 , #1816

@alexheifetz alexheifetz left a comment

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.

Decision needed on cohesiveness of semantics requireSpringAiLlm(llm).optionsConverter
vs requireSpringAiLlm(llm).buildXYZ

@igordayen

Copy link
Copy Markdown
Contributor Author

Decision needed on cohesiveness of semantics requireSpringAiLlm(llm).optionsConverter vs requireSpringAiLlm(llm).buildXYZ

Addressed :

  1. Added 2-arg function convertOptions (model is a second arg) to OptionConverter
  2. Renamed buildXyz into convertOptions to retain similar naming conventions (and reduce cognitive load for developers:))
  3. Marked the 1-arg function convertOptions as DEPRECATED
  4. Provided proper comments / TODO

@sonarqubecloud

Copy link
Copy Markdown

@alexheifetz
alexheifetz merged commit 957562b into main Jul 25, 2026
17 checks passed
@alexheifetz
alexheifetz deleted the post-spring-ai-2-fixes branch July 25, 2026 15:12
igordayen pushed a commit that referenced this pull request Jul 27, 2026
…#1828)

* test: add regression test for Ollama configured model binding (#1735)

Verifies that a registered Ollama LLM sends requests targeting the model
it was registered for, rather than the OllamaChatOptions mistral fallback,
by exercising the same SpringAiLlmService.convertOptions path that
ChatClientLlmOperations uses to build per-request options.

Complements the converter field-mapping tests added in #1818, which
deliberately leave model stamping uncovered.

Signed-off-by: sumin220 <173463176+sumin220@users.noreply.github.com>

* test: resolve nested config classes via ClassUtils

Addresses review feedback on #1828: use Spring's ClassUtils.forName with
the config class's own loader instead of Class.forName, avoiding classpath
issues in environments with non-default loader hierarchies.

Signed-off-by: sumin220 <173463176+sumin220@users.noreply.github.com>

* test: replace unguarded casts with assertIs

Addresses review feedback on #1828: both casts now fail with a clear
assertion message instead of a raw ClassCastException, and the bean
lookup reports the registered bean names when the expected bean is
missing (covering the null-from-map case as well).

Signed-off-by: sumin220 <173463176+sumin220@users.noreply.github.com>

---------

Signed-off-by: sumin220 <173463176+sumin220@users.noreply.github.com>
Co-authored-by: sumin220 <173463176+sumin220@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gemini OpenAI-compatible guardrail IT sends gpt-5-mini instead of selected Gemini model

2 participants