Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* Copyright 2024-2026 Embabel Pty Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.embabel.agent.anthropic

import com.embabel.agent.spi.support.springai.SpringAiLlmService
import com.embabel.common.ai.model.LlmOptions
import io.mockk.mockk
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.springframework.ai.anthropic.AnthropicChatOptions

/**
* Regression for the Spring AI 2.0 model-binding bug affecting Anthropic:
* [AnthropicOptionsConverter] never calls .model(), so its [AnthropicChatOptions] carry
* DEFAULT_MODEL ("claude-haiku-4-5"). Spring AI 2.0 no longer merges the ChatModel bean's
* configured model into per-request options, so haiku would go on the wire silently.
* [SpringAiLlmService.convertOptions] now stamps the service name explicitly.
*/
class AnthropicModelBindingTest {

@Test
fun `convertOptions stamps configured model not AnthropicChatOptions haiku default`() {
val service = SpringAiLlmService(
name = "claude-sonnet-4-5",
provider = "Anthropic",
chatModel = mockk(relaxed = true),
optionsConverter = AnthropicOptionsConverter,
)

val result = service.convertOptions(LlmOptions())

assertThat(result).isInstanceOf(AnthropicChatOptions::class.java)
assertThat(result.model).isEqualTo("claude-sonnet-4-5")
assertThat(result.model).isNotEqualTo(AnthropicChatOptions.DEFAULT_MODEL)
}

@Test
fun `convertOptions preserves AnthropicChatOptions fields alongside model`() {
val service = SpringAiLlmService(
name = "claude-sonnet-4-5",
provider = "Anthropic",
chatModel = mockk(relaxed = true),
optionsConverter = AnthropicOptionsConverter,
)

val result = service.convertOptions(LlmOptions().withMaxTokens(500))

assertThat(result).isInstanceOf(AnthropicChatOptions::class.java)
assertThat(result.model).isEqualTo("claude-sonnet-4-5")
assertThat(result.maxTokens).isEqualTo(500)
}

@Test
fun `convertOptions result type is AnthropicChatOptions not generic fallback`() {
val service = SpringAiLlmService(
name = "claude-haiku-4-5",
provider = "Anthropic",
chatModel = mockk(relaxed = true),
optionsConverter = AnthropicOptionsConverter,
)

val result = service.convertOptions(LlmOptions())

assertThat(result).isInstanceOf(AnthropicChatOptions::class.java)
assertThat(result.model).isEqualTo("claude-haiku-4-5")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import org.springframework.ai.anthropic.AnthropicCacheStrategy
import org.springframework.ai.anthropic.AnthropicCacheTtl
import org.springframework.ai.anthropic.AnthropicChatOptions

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class AnthropicOptionsConverterTest : OptionsConverterTestSupport<AnthropicChatOptions>(
optionsConverter = AnthropicOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ internal class ChatClientLlmOperations(
): LlmMessageSender {
if (llmRequestEvent != null) {
val springAiLlm = requireSpringAiLlm(llm)
val chatOptions = springAiLlm.optionsConverter.convertOptions(options)
val chatOptions = springAiLlm.convertOptions(options)
val instrumentedModel = InstrumentedChatModel(springAiLlm.chatModel, llmRequestEvent)
return SpringAiLlmMessageSender(
chatModel = instrumentedModel,
Expand Down Expand Up @@ -348,7 +348,7 @@ internal class ChatClientLlmOperations(

val schemaFormat = converter?.getFormat()

val chatOptions = requireSpringAiLlm(llm).optionsConverter.convertOptions(interaction.llm)
val chatOptions = requireSpringAiLlm(llm).convertOptions(interaction.llm)
val timeoutMillis = getTimeoutMillis(interaction.llm)

val basePrompt = if (schemaFormat != null) {
Expand Down Expand Up @@ -507,7 +507,7 @@ internal class ChatClientLlmOperations(
// Get the complete format (examples + JSON schema)
val schemaFormat = converter.getFormat()

val chatOptions = requireSpringAiLlm(llm).optionsConverter.convertOptions(interaction.llm)
val chatOptions = requireSpringAiLlm(llm).convertOptions(interaction.llm)
val timeoutMillis = getTimeoutMillis(interaction.llm)

val basePrompt = buildPromptWithMaybeReturnAndSchema(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.springframework.ai.chat.client.ChatClient
import org.springframework.ai.chat.messages.UserMessage
import org.springframework.ai.chat.model.ChatModel
import org.springframework.ai.chat.model.ChatResponse
import org.springframework.ai.chat.prompt.ChatOptions
import org.springframework.ai.chat.prompt.Prompt
import reactor.core.publisher.Flux
import java.time.Duration
Expand Down Expand Up @@ -85,7 +86,9 @@ private object StreamingCapabilityVerifier {
* @param name Name of the LLM
* @param provider Name of the provider (e.g., "OpenAI", "Anthropic")
* @param chatModel The Spring AI ChatModel to use for LLM calls
* @param optionsConverter Function to convert [LlmOptions] to Spring AI ChatOptions
* @param optionsConverter Function to convert [LlmOptions] to Spring AI ChatOptions.
* Do not call [OptionsConverter.convertOptions] directly — use [SpringAiLlmService.convertOptions]
* which also stamps the configured model name.
* @param knowledgeCutoffDate Model's knowledge cutoff date, if known
* @param promptContributors List of prompt contributors for this model.
* Knowledge cutoff is automatically included if knowledgeCutoffDate is set.
Expand Down Expand Up @@ -123,11 +126,13 @@ data class SpringAiLlmService @JvmOverloads constructor(
*/
override val model: ChatModel get() = chatModel

fun convertOptions(llmOptions: LlmOptions): ChatOptions =
optionsConverter.convertOptions(llmOptions, name)

override fun createMessageSender(options: LlmOptions): LlmMessageSender {
val chatOptions = optionsConverter.convertOptions(options)
return SpringAiLlmMessageSender(
chatModel = chatModel,
chatOptions = chatOptions,
chatOptions = convertOptions(options),
toolResponseContentAdapter = toolResponseContentAdapter,
nativeStructuredOutputConfigurer = nativeStructuredOutputConfigurer,
nativeSupport = nativeSupport,
Expand All @@ -136,9 +141,8 @@ data class SpringAiLlmService @JvmOverloads constructor(
}

override fun createMessageStreamer(options: LlmOptions): LlmMessageStreamer {
val chatOptions = optionsConverter.convertOptions(options)
val chatClient = ChatClient.create(chatModel)
return SpringAiLlmMessageStreamer(chatClient, chatOptions)
return SpringAiLlmMessageStreamer(chatClient, convertOptions(options))
}

override fun supportsStreaming(): Boolean = StreamingCapabilityVerifier.supportsStreaming(chatModel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ internal class StreamingChatClientOperations(
val userMessages = messages.filterIsInstance<com.embabel.chat.UserMessage>()
validateUserInput(userMessages, interaction, llmRequestEvent?.agentProcess?.blackboard)

val chatOptions = requireSpringAiLlm(llm).optionsConverter.convertOptions(interaction.llm)
val chatOptions = requireSpringAiLlm(llm).convertOptions(interaction.llm)

// Resolve tool groups and decorate tools
val tools = chatClientLlmOperations.resolveAndDecorateTools(interaction, agentProcess, action)
Expand Down Expand Up @@ -346,7 +346,7 @@ internal class StreamingChatClientOperations(
// Chat Client
val chatClient = chatClientLlmOperations.createChatClient(llm)
// Chat Options, additional potential option "streaming"
val chatOptions = requireSpringAiLlm(llm).optionsConverter.convertOptions(interaction.llm)
val chatOptions = requireSpringAiLlm(llm).convertOptions(interaction.llm)

// Spring AI 2.0's StreamingJacksonOutputConverter requires T : Any;
// erase O via Class<Any> for the construction, cast result back at use sites.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import io.mockk.mockk
import io.mockk.slot
import jakarta.validation.Validation
import jakarta.validation.constraints.Pattern
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Assertions.*
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -1323,4 +1324,26 @@ class ChatClientLlmOperationsTest {
}
}

@Nested
inner class ModelBinding {

@Test
fun `model name from SpringAiLlmService reaches ChatModel call`() {
val duke = Dog("Duke")
val fakeChatModel = FakeChatModel(jacksonObjectMapper().writeValueAsString(duke))
val setup = createChatClientLlmOperations(fakeChatModel)

setup.llmOperations.createObject(
messages = listOf(UserMessage("prompt")),
interaction = LlmInteraction(id = InteractionId("id"), llm = LlmOptions()),
outputClass = Dog::class.java,
action = SimpleTestAgent.actions.first(),
agentProcess = setup.mockAgentProcess,
)

assertThat(fakeChatModel.optionsPassed).isNotEmpty()
assertThat(fakeChatModel.optionsPassed[0].model).isEqualTo("fake")
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.springframework.ai.chat.model.ChatModel
import org.springframework.ai.chat.prompt.ChatOptions
import org.springframework.ai.model.tool.ToolCallingChatOptions
import java.time.LocalDate

class SpringAiLlmServiceTest {
Expand Down Expand Up @@ -249,6 +250,45 @@ class SpringAiLlmServiceTest {
}
}

@Nested
inner class ConvertOptionsTests {

@Test
fun `convertOptions stamps service name as model`() {
val service = SpringAiLlmService(
name = "my-model",
provider = "Provider",
chatModel = mockChatModel,
)
assertThat(service.convertOptions(LlmOptions()).model).isEqualTo("my-model")
}

@Test
fun `convertOptions overrides converter default model with service name`() {
val service = SpringAiLlmService(
name = "my-model",
provider = "Provider",
chatModel = mockChatModel,
)
val result = service.convertOptions(LlmOptions())
assertThat(result.model).isEqualTo("my-model")
assertThat(result.model).isNotEqualTo("some-default-model")
}

@Test
fun `convertOptions preserves converter fields alongside model`() {
val service = SpringAiLlmService(
name = "my-model",
provider = "Provider",
chatModel = mockChatModel,
)
val result = service.convertOptions(LlmOptions().withTemperature(0.5).withMaxTokens(100))
assertThat(result.model).isEqualTo("my-model")
assertThat(result.temperature).isEqualTo(0.5)
assertThat(result.maxTokens).isEqualTo(100)
}
}

@Nested
inner class CreateMessageSenderTests {

Expand All @@ -272,7 +312,7 @@ class SpringAiLlmServiceTest {
val customConverter = object : OptionsConverter<ChatOptions> {
override fun convertOptions(options: LlmOptions): ChatOptions {
converterCalled = true
return mockk()
return ToolCallingChatOptions.builder().build()
}
}
val service = SpringAiLlmService(
Expand Down Expand Up @@ -314,7 +354,7 @@ class SpringAiLlmServiceTest {
val customConverter = object : OptionsConverter<ChatOptions> {
override fun convertOptions(options: LlmOptions): ChatOptions {
converterCalled = true
return mockk()
return ToolCallingChatOptions.builder().build()
}
}
val service = SpringAiLlmService(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ public Integer calculateDewPoint(Integer temperatureFahrenheit, Integer relative

@Test
void testSystemPromptCaching() {
logger.info("Testing system prompt caching");
logger.info("Testing system prompt caching with thinking enabled");

AnthropicCachingConfig cachingConfig = new AnthropicCachingConfig();
cachingConfig.setSystemPrompt(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.springframework.ai.deepseek.DeepSeekChatOptions

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class DeepSeekOptionsConverterTest : OptionsConverterTestSupport<DeepSeekChatOptions>(
optionsConverter = DeepSeekOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Test
import org.springframework.ai.google.genai.GoogleGenAiChatOptions

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class GoogleGenAiOptionsConverterTest : OptionsConverterTestSupport<GoogleGenAiChatOptions>(
optionsConverter = GoogleGenAiOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.springframework.ai.openai.OpenAiChatOptions

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class MiniMaxOptionsConverterTest : OptionsConverterTestSupport<OpenAiChatOptions>(
optionsConverter = MiniMaxOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import org.junit.jupiter.api.Test
import org.springframework.ai.ollama.api.OllamaChatOptions
import org.springframework.ai.ollama.api.ThinkOption

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class OllamaOptionsConverterTest : OptionsConverterTestSupport<OllamaChatOptions>(
optionsConverter = OllamaOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.springframework.ai.openai.OpenAiChatOptions

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
class ZaiOptionsConverterTest : OptionsConverterTestSupport<OpenAiChatOptions>(
optionsConverter = ZaiOptionsConverter
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,25 @@
import org.springframework.ai.model.tool.ToolCallingChatOptions

/**
* Convert our LLM options to Spring AI ChatOptions
* Convert our LLM options to Spring AI ChatOptions.
*
* Prefer [convertOptions] with an explicit model over the no-model overload.
* The no-model form is deprecated because provider converters carry hardcoded
* default models that will silently go to the wire if not overridden.
*/
// TODO: update all converter implementations to override convertOptions(options, model) directly,

Check warning on line 28 in embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/OptionsConverter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this TODO comment.

See more on https://sonarcloud.io/project/issues?id=embabel_embabel-agent&issues=AZ-XEEoa4HFOx29gWrIU&open=AZ-XEEoa4HFOx29gWrIU&pullRequest=1818
// then remove the deprecated 1-arg form and the @Suppress below.
fun interface OptionsConverter<O : ChatOptions> {

@Deprecated(
message = "Provide the model explicitly — provider converters carry hardcoded defaults that bypass the configured model.",
replaceWith = ReplaceWith("convertOptions(options, model)"),
)
fun convertOptions(options: LlmOptions): O

Check warning on line 36 in embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/OptionsConverter.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Do not forget to remove this deprecated code someday.

See more on https://sonarcloud.io/project/issues?id=embabel_embabel-agent&issues=AZ-XEEoa4HFOx29gWrIT&open=AZ-XEEoa4HFOx29gWrIT&pullRequest=1818

@Suppress("DEPRECATION")
fun convertOptions(options: LlmOptions, model: String): ChatOptions =
convertOptions(options).mutate().model(model).build()
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import com.embabel.common.ai.model.OptionsConverter
import org.junit.jupiter.api.Assertions.assertEquals

// Calls the deprecated 1-arg convertOptions() directly to verify field mapping in isolation.
// Model stamping is not tested here — it is covered by OptionsConverter.convertOptions(options, model).
fun checkOptionsConverterPreservesCoreValues(optionsConverter: OptionsConverter<*>) {
val llmo = LlmOptions().withTemperature(temperature = 0.5).withTopK(10).withTopP(.2).withFrequencyPenalty(.2)
val options = optionsConverter.convertOptions(llmo)

Check warning on line 26 in embabel-agent-test-support/embabel-agent-test-internal/src/main/kotlin/com/embabel/agent/test/models/optionsConverterTestUtils.kt

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Deprecated code should not be used.

See more on https://sonarcloud.io/project/issues?id=embabel_embabel-agent&issues=AZ-XEEt84HFOx29gWrIV&open=AZ-XEEt84HFOx29gWrIV&pullRequest=1818
assertEquals(llmo.temperature, options.temperature, "Should have preserved temperature")
// assertEquals(llmo.topK, options.topK, "Should have preserved topK")
assertEquals(llmo.topP, options.topP, "Should have preserved topP")
Expand Down
Loading