diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModel.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModel.kt index 199e1040f..f47ca9ec2 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModel.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModel.kt @@ -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 + val strippedOptions = UnsupportedRequestParameterRetry.stripParameter(options, parameter) + // 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)) } // ------------------------------------------------------------------- diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSender.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSender.kt index a3e0f89dc..613031b30 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSender.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSender.kt @@ -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) } + 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() } } diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetry.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetry.kt new file mode 100644 index 000000000..2fdfa1b88 --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetry.kt @@ -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) + "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 = + parameter.lowercase().replace("_", "").replace("-", "").replace(" ", "") +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModelTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModelTest.kt index 0d7bf62b7..d250e0d32 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModelTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/InstrumentedChatModelTest.kt @@ -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()) } throws rejection andThen expectedResponse + + val result = instrumentedModel.call(prompt) + + assertThat(result).isSameAs(expectedResponse) + val prompts = mutableListOf() + 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." + ) + + assertThrows { + instrumentedModel.call(prompt) + } + verify(exactly = 1) { delegate.call(any()) } + } + + @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 { + instrumentedModel.call(prompt) + } + verify(exactly = 1) { delegate.call(any()) } + } } @Nested diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSenderTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSenderTest.kt index 8c0a005cc..e2d400c84 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSenderTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/SpringAiLlmMessageSenderTest.kt @@ -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 { + 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() + val generation = Generation(SpringAiAssistantMessage("done")) + val mockMetadata = mockk { + every { usage } returns null + } + val chatResponse = mockk { + every { result } returns generation + every { results } returns listOf(generation) + every { metadata } returns mockMetadata + } + val chatModel = mockk { + 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 diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetryTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetryTest.kt new file mode 100644 index 000000000..e67696a5f --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/springai/UnsupportedRequestParameterRetryTest.kt @@ -0,0 +1,113 @@ +/* + * 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.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.springframework.ai.model.tool.ToolCallingChatOptions + +class UnsupportedRequestParameterRetryTest { + + @Nested + inner class ExtractUnsupportedParameter { + + @Test + fun `parses structured OpenAI JSON error param from Spring AI message`() { + val message = + """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" } }""" + val param = UnsupportedRequestParameterRetry.extractUnsupportedParameter( + RuntimeException(message) + ) + assertThat(param).isEqualTo("temperature") + } + + @Test + fun `fails closed on prose-only body without structured error param`() { + // Fail-closed *contract* test (intentionally kept, not leftover unstructured + // matching): we do NOT match English prose. Only structured error.param + // drives a strip/retry. + val message = + "400 Unsupported value: 'temperature' does not support 0.8 with this model. Only the default (1) value is supported." + val param = UnsupportedRequestParameterRetry.extractUnsupportedParameter( + RuntimeException(message) + ) + assertThat(param).isNull() + } + + @Test + fun `walks cause chain`() { + val root = RuntimeException( + """400 - {"error":{"param":"top_p","code":"unsupported_value","message":"..."}}""" + ) + val wrapped = RuntimeException("call failed", root) + assertThat(UnsupportedRequestParameterRetry.extractUnsupportedParameter(wrapped)) + .isEqualTo("top_p") + } + + @Test + fun `returns null for unrelated errors`() { + val param = UnsupportedRequestParameterRetry.extractUnsupportedParameter( + RuntimeException("rate limit exceeded") + ) + assertThat(param).isNull() + } + + @Test + fun `returns null for structured body with non-retryable code`() { + val message = + """400 - {"error":{"param":"messages","code":"context_length_exceeded","message":"too long"}}""" + assertThat( + UnsupportedRequestParameterRetry.extractUnsupportedParameter(RuntimeException(message)) + ).isNull() + } + } + + @Nested + inner class StripParameter { + + @Test + fun `clears temperature while keeping other options`() { + val options = ToolCallingChatOptions.builder() + .temperature(0.8) + .topP(0.9) + .maxTokens(100) + .build() + + val stripped = UnsupportedRequestParameterRetry.stripParameter(options, "temperature") + + assertThat(stripped).isNotNull + // Use property names: Spring AI @NullMarked getters NPE in Kotlin when value is null. + assertThat(stripped).extracting("temperature").isNull() + assertThat(stripped).extracting("topP").isEqualTo(0.9) + assertThat(stripped).extracting("maxTokens").isEqualTo(100) + } + + @Test + fun `clears top_p by snake_case name`() { + val options = ToolCallingChatOptions.builder().topP(0.5).temperature(0.2).build() + val stripped = UnsupportedRequestParameterRetry.stripParameter(options, "top_p") + assertThat(stripped).extracting("topP").isNull() + assertThat(stripped).extracting("temperature").isEqualTo(0.2) + } + + @Test + fun `returns null for unknown parameter`() { + val options = ToolCallingChatOptions.builder().temperature(0.5).build() + assertThat(UnsupportedRequestParameterRetry.stripParameter(options, "seed")).isNull() + } + } +} diff --git a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoader.kt b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoader.kt index c86ec3c20..01029f6b5 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoader.kt +++ b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoader.kt @@ -63,7 +63,8 @@ data class OpenAiModelDefinitions( * @property maxTokens maximum tokens for completion (default 16384) * @property temperature sampling temperature (default 1.0) * @property topP nucleus sampling parameter - * @property specialHandling optional special handling configuration (e.g., GPT-5 temperature) + * @property specialHandling optional sampling-parameter support flags (YAML `special_handling`) + * @property apiFormat wire format this model is served over (Chat Completions vs Responses) * @property nativeSupport optional provider-native support metadata */ data class OpenAiModelDefinition( @@ -75,7 +76,11 @@ data class OpenAiModelDefinition( val maxTokens: Int = 16384, val temperature: Double = 1.0, val topP: Double? = null, - val specialHandling: SpecialHandlingConfiguration? = null, + /** + * YAML key remains `special_handling` (Jackson property name). Type is + * [SupportFeaturesConfiguration] — which sampling parameters this model accepts. + */ + val specialHandling: SupportFeaturesConfiguration? = null, val apiFormat: OpenAiApiFormat = OpenAiApiFormat.CHAT_COMPLETIONS, @param:JsonAlias("native-support") override val nativeSupport: NativeSupport? = null, @@ -99,14 +104,27 @@ enum class OpenAiApiFormat { } /** - * Special handling configuration for models with unique requirements. + * Which sampling / request parameters a model supports. * - * @property supportsTemperature whether the model supports temperature adjustment. A model that - * does not is served by `Gpt5ChatOptionsConverter`, which sends no sampling parameter at all — - * the GPT-5 models that refuse `temperature` refuse `top_p` and both penalties with it. + * Sourced from `special_handling` in model YAML (property [OpenAiModelDefinition.specialHandling]). + * Defaults preserve historical "all parameters supported" behavior when a flag is omitted. + * Mapped at registration time to [com.embabel.agent.openai.ModelCapabilities]. + * + * Formerly named `SpecialHandlingConfiguration`; renamed for clarity (YAML key unchanged). + * + * @property supportsTemperature whether the model supports non-default temperature + * @property supportsTopP whether the model supports top_p + * @property supportsFrequencyPenalty whether the model supports frequency_penalty + * @property supportsPresencePenalty whether the model supports presence_penalty + * @property usesMaxCompletionTokens when true, map the token limit to `max_completion_tokens` + * (GPT-5 family rejects `max_tokens` for presence alone) */ -data class SpecialHandlingConfiguration( - val supportsTemperature: Boolean = true +data class SupportFeaturesConfiguration( + val supportsTemperature: Boolean = true, + val supportsTopP: Boolean = true, + val supportsFrequencyPenalty: Boolean = true, + val supportsPresencePenalty: Boolean = true, + val usesMaxCompletionTokens: Boolean = false, ) /** diff --git a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfig.kt b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfig.kt index ac4e73b73..118151f09 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfig.kt +++ b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfig.kt @@ -17,10 +17,13 @@ package com.embabel.agent.config.models.openai import com.embabel.agent.api.models.OpenAiModels import com.embabel.agent.config.models.openai.OpenAiProperties.Companion.PREFIX +import com.embabel.agent.openai.CapabilityAwareOpenAiOptionsConverter import com.embabel.agent.openai.Gpt5ChatOptionsConverter +import com.embabel.agent.openai.ModelCapabilities import com.embabel.agent.openai.OpenAiCompatibleModelFactory import com.embabel.agent.openai.StandardOpenAiOptionsConverter import com.embabel.agent.spi.LlmService +import com.embabel.common.ai.model.OptionsConverter import com.embabel.agent.spi.common.RetryProperties import com.embabel.agent.spi.support.springai.SpringAiLlmService import com.embabel.agent.spi.support.springai.SpringAiNativeStructuredOutputConfigurer @@ -204,12 +207,10 @@ class OpenAiModelsConfig( * Uses custom SpringAiLlm constructor when pricing model is not available. */ private fun createOpenAiLlm(modelDef: OpenAiModelDefinition): LlmService<*> { - // Determine the appropriate options converter based on model configuration - val optionsConverter = if (modelDef.specialHandling?.supportsTemperature == false) { - Gpt5ChatOptionsConverter - } else { - StandardOpenAiOptionsConverter - } + // Capability-aware converter from YAML special_handling (warn-and-drop, never throw). + // Canonical DEFAULT / GPT5_FAMILY map back to the shared object aliases so identity + // checks in tests and equals-based wiring stay stable. + val optionsConverter = optionsConverterFor(modelDef.specialHandling.toModelCapabilities()) // Transport is declared per model, like the converter above: most models speak Chat // Completions, the *-pro family is served only over the Responses API. @@ -261,4 +262,23 @@ class OpenAiModelsConfig( pricingModel = pricing, ) } + + private fun SupportFeaturesConfiguration?.toModelCapabilities(): ModelCapabilities { + if (this == null) { + return ModelCapabilities.DEFAULT + } + return ModelCapabilities( + supportsTemperature = supportsTemperature, + supportsTopP = supportsTopP, + supportsFrequencyPenalty = supportsFrequencyPenalty, + supportsPresencePenalty = supportsPresencePenalty, + usesMaxCompletionTokens = usesMaxCompletionTokens, + ) + } + + private fun optionsConverterFor(capabilities: ModelCapabilities): OptionsConverter = when (capabilities) { + ModelCapabilities.DEFAULT -> StandardOpenAiOptionsConverter + ModelCapabilities.GPT5_FAMILY -> Gpt5ChatOptionsConverter + else -> CapabilityAwareOpenAiOptionsConverter(capabilities) + } } diff --git a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/resources/models/openai-models.yml b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/resources/models/openai-models.yml index fd084952c..4d70f0c5a 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/resources/models/openai-models.yml +++ b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/main/resources/models/openai-models.yml @@ -58,6 +58,10 @@ models: usd_per1m_output_tokens: 30.00 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt56terra" model_id: "gpt-5.6-terra" @@ -69,6 +73,10 @@ models: usd_per1m_output_tokens: 12.00 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt56luna" model_id: "gpt-5.6-luna" @@ -80,6 +88,10 @@ models: usd_per1m_output_tokens: 1.20 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5.5 FAMILY (June 2026) @@ -96,6 +108,10 @@ models: usd_per1m_output_tokens: 30.00 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt55pro" model_id: "gpt-5.5-pro" @@ -108,6 +124,10 @@ models: usd_per1m_output_tokens: 180.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5.4 FAMILY (March 2026) @@ -125,6 +145,10 @@ models: usd_per1m_output_tokens: 15.00 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt54mini" model_id: "gpt-5.4-mini" @@ -136,6 +160,10 @@ models: usd_per1m_output_tokens: 4.500 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt54nano" model_id: "gpt-5.4-nano" @@ -147,6 +175,10 @@ models: usd_per1m_output_tokens: 1.25 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt54pro" model_id: "gpt-5.4-pro" @@ -159,6 +191,10 @@ models: usd_per1m_output_tokens: 180.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5.3 CHAT (March 2026) @@ -176,6 +212,10 @@ models: usd_per1m_output_tokens: 14.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5.2 FAMILY (December 2025) @@ -191,6 +231,10 @@ models: usd_per1m_output_tokens: 14.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt52pro" model_id: "gpt-5.2-pro" @@ -203,6 +247,10 @@ models: usd_per1m_output_tokens: 168.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5.1 FAMILY (November 2025) @@ -218,6 +266,10 @@ models: usd_per1m_output_tokens: 10.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-5 FAMILY (August 2025) @@ -233,6 +285,10 @@ models: usd_per1m_output_tokens: 10.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt5mini" model_id: "gpt-5-mini" @@ -244,6 +300,10 @@ models: usd_per1m_output_tokens: 2.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt5nano" model_id: "gpt-5-nano" @@ -255,6 +315,10 @@ models: usd_per1m_output_tokens: 0.40 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: "gpt5pro" model_id: "gpt-5-pro" @@ -267,6 +331,10 @@ models: usd_per1m_output_tokens: 120.0 special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true # ======================================== # GPT-4.1 FAMILY (April 2025 - 1M token context window) @@ -280,6 +348,8 @@ models: pricing_model: usd_per1m_input_tokens: 2.0 usd_per1m_output_tokens: 8.0 + special_handling: + supports_temperature: false - name: "gpt41mini" model_id: "gpt-4.1-mini" @@ -289,6 +359,8 @@ models: pricing_model: usd_per1m_input_tokens: 0.40 usd_per1m_output_tokens: 1.60 + special_handling: + supports_temperature: false - name: "gpt41nano" model_id: "gpt-4.1-nano" @@ -298,6 +370,8 @@ models: pricing_model: usd_per1m_input_tokens: 0.10 usd_per1m_output_tokens: 0.40 + special_handling: + supports_temperature: false # ======================================== # GPT-4o FAMILY (retained for audio support only) diff --git a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoaderTest.kt b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoaderTest.kt index 61feb235d..5b4432c53 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoaderTest.kt +++ b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelLoaderTest.kt @@ -71,12 +71,12 @@ class OpenAiModelLoaderTest { } /** - * The whole GPT-5 line rejects any temperature but the default, and `createOpenAiLlm` keys - * the [Gpt5ChatOptionsConverter] selection off this flag. A model shipped without it sends - * a temperature OpenAI will reject. + * The whole GPT-5 line rejects sampling parameters and `max_tokens`. Catalog flags drive + * [com.embabel.agent.openai.CapabilityAwareOpenAiOptionsConverter] / the GPT-5 alias + * (warn-and-drop, never throw). A model shipped without these sends fields OpenAI rejects. */ @Test - fun `every GPT-5 model declares that it does not support temperature`() { + fun `every GPT-5 model declares sampling and max_completion_tokens capabilities`() { val gpt5Models = shippedCatalogue.models.filter { it.name.startsWith("gpt5") } assertTrue(gpt5Models.isNotEmpty(), "Should have GPT-5 models") @@ -85,7 +85,56 @@ class OpenAiModelLoaderTest { assertEquals( false, model.specialHandling?.supportsTemperature, - "GPT-5 models should not support temperature adjustment", + "GPT-5 models should not support temperature adjustment (${model.modelId})", + ) + assertEquals( + false, + model.specialHandling?.supportsTopP, + "GPT-5 models should not support top_p (${model.modelId})", + ) + assertEquals( + false, + model.specialHandling?.supportsFrequencyPenalty, + "GPT-5 models should not support frequency_penalty (${model.modelId})", + ) + assertEquals( + false, + model.specialHandling?.supportsPresencePenalty, + "GPT-5 models should not support presence_penalty (${model.modelId})", + ) + assertEquals( + true, + model.specialHandling?.usesMaxCompletionTokens, + "GPT-5 models map the token limit to max_completion_tokens (${model.modelId})", + ) + } + } + + /** + * GPT-4.1 family is temperature-restricted but still accepts classic `max_tokens` and + * other sampling fields (unlike the GPT-5 family). + */ + @Test + fun `every GPT-4_1 model declares temperature-only special handling`() { + val gpt41Models = shippedCatalogue.models.filter { it.name.startsWith("gpt41") } + assertTrue(gpt41Models.isNotEmpty(), "Should have GPT-4.1 models") + + gpt41Models.forEach { model -> + assertNotNull(model.specialHandling, "GPT-4.1 models should have special handling") + assertEquals( + false, + model.specialHandling?.supportsTemperature, + "GPT-4.1 models should not support temperature adjustment (${model.modelId})", + ) + assertEquals( + true, + model.specialHandling?.supportsTopP ?: true, + "GPT-4.1 still accepts top_p (${model.modelId})", + ) + assertEquals( + false, + model.specialHandling?.usesMaxCompletionTokens ?: false, + "GPT-4.1 still uses max_tokens (${model.modelId})", ) } } diff --git a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfigRoutingTest.kt b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfigRoutingTest.kt index 9398b688f..ce1f02a46 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfigRoutingTest.kt +++ b/embabel-agent-autoconfigure/models/embabel-agent-openai-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/openai/OpenAiModelsConfigRoutingTest.kt @@ -47,6 +47,10 @@ class OpenAiModelsConfigRoutingTest { api_format: RESPONSES special_handling: supports_temperature: false + supports_top_p: false + supports_frequency_penalty: false + supports_presence_penalty: false + uses_max_completion_tokens: true - name: chatModel model_id: gpt-4o-mini """.trimIndent() diff --git a/embabel-agent-openai/src/main/kotlin/com/embabel/agent/openai/converters.kt b/embabel-agent-openai/src/main/kotlin/com/embabel/agent/openai/converters.kt index 52c98d827..87370a986 100644 --- a/embabel-agent-openai/src/main/kotlin/com/embabel/agent/openai/converters.kt +++ b/embabel-agent-openai/src/main/kotlin/com/embabel/agent/openai/converters.kt @@ -22,73 +22,195 @@ import org.springframework.ai.chat.prompt.ChatOptions import org.springframework.ai.openai.OpenAiChatOptions /** - * Options converter for GPT-5 models, which take a token limit and no other hyperparameter. + * Shared wire/param names for the sampling fields [CapabilityAwareOpenAiOptionsConverter] + * checks against [ModelCapabilities]. One place to keep the strings used in warnings and + * the capability flags in sync. + */ +object SamplingParameterNames { + const val TEMPERATURE = "temperature" + const val TOP_P = "topP" + const val PRESENCE_PENALTY = "presencePenalty" + const val FREQUENCY_PENALTY = "frequencyPenalty" +} + +/** + * Declarative sampling / token-limit capabilities for a model. * - * `temperature`, `top_p`, `presence_penalty` and `frequency_penalty` are refused as a block — each - * for its presence alone, exactly like `max_tokens`: + * Sourced from model YAML (`special_handling` / + * [com.embabel.agent.config.models.openai.SupportFeaturesConfiguration]) — the LLM model + * database is the source of truth for which request params a model accepts. * - * `400 Unsupported parameter: 'top_p' is not supported with this model.` + * Use this type for **proactive** checks before building [LlmOptions], or rely on + * [CapabilityAwareOpenAiOptionsConverter] to **warn and drop** unsupported values at + * conversion time (same strategy as the GPT-5 path landed in #1874 — never fail a + * request over a sampling parameter that was never essential). * - * so none of them is sent. The request carries the model id and, when the caller asked for one, a - * limit on `max_completion_tokens`. Nothing else. + * Consistent with the GPT-5 findings: some models reject sampling fields for their + * presence alone, and the GPT-5 family maps the token limit to `max_completion_tokens` + * rather than `max_tokens`. + */ +data class ModelCapabilities( + val supportsTemperature: Boolean = true, + val supportsTopP: Boolean = true, + val supportsFrequencyPenalty: Boolean = true, + val supportsPresencePenalty: Boolean = true, + /** + * When true, map [LlmOptions.maxTokens] → [OpenAiChatOptions.maxCompletionTokens] + * and leave `maxTokens` unset. The GPT-5 family rejects `max_tokens` for presence alone. + */ + val usesMaxCompletionTokens: Boolean = false, +) { + companion object { + @JvmField + val DEFAULT = ModelCapabilities() + + /** Temperature-restricted only (e.g. GPT-4.1 family) — still uses `max_tokens`. */ + @JvmField + val WITHOUT_TEMPERATURE = ModelCapabilities(supportsTemperature = false) + + /** + * Conservative GPT-5 baseline: no sampling parameters and `max_completion_tokens`. + * Prefer per-model YAML when a specific GPT-5 tier still accepts top_p / penalties + * (e.g. 5.1 / 5.2 / 5.4 tiers verified live). + */ + @JvmField + val GPT5_FAMILY = ModelCapabilities( + supportsTemperature = false, + supportsTopP = false, + supportsFrequencyPenalty = false, + supportsPresencePenalty = false, + usesMaxCompletionTokens = true, + ) + } +} + +/** + * Single OpenAI options converter that consults [ModelCapabilities]. + * + * **Warn-and-drop (not throw):** if the model does not support a sampling parameter and + * the caller set a value that would be sent to the API, the value is omitted and a + * warning is logged. Refusing the call outright would cost the caller an answer over a + * parameter that was never essential (cost argument from #1852 / #1874 review). + * + * Default temperature (`1.0`) on temperature-restricted models is omitted without warning + * (providers often only reject non-default temperature; 1.0 is what the model uses anyway). + * Null parameters are never sent. + * + * Token limit: when [ModelCapabilities.usesMaxCompletionTokens] is true, the limit is + * placed on `maxCompletionTokens` and `maxTokens` is left unset (GPT-5 family). * - * The refusal is per model rather than per family, verified against the live API on 2026-08-05: - * gpt-5, gpt-5-mini, gpt-5-nano, gpt-5.3-chat, gpt-5.5 and the gpt-5.6 tiers refuse all four, while - * gpt-5.1, gpt-5.2 and the gpt-5.4 tiers accept them. Sending none of them to any of them keeps one - * converter for the whole family: the models that would have honoured a `topP` lose it the way they - * already lost `temperature`, and no caller gets a 400 for a parameter that was never essential. + * ### Request execution chain + * + * ``` + * LlmOptions (caller sets e.g. temperature) + * → OptionsConverter.convertOptions(options, model) + * → CapabilityAwareOpenAiOptionsConverter.convertOptions + * - capability supports the field? → pass value through to OpenAiChatOptions + * - capability rejects it → warn (if non-default/non-null) and omit + * → Prompt.options passed to ChatModel.call + * → provider HTTP call + * - if a capability flag was missing/stale and the provider still rejects the field, + * InstrumentedChatModel parses structured error.param and retries once with that + * field stripped — a **safety net**, not the primary path. + * ``` */ -object Gpt5ChatOptionsConverter : OptionsConverter { +class CapabilityAwareOpenAiOptionsConverter( + private val capabilities: ModelCapabilities = ModelCapabilities.DEFAULT, +) : OptionsConverter { + + private val logger = loggerFor() override fun convertOptions(options: LlmOptions, model: String): ChatOptions { - warnAboutIgnoredParameters(options) - return OpenAiChatOptions.builder() - .model(model) - // Not maxTokens: the GPT-5 family rejects `max_tokens` with - // "Unsupported parameter ... use 'max_completion_tokens' instead", and refuses the - // request for the field's presence alone. - .maxCompletionTokens(options.maxTokens) - .build() + require(model.isNotBlank()) { "model id must not be blank" } + + warnAboutIgnoredParameters(options, model) + + val builder = OpenAiChatOptions.builder().model(model) + + if (capabilities.usesMaxCompletionTokens) { + // GPT-5 family: max_tokens is a 400 for presence alone; use max_completion_tokens. + options.maxTokens?.let { builder.maxCompletionTokens(it) } + } else { + builder.maxTokens(options.maxTokens) + } + + if (capabilities.supportsTemperature) { + builder.temperature(options.temperature) + } + // else omit — already warned when non-default + + if (capabilities.supportsTopP) { + builder.topP(options.topP) + } + + if (capabilities.supportsPresencePenalty) { + builder.presencePenalty(options.presencePenalty) + } + + if (capabilities.supportsFrequencyPenalty) { + builder.frequencyPenalty(options.frequencyPenalty) + } + + return builder.build() } /** - * Dropping these silently would read as the model ignoring them; refusing the call outright - * would cost the caller an answer over a parameter that was never essential. + * Dropping unsupported sampling values silently would read as the model ignoring them; + * throwing would cost the caller an answer. Warn once per conversion when anything is dropped. */ - private fun warnAboutIgnoredParameters(options: LlmOptions) { + private fun warnAboutIgnoredParameters(options: LlmOptions, model: String) { val ignored = buildList { - // The default temperature is what the model uses anyway, so asking for it is not a - // request that went unanswered. - options.temperature?.takeIf { it != 1.0 }?.let { add("temperature=$it") } - options.topP?.let { add("topP=$it") } - options.presencePenalty?.let { add("presencePenalty=$it") } - options.frequencyPenalty?.let { add("frequencyPenalty=$it") } + if (!capabilities.supportsTemperature) { + // Default temperature is what the model uses anyway — not an unanswered request. + options.temperature?.takeIf { it != DEFAULT_TEMPERATURE }?.let { + add("${SamplingParameterNames.TEMPERATURE}=$it") + } + } + if (!capabilities.supportsTopP) { + options.topP?.let { add("${SamplingParameterNames.TOP_P}=$it") } + } + if (!capabilities.supportsPresencePenalty) { + options.presencePenalty?.let { add("${SamplingParameterNames.PRESENCE_PENALTY}=$it") } + } + if (!capabilities.supportsFrequencyPenalty) { + options.frequencyPenalty?.let { add("${SamplingParameterNames.FREQUENCY_PENALTY}=$it") } + } } if (ignored.isNotEmpty()) { - loggerFor().warn( - "This model rejects sampling parameters outright, so the following are ignored rather than sent: {}", + logger.warn( + "Model '{}' rejects some sampling parameters, so the following are ignored rather than sent: {}", + model, ignored.joinToString(", "), ) } } + + companion object { + private const val DEFAULT_TEMPERATURE = 1.0 + } } /** - * Standard options converter for OpenAI models that support all parameters. + * Options converter for the GPT-5 family baseline: no sampling parameters and + * `max_completion_tokens` (see [ModelCapabilities.GPT5_FAMILY]). * - * Keeps `maxTokens`: the models routed here — the GPT-4 family and OpenAI-compatible providers — - * accept it, and several do not know `max_completion_tokens` at all. + * Prefer constructing [CapabilityAwareOpenAiOptionsConverter] with per-model + * [ModelCapabilities] from YAML when a specific tier still accepts top_p / penalties. + */ +object Gpt5ChatOptionsConverter : OptionsConverter { + private val delegate = CapabilityAwareOpenAiOptionsConverter(ModelCapabilities.GPT5_FAMILY) + + override fun convertOptions(options: LlmOptions, model: String): ChatOptions = + delegate.convertOptions(options, model) +} + +/** + * Standard options converter for OpenAI models that support all sampling parameters + * and the classic `max_tokens` field. */ object StandardOpenAiOptionsConverter : OptionsConverter { + private val delegate = CapabilityAwareOpenAiOptionsConverter(ModelCapabilities.DEFAULT) - override fun convertOptions(options: LlmOptions, model: String): ChatOptions { - return OpenAiChatOptions.builder() - .model(model) - .temperature(options.temperature) - .topP(options.topP) - .maxTokens(options.maxTokens) - .presencePenalty(options.presencePenalty) - .frequencyPenalty(options.frequencyPenalty) - .build() - } + override fun convertOptions(options: LlmOptions, model: String): ChatOptions = + delegate.convertOptions(options, model) } diff --git a/embabel-agent-openai/src/test/kotlin/com/embabel/agent/config/models/openai/Gpt5ChatOptionsConverterTest.kt b/embabel-agent-openai/src/test/kotlin/com/embabel/agent/config/models/openai/Gpt5ChatOptionsConverterTest.kt index 4336bc1cc..9663d4933 100644 --- a/embabel-agent-openai/src/test/kotlin/com/embabel/agent/config/models/openai/Gpt5ChatOptionsConverterTest.kt +++ b/embabel-agent-openai/src/test/kotlin/com/embabel/agent/config/models/openai/Gpt5ChatOptionsConverterTest.kt @@ -24,8 +24,16 @@ import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test import org.springframework.ai.openai.OpenAiChatOptions -class Gpt5ChatOptionsConverterTest( -) { +/** + * [Gpt5ChatOptionsConverter] is the conservative GPT-5 family alias: + * no sampling parameters, token limit on `max_completion_tokens`. + * + * Per-model YAML may be looser for tiers that still accept top_p / penalties + * (e.g. 5.1 / 5.2 / 5.4); those go through [com.embabel.agent.openai.CapabilityAwareOpenAiOptionsConverter] + * with flags from `special_handling`. Unsupported values are warned and dropped + * (not thrown) — same strategy as #1874. + */ +class Gpt5ChatOptionsConverterTest { @Test fun `ignores temperature`() { @@ -58,8 +66,6 @@ class Gpt5ChatOptionsConverterTest( val options = Gpt5ChatOptionsConverter.convertOptions(llmo, "test-model") - // Spring AI 2.0's OpenAiChatOptions package is @NullMarked, so Kotlin treats these getters - // as returning non-null — read into nullable locals to bypass, as in `ignores temperature`. val topP: Double? = options.topP val presencePenalty: Double? = options.presencePenalty val frequencyPenalty: Double? = options.frequencyPenalty @@ -71,32 +77,28 @@ class Gpt5ChatOptionsConverterTest( @Disabled("We not support thinking effort yet") @Test fun `supports thinking effort`() { - } @Test fun `handles temperature equal to 1_0 without warning`() { val llmo = LlmOptions().withTemperature(temperature = 1.0) val options = Gpt5ChatOptionsConverter.convertOptions(llmo, "test-model") - assertNull(options.temperature, "Temperature 1.0 should be ignored silently") + val temperature: Double? = options.temperature + assertNull(temperature, "Temperature 1.0 should be ignored silently") } @Test fun `handles null temperature`() { val llmo = LlmOptions() val options = Gpt5ChatOptionsConverter.convertOptions(llmo, "test-model") - assertNull(options.temperature, "Null temperature should remain null") + val temperature: Double? = options.temperature + assertNull(temperature, "Null temperature should remain null") } /** * The GPT-5 family rejects `max_tokens` outright: * `400 Unsupported parameter: 'max_tokens' is not supported with this model. * Use 'max_completion_tokens' instead.` - * - * Spring AI serialises [org.springframework.ai.openai.OpenAiChatOptions.maxTokens] to the - * former and `maxCompletionTokens` to the latter, so the limit has to be carried on the - * second field — and the first must stay unset, or the request is refused for its presence - * alone. */ @Test fun `carries a token limit on maxCompletionTokens, which is the field GPT-5 accepts`() { diff --git a/embabel-agent-openai/src/test/kotlin/com/embabel/agent/openai/CapabilityAwareOpenAiOptionsConverterTest.kt b/embabel-agent-openai/src/test/kotlin/com/embabel/agent/openai/CapabilityAwareOpenAiOptionsConverterTest.kt new file mode 100644 index 000000000..f539acc5f --- /dev/null +++ b/embabel-agent-openai/src/test/kotlin/com/embabel/agent/openai/CapabilityAwareOpenAiOptionsConverterTest.kt @@ -0,0 +1,243 @@ +/* + * 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.openai + +import com.embabel.common.ai.model.LlmOptions +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.springframework.ai.openai.OpenAiChatOptions + +class CapabilityAwareOpenAiOptionsConverterTest { + + @Nested + inner class DefaultCapabilities { + + @Test + fun `includes all sampling parameters and maxTokens`() { + val options = LlmOptions() + .withTemperature(0.7) + .withTopP(0.9) + .withMaxTokens(1000) + .withPresencePenalty(0.5) + .withFrequencyPenalty(0.3) + + val result = (CapabilityAwareOpenAiOptionsConverter().convertOptions(options, "test-model") as OpenAiChatOptions) + + assertEquals(0.7, result.temperature) + assertEquals(0.9, result.topP) + assertEquals(1000, result.maxTokens) + val maxCompletionTokens: Int? = result.maxCompletionTokens + assertNull(maxCompletionTokens) + assertEquals(0.5, result.presencePenalty) + assertEquals(0.3, result.frequencyPenalty) + } + } + + @Nested + inner class TemperatureRestricted { + + private val converter = CapabilityAwareOpenAiOptionsConverter(ModelCapabilities.WITHOUT_TEMPERATURE) + + @Test + fun `drops non-default temperature rather than sending it`() { + val options = LlmOptions().withTemperature(0.8) + val result = (converter.convertOptions(options, "gpt-4.1") as OpenAiChatOptions) + val temperature: Double? = result.temperature + assertNull(temperature, "Non-default temperature must be omitted, not sent") + } + + @Test + fun `omits default temperature without error`() { + val options = LlmOptions().withTemperature(1.0) + val result = (converter.convertOptions(options, "test-model") as OpenAiChatOptions) + val temperature: Double? = result.temperature + assertNull(temperature) + } + + @Test + fun `omits null temperature without error`() { + val options = LlmOptions().withTopP(0.9).withMaxTokens(500) + val result = (converter.convertOptions(options, "test-model") as OpenAiChatOptions) + val temperature: Double? = result.temperature + assertNull(temperature) + assertEquals(0.9, result.topP) + assertEquals(500, result.maxTokens) + } + + @Test + fun `preserves other parameters when temperature omitted`() { + val options = LlmOptions() + .withTemperature(0.7) + .withTopP(0.9) + .withMaxTokens(500) + .withPresencePenalty(0.2) + .withFrequencyPenalty(0.1) + + val result = (converter.convertOptions(options, "test-model") as OpenAiChatOptions) + + val temperature: Double? = result.temperature + assertNull(temperature) + assertEquals(0.9, result.topP) + assertEquals(500, result.maxTokens) + assertEquals(0.2, result.presencePenalty) + assertEquals(0.1, result.frequencyPenalty) + } + } + + @Nested + inner class MaxCompletionTokens { + + private val converter = CapabilityAwareOpenAiOptionsConverter( + ModelCapabilities(supportsTemperature = false, usesMaxCompletionTokens = true) + ) + + @Test + fun `maps token limit to maxCompletionTokens and leaves maxTokens unset`() { + val options = LlmOptions().withMaxTokens(500) + val result = (converter.convertOptions(options, "gpt-5") as OpenAiChatOptions) + + assertEquals(500, result.maxCompletionTokens) + val maxTokens: Int? = result.maxTokens + assertNull(maxTokens) + } + + @Test + fun `sends neither token field when no limit was asked for`() { + val options = LlmOptions() + val result = (converter.convertOptions(options, "gpt-5") as OpenAiChatOptions) + + val maxCompletionTokens: Int? = result.maxCompletionTokens + val maxTokens: Int? = result.maxTokens + assertNull(maxCompletionTokens) + assertNull(maxTokens) + } + } + + @Nested + inner class MultipleRestrictedParameters { + + @Test + fun `drops all unsupported sampling params and keeps the token limit`() { + val converter = CapabilityAwareOpenAiOptionsConverter( + ModelCapabilities( + supportsTemperature = false, + supportsTopP = false, + supportsFrequencyPenalty = false, + supportsPresencePenalty = false, + usesMaxCompletionTokens = true, + ) + ) + val options = LlmOptions() + .withTemperature(0.5) + .withTopP(0.8) + .withMaxTokens(200) + .withFrequencyPenalty(0.4) + .withPresencePenalty(0.3) + + val result = (converter.convertOptions(options, "restricted-model") as OpenAiChatOptions) + + val temperature: Double? = result.temperature + val topP: Double? = result.topP + val frequencyPenalty: Double? = result.frequencyPenalty + val presencePenalty: Double? = result.presencePenalty + assertNull(temperature) + assertNull(topP) + assertNull(frequencyPenalty) + assertNull(presencePenalty) + assertEquals(200, result.maxCompletionTokens) + } + + @Test + fun `drops unsupported topP while keeping temperature`() { + val converter = CapabilityAwareOpenAiOptionsConverter( + ModelCapabilities(supportsTopP = false) + ) + val options = LlmOptions().withTemperature(0.5).withTopP(0.8) + val result = (converter.convertOptions(options, "no-top-p") as OpenAiChatOptions) + assertEquals(0.5, result.temperature) + val topP: Double? = result.topP + assertNull(topP) + } + + @Test + fun `omits all unsupported when values are null or default temperature`() { + val converter = CapabilityAwareOpenAiOptionsConverter( + ModelCapabilities( + supportsTemperature = false, + supportsTopP = false, + supportsFrequencyPenalty = false, + supportsPresencePenalty = false, + usesMaxCompletionTokens = true, + ) + ) + val options = LlmOptions().withTemperature(1.0).withMaxTokens(200) + val result = (converter.convertOptions(options, "test-model") as OpenAiChatOptions) + + val temperature: Double? = result.temperature + val topP: Double? = result.topP + val frequencyPenalty: Double? = result.frequencyPenalty + val presencePenalty: Double? = result.presencePenalty + assertNull(temperature) + assertNull(topP) + assertNull(frequencyPenalty) + assertNull(presencePenalty) + assertEquals(200, result.maxCompletionTokens) + val maxTokens: Int? = result.maxTokens + assertNull(maxTokens) + } + } + + @Nested + inner class BackwardCompatibleAliases { + + @Test + fun `Gpt5ChatOptionsConverter drops non-default temperature`() { + val options = LlmOptions().withTemperature(0.5) + val result = (Gpt5ChatOptionsConverter.convertOptions(options, "test-model") as OpenAiChatOptions) + val temperature: Double? = result.temperature + assertNull(temperature) + } + + @Test + fun `Gpt5ChatOptionsConverter drops topP when set`() { + val options = LlmOptions().withTopP(0.9) + val result = (Gpt5ChatOptionsConverter.convertOptions(options, "test-model") as OpenAiChatOptions) + val topP: Double? = result.topP + assertNull(topP) + } + + @Test + fun `Gpt5ChatOptionsConverter omits defaults and uses maxCompletionTokens`() { + val options = LlmOptions().withTemperature(1.0).withMaxTokens(300) + val result = (Gpt5ChatOptionsConverter.convertOptions(options, "test-model") as OpenAiChatOptions) + val temperature: Double? = result.temperature + assertNull(temperature) + assertEquals(300, result.maxCompletionTokens) + val maxTokens: Int? = result.maxTokens + assertNull(maxTokens) + } + + @Test + fun `StandardOpenAiOptionsConverter includes temperature and maxTokens`() { + val options = LlmOptions().withTemperature(0.5).withMaxTokens(100) + val result = (StandardOpenAiOptionsConverter.convertOptions(options, "test-model") as OpenAiChatOptions) + assertEquals(0.5, result.temperature) + assertEquals(100, result.maxTokens) + } + } +}