Skip to content
Open
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
Expand Up @@ -72,7 +72,45 @@ internal class InstrumentedChatModel(
llmRequestEvent.chatModelCallEvent(prompt)
)

return delegate.call(prompt)
return try {
delegate.call(prompt)
} catch (ex: RuntimeException) {
retryWithoutUnsupportedParameter(prompt, ex)
}
}

/**
* Safety net when a model rejects a request field that slipped past capability-aware
* option conversion. Parses structured OpenAI-style `error.param`, strips that field
* from [Prompt.options], and retries once.
*
* Fail closed (rethrow [ex]) when:
* - no structured `error.param` in the exception chain
* - prompt has no options (nothing to strip → cannot recover)
* - [UnsupportedRequestParameterRetry.stripParameter] returns null because the field
* name is **unknown** to our portable mutator map — we never invent options.
* Note: a *known* field that is already null still returns a built copy (retry once);
* null from strip means "cannot strip this name", not "field was already omitted".
*
* Prefer [com.embabel.agent.openai.ModelCapabilities] + capability-aware conversion
* (warn-and-drop) so restricted fields never reach the wire. Keep this path thin:
* extend YAML capabilities first.
*/
private fun retryWithoutUnsupportedParameter(prompt: Prompt, ex: RuntimeException): ChatResponse {
val parameter = UnsupportedRequestParameterRetry.extractUnsupportedParameter(ex)
?: throw ex
val options = prompt.options ?: throw ex
Comment thread
arimu1 marked this conversation as resolved.
val strippedOptions = UnsupportedRequestParameterRetry.stripParameter(options, parameter)
Comment thread
arimu1 marked this conversation as resolved.
// null = unknown field name only (known-but-already-null still yields a copy)
if (strippedOptions == null) {
throw ex
}
logger.warn(
"Model rejected unsupported request parameter '{}'; retrying once without it. Original error: {}",
parameter,
ex.message,
)
return delegate.call(Prompt(prompt.instructions, strippedOptions))
}

// -------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,19 +209,22 @@ internal class SpringAiLlmMessageSender(
.build()
}

// Fallback: Create generic ToolCallingChatOptions.
// Fallback: create generic ToolCallingChatOptions.
// Only copy parameters that are present so we never re-introduce values the
// options converter intentionally omitted (e.g. temperature on restricted models).
// We handle tools ourselves in DefaultToolLoop. Spring AI 2.0 GA removed the per-request
// internalToolExecutionEnabled flag; internal execution is disabled on the ChatModel.
return ToolCallingChatOptions.builder()
.model(chatOptions.model)
.temperature(chatOptions.temperature)
.maxTokens(chatOptions.maxTokens)
.topP(chatOptions.topP)
.topK(chatOptions.topK)
.frequencyPenalty(chatOptions.frequencyPenalty)
.presencePenalty(chatOptions.presencePenalty)
.stopSequences(chatOptions.stopSequences)
.toolCallbacks(toolCallbacks)
.build()
val builder = ToolCallingChatOptions.builder()
// Spring AI ChatOptions.model is nullable (String?). Only copy when non-null so we
// never force "" or invent a model id the converter intentionally left unset.
chatOptions.model?.let { builder.model(it) }
Comment thread
arimu1 marked this conversation as resolved.
chatOptions.temperature?.let { builder.temperature(it) }
chatOptions.maxTokens?.let { builder.maxTokens(it) }
chatOptions.topP?.let { builder.topP(it) }
chatOptions.topK?.let { builder.topK(it) }
chatOptions.frequencyPenalty?.let { builder.frequencyPenalty(it) }
chatOptions.presencePenalty?.let { builder.presencePenalty(it) }
chatOptions.stopSequences?.let { builder.stopSequences(it) }
return builder.toolCallbacks(toolCallbacks).build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* 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.spi.support.springai

import org.springframework.ai.chat.prompt.ChatOptions
import tools.jackson.databind.ObjectMapper

/**
* Best-effort parse + strip helpers for a one-shot HTTP-layer retry when a model rejects a
* request field that slipped past capability-aware option conversion.
*
* **Primary defence (preferred):** model YAML `special_handling` →
* [com.embabel.agent.openai.ModelCapabilities] →
* [com.embabel.agent.openai.CapabilityAwareOpenAiOptionsConverter] **warns and drops**
* unsupported sampling values **before** the call (same strategy as #1874).
*
* **This retry is only a safety net** when a capability flag is missing/incomplete
* (e.g. YAML lagging a new model restriction). Maintenance cost is deliberately low:
* only portable [ChatOptions] mutators that [stripParameter] knows how to clear are
* in scope; unknown field names return null and the original provider error is rethrown.
* Prefer extending YAML / [com.embabel.agent.openai.ModelCapabilities] over growing this map.
*
* **Extraction (OpenAI / Azure via Spring AI):** Spring AI surfaces the HTTP body as the
* exception message, typically
* `"400 - { \"error\": { \"param\": \"temperature\", \"code\": \"unsupported_value\", ... } }"`.
* We parse structured JSON `error.param` (and `error.code` when present) only.
* Prose-only bodies without `error.param` return null → caller rethrows (fail closed).
*
* **Reliability:** not guaranteed for every provider. Non-OpenAI providers rarely share
* this JSON shape. On mismatch we return null and the caller rethrows.
*/
internal object UnsupportedRequestParameterRetry {

private val objectMapper = ObjectMapper()

/**
* Codes that indicate a request field was rejected and may be stripped for a
* one-shot retry. Missing code with a present `param` still retries (OpenAI sometimes
* omits code while setting param).
*/
private val RETRYABLE_ERROR_CODES = setOf(
"unsupported_value",
"unknown_parameter",
"invalid_value",
)

fun extractUnsupportedParameter(throwable: Throwable): String? {
var current: Throwable? = throwable
while (current != null) {
val message = current.message
if (message != null) {
extractFromStructuredOpenAiBody(message)?.let { return it }
}
current = current.cause
}
return null
}

/**
* Parse Spring AI style `"NNN - {json}"` OpenAI error bodies and return
* `error.param` when it looks like an unsupported/invalid parameter error.
*/
private fun extractFromStructuredOpenAiBody(message: String): String? {
val jsonStart = message.indexOf('{')
if (jsonStart < 0) return null
return try {
val root = objectMapper.readTree(message.substring(jsonStart))
val error = root.get("error") ?: return null
val param = error.get("param")?.takeIf { !it.isNull }?.asString()?.takeIf { it.isNotBlank() }
?: return null
val code = error.get("code")?.takeIf { !it.isNull }?.asString()
// Fail closed on clearly non-parameter codes when present
if (code != null && code !in RETRYABLE_ERROR_CODES) {
return null
}
param
} catch (_: Exception) {
null
}
}

/**
* Returns a copy of [options] with [parameter] cleared, or **null** if the name is not
* a known portable [ChatOptions] field we can strip safely.
*
* Return semantics (important for [InstrumentedChatModel] retry):
* - **null** = *unknown field name* (e.g. provider-private `seed`) — caller must rethrow;
* we never invent options for fields we cannot clear.
* - **non-null copy** = known field was cleared (even if it was already null). A no-op
* strip still allows one harmless retry; it is *not* treated as failure.
*
* Scope is deliberately the portable [ChatOptions] surface (temperature, topP,
* penalties, maxTokens, topK) — the same fields [com.embabel.common.ai.model.LlmOptions]
* carries. Provider-private fields are not stripped; add a `when` arm only when Spring AI
* exposes a mutator. This is a safety net, not a product hyperparameter abstraction.
*/
fun stripParameter(options: ChatOptions, parameter: String): ChatOptions? {
require(parameter.isNotBlank()) { "parameter name must not be blank" }
val builder = options.mutate()
when (normalize(parameter)) {
"temperature" -> builder.temperature(null)
Comment thread
arimu1 marked this conversation as resolved.
"topp" -> builder.topP(null)
"frequencypenalty" -> builder.frequencyPenalty(null)
"presencepenalty" -> builder.presencePenalty(null)
"maxtokens", "maxcompletiontokens" -> builder.maxTokens(null)
"topk" -> builder.topK(null)
else -> return null
}
return builder.build()
}

/**
* Lowercases and strips separators so `top_p`, `top-p`, and `topP` all become `topp`.
* After this, a single `when` arm matches every common wire spelling.
*/
private fun normalize(parameter: String): String =
Comment thread
arimu1 marked this conversation as resolved.
parameter.lowercase().replace("_", "").replace("-", "").replace(" ", "")
}
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,63 @@ class InstrumentedChatModelTest {

verify { delegate.call(prompt) }
}

@Test
fun `retries once after unsupported temperature rejection`() {
val options = org.springframework.ai.model.tool.ToolCallingChatOptions.builder()
.temperature(0.8)
.topP(0.9)
.build()
val prompt = Prompt(listOf(UserMessage("hello")), options)
val expectedResponse: ChatResponse = mockk()
// Spring AI surfaces OpenAI body as "NNN - {json}" with structured error.param
val rejection = RuntimeException(
"""400 - { "error": { "message": "Unsupported value: 'temperature' does not support 0.8 with this model. Only the default (1) value is supported.", "type": "invalid_request_error", "param": "temperature", "code": "unsupported_value" } }"""
)
every { delegate.call(any<Prompt>()) } throws rejection andThen expectedResponse

val result = instrumentedModel.call(prompt)

assertThat(result).isSameAs(expectedResponse)
val prompts = mutableListOf<Prompt>()
verify(exactly = 2) { delegate.call(capture(prompts)) }
assertThat(prompts[0]).isSameAs(prompt)
assertThat(prompts[1].options).extracting("temperature").isNull()
assertThat(prompts[1].options).extracting("topP").isEqualTo(0.9)
}

@Test
fun `does not retry prose-only temperature rejection without error param`() {
// Fail-closed *contract* test (not leftover unstructured matching):
// prose-only rejection must not strip/retry. Only structured JSON error.param
// drives a recovery path; English wording alone rethrows after one call.
val options = org.springframework.ai.model.tool.ToolCallingChatOptions.builder()
.temperature(0.8)
.build()
val prompt = Prompt(listOf(UserMessage("hello")), options)
every { delegate.call(prompt) } throws RuntimeException(
"400 Unsupported value: 'temperature' does not support 0.8 with this model. Only the default (1) value is supported."
Comment thread
arimu1 marked this conversation as resolved.
)

assertThrows<RuntimeException> {
instrumentedModel.call(prompt)
}
verify(exactly = 1) { delegate.call(any<Prompt>()) }
}

@Test
fun `does not retry for unrelated errors`() {
val options = org.springframework.ai.model.tool.ToolCallingChatOptions.builder()
.temperature(0.8)
.build()
val prompt = Prompt(listOf(UserMessage("fail")), options)
every { delegate.call(prompt) } throws RuntimeException("rate limit exceeded")

assertThrows<RuntimeException> {
instrumentedModel.call(prompt)
}
verify(exactly = 1) { delegate.call(any<Prompt>()) }
}
}

@Nested
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,61 @@ class SpringAiLlmMessageSenderTest {
}
}

@Nested
inner class FallbackOptionsCopyTests {

@Test
fun `does not re-add omitted temperature when attaching tools`() {
// Plain ChatOptions (not ToolCallingChatOptions) forces the fallback builder path.
val chatOptions = mockk<ChatOptions> {
every { model } returns "gpt-4.1-mini"
every { temperature } returns null
every { maxTokens } returns 100
every { topP } returns 0.9
every { topK } returns null
every { frequencyPenalty } returns null
every { presencePenalty } returns null
every { stopSequences } returns null
}
val capturedPrompt = slot<Prompt>()
val generation = Generation(SpringAiAssistantMessage("done"))
val mockMetadata = mockk<ChatResponseMetadata> {
every { usage } returns null
}
val chatResponse = mockk<ChatResponse> {
every { result } returns generation
every { results } returns listOf(generation)
every { metadata } returns mockMetadata
}
val chatModel = mockk<ChatModel> {
every { call(capture(capturedPrompt)) } returns chatResponse
}
val tool = object : com.embabel.agent.api.tool.Tool {
override val definition = com.embabel.agent.api.tool.Tool.Definition(
name = "echo",
description = "Echo",
inputSchema = com.embabel.agent.api.tool.Tool.InputSchema.empty(),
)

override fun call(input: String) =
com.embabel.agent.api.tool.Tool.Result.text(input)
}
val sender = SpringAiLlmMessageSender(chatModel, chatOptions)

sender.call(
messages = listOf(UserMessage("hi")),
tools = listOf(tool),
)

val built = capturedPrompt.captured.options
assertThat(built).isNotNull
// Property extract avoids @NullMarked NPE when temperature was intentionally omitted.
assertThat(built).extracting("temperature").isNull()
assertThat(built).extracting("topP").isEqualTo(0.9)
assertThat(built).extracting("maxTokens").isEqualTo(100)
}
}

private fun testChatOptions(): ChatOptions = mockk {
every { model } returns "test-model"
every { temperature } returns null
Expand Down
Loading
Loading