-
Notifications
You must be signed in to change notification settings - Fork 401
#1815 - Fix Gemini OpenAI-compatible guardrail IT sends gpt-5-mini instead of selected Gemini model #1816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1.5.0
Are you sure you want to change the base?
#1815 - Fix Gemini OpenAI-compatible guardrail IT sends gpt-5-mini instead of selected Gemini model #1816
Changes from all commits
2fd4251
9162b39
276baf3
71340b8
d874282
97cd9cc
7164aa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| /* | ||
| * 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.api.models.AnthropicModels | ||
| import com.embabel.agent.spi.support.springai.SpringAiLlmService | ||
| import com.embabel.chat.UserMessage | ||
| import com.embabel.common.ai.model.LlmOptions | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import io.mockk.slot | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.junit.jupiter.api.Test | ||
| import org.springframework.ai.anthropic.AnthropicChatOptions | ||
| import org.springframework.ai.chat.messages.AssistantMessage as SpringAiAssistantMessage | ||
| import org.springframework.ai.chat.metadata.ChatResponseMetadata | ||
| import org.springframework.ai.chat.model.ChatModel | ||
| import org.springframework.ai.chat.model.ChatResponse | ||
| import org.springframework.ai.chat.model.Generation | ||
| import org.springframework.ai.chat.prompt.Prompt | ||
|
|
||
| /** | ||
| * End-to-end regression for the Anthropic half of the Spring AI 2.0 model-binding bug | ||
| * (see #1815 for the OpenAI/Gemini equivalent). | ||
| * | ||
| * [AnthropicOptionsConverter] never sets a model, so its [AnthropicChatOptions] carry the | ||
| * SDK's baked-in `DEFAULT_MODEL` (`claude-haiku-4-5`). Because Embabel always passes these | ||
| * per-request options and Spring AI 2.0 no longer merges the model bean's configured model, | ||
| * every call would silently target Haiku. [SpringAiLlmService] now binds the configured | ||
| * model generically; this test proves the selected model reaches the wire while the concrete | ||
| * [AnthropicChatOptions] type and other fields are preserved. | ||
| */ | ||
| class AnthropicModelBindingTest { | ||
|
|
||
| @Test | ||
| fun `sends the selected Anthropic model rather than the converter default`() { | ||
| // Prepare: bean configured with Sonnet; converter (unchanged) bakes in Haiku default. | ||
| val configuredOptions = AnthropicChatOptions.builder().model("claude-sonnet-4-5").build() | ||
| val capturedPrompt = slot<Prompt>() | ||
| val generation = Generation(SpringAiAssistantMessage("done")) | ||
| val chatResponse = mockk<ChatResponse> { | ||
| every { result } returns generation | ||
| every { results } returns listOf(generation) | ||
| every { metadata } returns mockk<ChatResponseMetadata>(relaxed = true) { every { usage } returns null } | ||
| } | ||
| val chatModel = mockk<ChatModel> { | ||
| every { options } returns configuredOptions | ||
| every { call(capture(capturedPrompt)) } returns chatResponse | ||
| } | ||
| val service = SpringAiLlmService( | ||
| name = "claude-sonnet-4-5", | ||
| provider = AnthropicModels.PROVIDER, | ||
| chatModel = chatModel, | ||
| optionsConverter = AnthropicOptionsConverter, | ||
| thinkingSupported = true, | ||
| ) | ||
|
|
||
| // Execute | ||
| service.createMessageSender(LlmOptions().withMaxTokens(500)) | ||
| .call(messages = listOf(UserMessage("Hi")), tools = emptyList()) | ||
|
|
||
| // Verify: concrete Anthropic options carry the selected model, maxTokens preserved. | ||
| val sentOptions = capturedPrompt.captured.options | ||
| assertThat(sentOptions).isInstanceOf(AnthropicChatOptions::class.java) | ||
| assertThat(sentOptions.model).isEqualTo("claude-sonnet-4-5") | ||
| assertThat(sentOptions.maxTokens).isEqualTo(500) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /* | ||
| * 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.api.models.AnthropicModels | ||
| import com.embabel.chat.UserMessage | ||
| import com.embabel.common.ai.model.LlmOptions | ||
| import org.assertj.core.api.Assertions.assertThat | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable | ||
| import org.opentest4j.TestAbortedException | ||
|
|
||
| /** | ||
| * Live regression for the Anthropic same-provider silent variant of #1815: select Sonnet and | ||
| * assert Anthropic reports a sonnet model back. If binding regressed, the request would run on | ||
| * the AnthropicChatOptions default ("claude-haiku-4-5") and the served model would be a haiku. | ||
| * | ||
| * Requires ANTHROPIC_API_KEY. Skipped (aborted) if the key lacks access to the chosen model. | ||
| */ | ||
| class AnthropicServedModelBindingIT { | ||
|
|
||
| @Test | ||
| @EnabledIfEnvironmentVariable(named = "ANTHROPIC_API_KEY", matches = ".+") | ||
| fun `real Anthropic serves the selected model, not the default`() { | ||
| // Prepare | ||
| val model = AnthropicModels.CLAUDE_SONNET_4_5 | ||
| val llm = AnthropicModelFactory(apiKey = System.getenv("ANTHROPIC_API_KEY")).build(model) | ||
|
|
||
| // Execute | ||
| val response = try { | ||
| llm.createMessageSender(LlmOptions()) | ||
| .call(listOf(UserMessage("Reply with exactly the word READY.")), emptyList()) | ||
| } catch (ex: Exception) { | ||
| if (isModelAccessError(ex)) { | ||
| throw TestAbortedException("ANTHROPIC_API_KEY is set but lacks access to $model", ex) | ||
| } | ||
| throw ex | ||
| } | ||
|
|
||
| // Verify: Anthropic served the selected Sonnet, not the claude-haiku-4-5 default. | ||
| assertThat(response.textContent).isNotBlank() | ||
| assertThat(response.model) | ||
| .withFailMessage("Served model was '%s', expected a sonnet model", response.model) | ||
| .isNotNull() | ||
| .contains("sonnet") | ||
| assertThat(response.model).doesNotContain("haiku") | ||
| } | ||
|
|
||
| /** Broad check for a provider "no access to this model" error, so the IT aborts rather than fails. */ | ||
| private fun isModelAccessError(ex: Throwable): Boolean { | ||
| val message = generateSequence<Throwable>(ex) { it.cause } | ||
| .mapNotNull { it.message } | ||
| .joinToString(" | ") | ||
| .lowercase() | ||
| return listOf( | ||
| "not_found", | ||
| "does not exist", | ||
| "not found", | ||
| "permission", | ||
| "invalid model", | ||
| "unsupported model", | ||
| "does not have access", | ||
| ).any { message.contains(it) } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,11 +68,14 @@ data class LlmMessageRequest @JvmOverloads constructor( | |
| * @param message The full message object from the LLM | ||
| * @param textContent The text content of the message | ||
| * @param usage Optional usage information (tokens, etc.) | ||
| * @param model The model that actually served the request, as reported by the provider | ||
| * (from the response metadata). Null/blank if the provider does not report it. | ||
| */ | ||
| data class LlmMessageResponse( | ||
| val message: Message, | ||
| val textContent: String, | ||
| val usage: Usage? = null, | ||
| val model: String? = null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this enhancement not in scope for this PR? |
||
| ) | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,4 +139,3 @@ class UnfoldingToolInjectionStrategy : ToolInjectionStrategy { | |
| val INSTANCE = UnfoldingToolInjectionStrategy() | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this related? |
||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,7 @@ | |
| message = embabelMessage, | ||
| textContent = assistantMessage.text ?: "", | ||
| usage = usage, | ||
| model = response.metadata?.model, | ||
|
Check warning on line 114 in embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSender.kt
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sonar violation |
||
| ) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| 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 | ||
|
|
@@ -123,8 +124,22 @@ | |
| */ | ||
| override val model: ChatModel get() = chatModel | ||
|
|
||
| /** | ||
| * Binds the model configured on the underlying [ChatModel] onto per-request options. | ||
| * | ||
| * Spring AI 2.0 no longer merges a model's configured options into a prompt that already | ||
| * carries options (OpenAiChatModel/AnthropicChatModel `buildRequestPrompt` returns the | ||
| * prompt unchanged when `getOptions() != null`). Because Embabel always supplies | ||
| * per-request options — and provider option types coerce a null model to a hard-coded | ||
| * default in their constructor (e.g. `gpt-5-mini`, `claude-haiku-4-5`) — the selected | ||
| * model would otherwise be ignored on every call. Binding the [ChatModel]'s own | ||
| * configured model restores the pre-2.0 merge semantics for every provider at once. | ||
| */ | ||
| private fun bindConfiguredModel(chatOptions: ChatOptions): ChatOptions = | ||
| bindModel(chatOptions, chatModel.options?.model) | ||
|
Check warning on line 139 in embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmService.kt
|
||
|
|
||
| override fun createMessageSender(options: LlmOptions): LlmMessageSender { | ||
| val chatOptions = optionsConverter.convertOptions(options) | ||
| val chatOptions = bindConfiguredModel(optionsConverter.convertOptions(options)) | ||
| return SpringAiLlmMessageSender( | ||
| chatModel = chatModel, | ||
| chatOptions = chatOptions, | ||
|
|
@@ -136,7 +151,7 @@ | |
| } | ||
|
|
||
| override fun createMessageStreamer(options: LlmOptions): LlmMessageStreamer { | ||
| val chatOptions = optionsConverter.convertOptions(options) | ||
| val chatOptions = bindConfiguredModel(optionsConverter.convertOptions(options)) | ||
| val chatClient = ChatClient.create(chatModel) | ||
| return SpringAiLlmMessageStreamer(chatClient, chatOptions) | ||
| } | ||
|
|
@@ -160,3 +175,13 @@ | |
| fun withOptionsConverter(converter: OptionsConverter<*>): SpringAiLlmService = | ||
| copy(optionsConverter = converter) | ||
| } | ||
|
|
||
| /** | ||
| * Binds [model] onto [options], preserving the concrete provider option type and all other | ||
| * fields via each provider's overridden `mutate()` (dynamic dispatch keeps e.g. | ||
| * `OpenAiChatOptions`/`AnthropicChatOptions` intact). Returns [options] unchanged when | ||
| * [model] is null or blank, so models that expose no configured default (bare test doubles, | ||
| * exotic providers) keep the converter's output. | ||
| */ | ||
| internal fun bindModel(options: ChatOptions, model: String?): ChatOptions = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the model is null or blank, you'd just be calling .mutate().build(), which clones the options unchanged — same result as returning options directly. |
||
| if (model.isNullOrBlank()) options else options.mutate().model(model).build() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How was it sourced? sufficiently representative?