From fb1ecff2affe3c73d5ca625d06bb800cd97ad56f Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 8 Aug 2026 14:57:29 +1000 Subject: [PATCH 01/12] Resolve LLM roles through an SPI so a role is not tied to one provider `embabel.models.llms` maps a role to a single model name, and a model belongs to one provider. A role therefore pins the provider it was configured against: give a deployment only an Anthropic key and every OpenAI-named role becomes unsatisfiable, which used to fail the Spring context at startup. There was also nowhere to fix it from outside. `ByRoleModelSelectionCriteria` was a static map lookup consulting no strategy, `ModelSelectionCriteria` is sealed, and `AutoLlmSelectionCriteriaResolver` takes no arguments so it cannot know the caller or the active key. The only escape was `withLlmService(...)`, which skips `ModelProvider` entirely - which is what BYOK applications reach for today, reimplementing role resolution in application code. Route `ByRole` through a `RoleResolver` chain instead. Resolvers are consulted in Ordered order and return null to delegate; the platform's own `ConfigurableRoleResolver` runs last, so existing configuration is unchanged. A resolver answers with `RoleResolution.Options` (a model plus tuning), `Credential` (a user's key - the platform looks the role up under that provider, builds the service and caches it), or `Service` (one it built itself). `ModelSelectionContext` carries who the call is for and which key is active; this is the argument `AutoLlmSelectionCriteriaResolver` lacks. Add `embabel.models.roles`, a role -> provider -> options map, for deployments whose provider is not fixed. The flat `llms` map still works and remains the right shape for a single-provider deployment. With no user key active, the provider is whichever one supplies the default LLM, so those deployments need no context at all. A role can now carry hyperparameters, not just a model name: `ModelProvider.resolveLlmOptions` merges them under whatever the caller set explicitly, and `AbstractLlmOperations` applies it once per operation, only when a role is actually named. An unsatisfiable role throws at the point of use rather than falling back to the default LLM - otherwise a role like "cheapest" silently becomes the most capable and most expensive model in the deployment. Startup is separate: it now warns instead of failing the context, so a partially keyed deployment boots and serves every role that does work. Co-Authored-By: Claude Opus 5 (1M context) --- .../spring/AgentPlatformConfiguration.kt | 8 + .../spi/support/AbstractLlmOperations.kt | 26 + .../ai/model/ConfigurableModelProvider.kt | 157 +++++- .../ai/model/ConfigurableRoleResolver.kt | 83 ++++ .../embabel/common/ai/model/ModelProvider.kt | 9 + .../common/ai/model/ModelSelectionContext.kt | 97 ++++ .../embabel/common/ai/model/RoleResolver.kt | 83 ++++ .../ai/model/RoleConfigurationBindingTest.kt | 122 +++++ .../common/ai/model/RoleResolutionTest.kt | 445 ++++++++++++++++++ .../com/embabel/common/ai/model/LlmOptions.kt | 37 ++ .../main/asciidoc/reference/llms/page.adoc | 84 ++++ 11 files changed, 1147 insertions(+), 4 deletions(-) create mode 100644 embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt create mode 100644 embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt create mode 100644 embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt create mode 100644 embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleConfigurationBindingTest.kt create mode 100644 embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/config/spring/AgentPlatformConfiguration.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/config/spring/AgentPlatformConfiguration.kt index 9ea1596c8..0b1e8d82f 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/config/spring/AgentPlatformConfiguration.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/config/spring/AgentPlatformConfiguration.kt @@ -36,8 +36,10 @@ import com.embabel.common.util.EmbabelObjectMapperHolder import com.embabel.common.ai.autoconfig.ProviderInitialization import com.embabel.common.ai.model.ConfigurableModelProvider import com.embabel.common.ai.model.ConfigurableModelProviderProperties +import com.embabel.common.ai.model.CredentialLlmServiceFactory import com.embabel.common.ai.model.EmbeddingService import com.embabel.common.ai.model.ModelProvider +import com.embabel.common.ai.model.RoleResolver import com.embabel.common.core.MobyNameGenerator import com.embabel.common.core.NameGenerator import com.embabel.common.textio.template.JinjavaTemplateRenderer @@ -194,6 +196,12 @@ class AgentPlatformConfiguration( llms = applicationContext.getBeansOfType(LlmService::class.java).values.toList(), embeddingServices = applicationContext.getBeansOfType(EmbeddingService::class.java).values.toList(), properties = properties, + // Ordered, so that one application resolver can take precedence over another + roleResolvers = applicationContext.getBeanProvider(RoleResolver::class.java) + .orderedStream().toList(), + credentialLlmServiceFactories = applicationContext + .getBeanProvider(CredentialLlmServiceFactory::class.java) + .orderedStream().toList(), ) } diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt index 27cdab5a6..dafe66964 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt @@ -34,6 +34,7 @@ import com.embabel.agent.spi.validation.ValidationPromptGenerator import com.embabel.chat.Message import com.embabel.chat.UserMessage import com.embabel.common.ai.model.AutoModelSelectionCriteria +import com.embabel.common.ai.model.ByRoleModelSelectionCriteria import com.embabel.common.ai.model.LlmOptions import com.embabel.common.ai.model.ModelProvider import com.embabel.common.ai.model.ModelSelectionCriteria @@ -144,6 +145,9 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): O { + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val (allTools, llmRequestEvent) = getToolsAndEvent( agentProcess = agentProcess, interaction = interaction, @@ -260,6 +264,9 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): Result { + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val (allTools, llmRequestEvent) = getToolsAndEvent( agentProcess = agentProcess, interaction = interaction, @@ -312,6 +319,9 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): ThinkingResponse { + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val (allTools, llmRequestEvent) = getToolsAndEvent( agentProcess = agentProcess, interaction = interaction, @@ -364,6 +374,9 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): Result> { + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val (allTools, llmRequestEvent) = getToolsAndEvent( agentProcess = agentProcess, interaction = interaction, @@ -409,6 +422,19 @@ abstract class AbstractLlmOperations( return response } + /** + * Resolve any role named by this interaction before anything reads its options: a role can + * carry hyperparameters, and which model it means depends on the provider active for this call. + * + * Interactions naming no role are returned untouched, so the common path costs nothing. + */ + private fun withRoleResolved(interaction: LlmInteraction): LlmInteraction = + if (interaction.llm.criteria is ByRoleModelSelectionCriteria) { + interaction.copy(llm = modelProvider.resolveLlmOptions(interaction.llm)) + } else { + interaction + } + protected fun chooseLlm( llmOptions: LlmOptions, ): LlmService<*> { diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 49b17c98c..aa1383e19 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -32,6 +32,23 @@ data class ConfigurableModelProviderProperties( * Map of role to LLM name. Each entry will require an LLM to be registered with the same name. May not include the default LLM. */ var llms: Map = emptyMap(), + /** + * Map of role to provider to options, for deployments whose provider is not fixed - a + * bring-your-own-key application, or one configured for failover across providers. + * + * ```yaml + * embabel: + * models: + * roles: + * cheapest: + * openai: { model: gpt-4.1-nano } + * anthropic: { model: claude-haiku-4-5 } + * ``` + * + * Takes precedence over [llms] for the active provider. Unlike [llms], an entry naming a model + * that is not registered is not an error: it applies only when that provider is the active one. + */ + var roles: Map> = emptyMap(), /** * Map of role to embedding service name. May not include the default embedding service. */ @@ -47,7 +64,7 @@ data class ConfigurableModelProviderProperties( ) { fun allWellKnownLlmNames(): Set { - return llms.values.toSet() + defaultLlm + return llms.values.toSet() + roles.values.flatMap { it.values }.mapNotNull { it.modelName } + defaultLlm } fun allWellKnownEmbeddingServiceNames(): Set { @@ -62,10 +79,32 @@ class ConfigurableModelProvider( private val llms: List>, private val embeddingServices: List, private val properties: ConfigurableModelProviderProperties, + roleResolvers: List = emptyList(), + private val credentialLlmServiceFactories: List = emptyList(), ) : ModelProvider { private val logger = loggerFor() + private val configurableRoleResolver = + ConfigurableRoleResolver(properties) { defaultLlm.provider } + + /** + * Application resolvers first, the configuration-driven one last, so an application can + * override any role and ignore the rest. + */ + private val roleResolvers: List = roleResolvers + configurableRoleResolver + + /** + * Services built from user keys, which are per-user and so cannot be Spring beans. + * Bounded, and least-recently-used entries are dropped: an unbounded map here would retain a + * service for every key the deployment has ever seen. + */ + private val credentialLlmServices: MutableMap> = + object : LinkedHashMap>(16, 0.75f, true) { + override fun removeEldestEntry(eldest: Map.Entry>) = + size > MAX_CACHED_CREDENTIAL_SERVICES + }.let { java.util.Collections.synchronizedMap(it) } + private val defaultLlm = if (llms.isNotEmpty()) llms.firstOrNull { it.name == properties.defaultLlm } @@ -207,9 +246,94 @@ class ConfigurableModelProvider( ) } + override fun resolveLlmOptions(llmOptions: LlmOptions): LlmOptions { + val criteria = llmOptions.criteria + if (criteria !is ByRoleModelSelectionCriteria) { + return llmOptions + } + val resolved = resolveRole(criteria.role, ModelSelectionContextHolder.get()) + return llmOptions + .withDefaultsFrom(resolved.llmOptions) + .copy(modelSelectionCriteria = ModelSelectionCriteria.preResolved(resolved.llmService)) + } + + /** + * Ask each resolver in turn what the role means, and materialize the answer. + * + * A role that cannot be satisfied throws, rather than quietly resolving to something else. + * Falling back to the default LLM would mean a role like "cheapest" silently becoming the most + * capable - and most expensive - model in the deployment, which is exactly the kind of thing + * nobody notices until the bill arrives. + * + * Booting is a separate question: an unsatisfiable role only warns at startup, so a deployment + * keyed for one provider still starts and still serves every role that does work. + */ + private fun resolveRole(role: String, context: ModelSelectionContext): ResolvedRole { + val resolution = roleResolvers.firstNotNullOfOrNull { it.resolve(role, context) } + val resolved = when (resolution) { + is RoleResolution.Service -> ResolvedRole(resolution.llmService, LlmOptions.withDefaults()) + + is RoleResolution.Options -> byName(resolution.llmOptions) + ?.let { ResolvedRole(it, resolution.llmOptions) } + + is RoleResolution.Credential -> fromCredential(role, resolution.credential) + + null -> null + } + if (resolved != null) { + return resolved + } + logger.warn( + "No model available for role '{}' (provider: {})", + role, context.provider ?: "deployment default", + ) + throw NoSuitableModelException(ByRoleModelSelectionCriteria(role), llms.map { it.name }) + } + + /** + * Build - or reuse - a service for the model this role names under the user's own provider. + */ + private fun fromCredential( + role: String, + credential: ProviderCredential, + ): ResolvedRole? { + val options = configurableRoleResolver.optionsFor(role, credential.provider) + val model = options?.modelName + if (model == null) { + logger.warn( + "Role '{}' has no model configured for provider '{}' under embabel.models.roles", + role, credential.provider, + ) + return null + } + // Read then put rather than computeIfAbsent: building a service can validate the key over + // the network, and computeIfAbsent would hold the map's lock for the duration. A race here + // costs one redundant build, never a wrong service. + val key = CredentialModelKey(credential, model) + val llmService = credentialLlmServices[key] + ?: credentialLlmServiceFactories + .firstNotNullOfOrNull { it.createLlmService(credential, model) } + ?.also { credentialLlmServices[key] = it } + if (llmService == null) { + logger.warn( + "No CredentialLlmServiceFactory handles provider '{}', needed for role '{}'", + credential.provider, role, + ) + return null + } + return ResolvedRole(llmService, options) + } + + /** + * The registered service named by these options, or null if it names none. + */ + private fun byName(llmOptions: LlmOptions): LlmService<*>? = + llmOptions.modelName?.let { name -> llms.firstOrNull { it.name == name } } + override fun listRoles(modelClass: Class<*>): List { return when { - LlmService::class.java.isAssignableFrom(modelClass) -> properties.llms.keys.toList() + LlmService::class.java.isAssignableFrom(modelClass) -> + (properties.llms.keys + properties.roles.keys).toList() EmbeddingService::class.java.isAssignableFrom(modelClass) -> properties.embeddingServices.keys.toList() else -> throw IllegalArgumentException("Unsupported model class: $modelClass") } @@ -226,8 +350,7 @@ class ConfigurableModelProvider( override fun getLlm(criteria: ModelSelectionCriteria): LlmService<*> = when (criteria) { is ByRoleModelSelectionCriteria -> { - val modelName = properties.llms[criteria.role] ?: throw NoSuitableModelException(criteria, llms.map { it.name }) - llms.firstOrNull { it.name == modelName } ?: throw NoSuitableModelException(criteria, llms.map { it.name }) + resolveRole(criteria.role, ModelSelectionContextHolder.get()).llmService } is ByNameModelSelectionCriteria -> { @@ -290,4 +413,30 @@ class ConfigurableModelProvider( defaultEmbeddingService() } } + + /** + * A role, materialized: the service to call, and the options configured alongside it. + */ + private data class ResolvedRole( + val llmService: LlmService<*>, + val llmOptions: LlmOptions, + ) + + /** + * Cache key for a service built from a user's key. Holds the credential itself rather than a + * hash of it, so two distinct keys can never collide onto one another's service. + */ + private data class CredentialModelKey( + val credential: ProviderCredential, + val model: String, + ) + + companion object { + + /** + * Upper bound on services built from user keys. Generous: each is a thin wrapper around a + * chat client, and a deployment with more concurrent keys than this will simply rebuild. + */ + const val MAX_CACHED_CREDENTIAL_SERVICES = 500 + } } diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt new file mode 100644 index 000000000..523514fee --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt @@ -0,0 +1,83 @@ +/* + * 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.common.ai.model + +import org.springframework.core.Ordered + +/** + * The platform's own [RoleResolver], consulted after any the application registers. + * + * Reads `embabel.models.roles` - role, then provider, then options: + * + * ```yaml + * embabel: + * models: + * roles: + * cheapest: + * openai: { model: gpt-4.1-nano } + * anthropic: { model: claude-haiku-4-5, temperature: 0.3 } + * ``` + * + * The active provider comes from [ModelSelectionContext.provider] when a user key is in play, and + * otherwise from whichever provider supplies the default LLM - so a single-provider deployment gets + * the right column without setting a context at all. + * + * Falls back to the flat `embabel.models.llms` map, which remains the right shape for a deployment + * that will only ever have one provider. + * + * @param properties the bound `embabel.models` configuration + * @param defaultProviderName provider of the default LLM, used when no key is active. A function + * because the default LLM is resolved by [ConfigurableModelProvider] against registered beans. + */ +class ConfigurableRoleResolver( + private val properties: ConfigurableModelProviderProperties, + private val defaultProviderName: () -> String?, +) : RoleResolver, Ordered { + + override fun getOrder(): Int = Ordered.LOWEST_PRECEDENCE + + override fun resolve(role: String, context: ModelSelectionContext): RoleResolution? { + val activeProvider = context.provider ?: defaultProviderName() + val configuredForProvider = optionsFor(role, activeProvider) + if (configuredForProvider != null) { + // A user key beats deployment credentials: hand the key back and let the platform + // build a service for the model this role names under that provider. + return context.credential + ?.let { RoleResolution.Credential(it) } + ?: RoleResolution.Options(configuredForProvider) + } + if (context.credential != null) { + // The flat map names models the deployment is keyed for. Serving them to a user who + // brought their own key would quietly bill the deployment for a call the user meant to + // pay for, so leave it alone and let the role fail for this user. + return null + } + return properties.llms[role]?.let { RoleResolution.Options(LlmOptions.withModel(it)) } + } + + /** + * Options configured for [role] under [provider], or null if the role says nothing about it. + */ + fun optionsFor(role: String, provider: String?): LlmOptions? { + if (provider == null) { + return null + } + return properties.roles[role] + ?.entries + ?.firstOrNull { it.key.equals(provider, ignoreCase = true) } + ?.value + } +} diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt index 7dbae6056..b1eabc9cb 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt @@ -29,6 +29,15 @@ interface ModelProvider : HasInfoString { @Throws(NoSuitableModelException::class) fun getEmbeddingService(criteria: ModelSelectionCriteria): EmbeddingService + /** + * Resolve any role in these options to a concrete model, applying whatever hyperparameters are + * configured against that role. Values the caller set explicitly are kept. + * + * Called once per LLM operation, before the model is chosen, so that a role can carry more than + * a model name. Options naming no role are returned unchanged. + */ + fun resolveLlmOptions(llmOptions: LlmOptions): LlmOptions = llmOptions + /** * List the roles available for this class of model */ diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt new file mode 100644 index 000000000..e4fc7876b --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt @@ -0,0 +1,97 @@ +/* + * 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.common.ai.model + +/** + * An API key for a named provider, supplied by a user rather than by deployment configuration. + * + * @param provider provider name, matching [ModelMetadata.provider] - for example "openai". + * Compared case-insensitively wherever the platform matches on it. + * @param apiKey the key itself. Never logged. + */ +data class ProviderCredential( + val provider: String, + val apiKey: String, +) { + + /** + * Deliberately hides the key: these objects end up in log lines and exception messages. + */ + override fun toString(): String = "ProviderCredential(provider=$provider, apiKey=***)" +} + +/** + * What a [RoleResolver] gets to decide with, beyond the role name itself. + * + * A role such as "cheapest" cannot be resolved to a model without knowing which provider is + * active, and in a bring-your-own-key deployment that is a property of the request rather than + * of the deployment. Set it for the duration of a request with [ModelSelectionContextHolder.with]. + * + * @param userId identity of the user on whose behalf the call is made, if any + * @param credential the provider key active for this call, if any + */ +data class ModelSelectionContext( + val userId: String? = null, + val credential: ProviderCredential? = null, +) { + + /** + * Name of the active provider, or null if the deployment default should be used. + */ + val provider: String? get() = credential?.provider + + companion object { + + /** + * No user, no key: resolution falls back to deployment configuration. + */ + @JvmField + val EMPTY = ModelSelectionContext() + } +} + +/** + * Makes the [ModelSelectionContext] for the current call available to model resolution without + * threading it through every LLM API. Applications set it at their request boundary - a servlet + * filter, an interceptor, or around the code that starts an agent process. + * + * The context does not propagate to threads you spawn yourself. Capture it with [get] and re-establish + * it with [with] inside the new thread. + */ +object ModelSelectionContextHolder { + + private val current = ThreadLocal.withInitial { ModelSelectionContext.EMPTY } + + /** + * The context for the current thread, or [ModelSelectionContext.EMPTY] if none was set. + */ + @JvmStatic + fun get(): ModelSelectionContext = current.get() + + /** + * Run [block] with [context] active, restoring the previous context afterwards. + */ + @JvmStatic + fun with(context: ModelSelectionContext, block: () -> T): T { + val previous = current.get() + current.set(context) + try { + return block() + } finally { + current.set(previous) + } + } +} diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt new file mode 100644 index 000000000..8be0d9a24 --- /dev/null +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt @@ -0,0 +1,83 @@ +/* + * 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.common.ai.model + +import com.embabel.agent.spi.LlmService + +/** + * What a role resolved to. + * + * Three cases, in increasing order of how much the resolver does itself. Most applications need + * only [Credential]: say whose key is active and let the platform do the rest. + */ +sealed interface RoleResolution { + + /** + * Configured options - a model name, and optionally hyperparameters that travel with the role. + * Values the caller set explicitly still win. + */ + data class Options( + val llmOptions: LlmOptions, + ) : RoleResolution + + /** + * A provider key. The platform looks the role up for that provider and builds the service + * through a [CredentialLlmServiceFactory], caching it. + */ + data class Credential( + val credential: ProviderCredential, + ) : RoleResolution + + /** + * An already-built service, for callers that construct their own. + */ + data class Service( + val llmService: LlmService<*>, + ) : RoleResolution +} + +/** + * Decides what a role means for a given call. + * + * Register as many as you like: the platform consults them in Spring [org.springframework.core.Ordered] + * order and takes the first non-null answer, so a resolver can handle the roles it cares about and + * delegate the rest. The platform's own [ConfigurableRoleResolver] runs last and reads + * `embabel.models.roles` and `embabel.models.llms`. + * + * Implementations must be thread-safe. + */ +fun interface RoleResolver { + + /** + * @param role the role requested, for example "cheapest" + * @param context who the call is for and which provider key is active + * @return how to satisfy the role, or null to let the next resolver decide + */ + fun resolve(role: String, context: ModelSelectionContext): RoleResolution? +} + +/** + * Builds an [LlmService] from a user-supplied key. Provider modules contribute implementations; + * a bring-your-own-key application does not need to write one. + */ +fun interface CredentialLlmServiceFactory { + + /** + * @return a service for [model] authenticated with [credential], or null if this factory does + * not handle that provider + */ + fun createLlmService(credential: ProviderCredential, model: String): LlmService<*>? +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleConfigurationBindingTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleConfigurationBindingTest.kt new file mode 100644 index 000000000..269289a7f --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleConfigurationBindingTest.kt @@ -0,0 +1,122 @@ +/* + * 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.common.ai.model + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.context.properties.EnableConfigurationProperties +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Configuration +import java.time.Duration + +/** + * The nested `embabel.models.roles` shape has to survive Spring configuration binding, which is a + * different path from the Kotlin factories: binding sets the `model` field where + * [LlmOptions.withModel] sets selection criteria. + */ +class RoleConfigurationBindingTest { + + @Configuration + @EnableConfigurationProperties(ConfigurableModelProviderProperties::class) + class TestConfig + + private val runner = ApplicationContextRunner() + .withUserConfiguration(TestConfig::class.java) + .withConfiguration(AutoConfigurations.of()) + + @Test + fun `binds role to provider to model`() { + runner + .withPropertyValues( + "embabel.models.roles.cheapest.openai.model=gpt-4.1-nano", + "embabel.models.roles.cheapest.anthropic.model=claude-haiku-4-5", + ) + .run { context -> + val properties = context.getBean(ConfigurableModelProviderProperties::class.java) + assertEquals( + setOf("openai", "anthropic"), + properties.roles["cheapest"]?.keys, + ) + assertEquals("gpt-4.1-nano", properties.roles["cheapest"]?.get("openai")?.modelName) + assertEquals("claude-haiku-4-5", properties.roles["cheapest"]?.get("anthropic")?.modelName) + } + } + + @Test + fun `binds hyperparameters alongside the model`() { + runner + .withPropertyValues( + "embabel.models.roles.best.anthropic.model=claude-sonnet-4-6", + "embabel.models.roles.best.anthropic.temperature=0.3", + "embabel.models.roles.best.anthropic.timeout=90s", + "embabel.models.roles.best.anthropic.max-tokens=4096", + ) + .run { context -> + val options = context.getBean(ConfigurableModelProviderProperties::class.java) + .roles["best"]?.get("anthropic") + assertEquals("claude-sonnet-4-6", options?.modelName) + assertEquals(0.3, options?.temperature) + assertEquals(Duration.ofSeconds(90), options?.timeout) + assertEquals(4096, options?.maxTokens) + } + } + + @Test + fun `the flat llms map still binds, unchanged`() { + runner + .withPropertyValues( + "embabel.models.llms.cheapest=gpt-4.1-nano", + "embabel.models.default-llm=gpt-4.1-mini", + ) + .run { context -> + val properties = context.getBean(ConfigurableModelProviderProperties::class.java) + assertEquals(mapOf("cheapest" to "gpt-4.1-nano"), properties.llms) + assertEquals("gpt-4.1-mini", properties.defaultLlm) + assertEquals(emptyMap>(), properties.roles) + } + } + + @Test + fun `bound options report the model they name`() { + // Binding sets the model field rather than selection criteria, so modelName has to read both. + runner + .withPropertyValues("embabel.models.roles.cheapest.openai.model=gpt-4.1-nano") + .run { context -> + val options = context.getBean(ConfigurableModelProviderProperties::class.java) + .roles["cheapest"]!!["openai"]!! + assertEquals("gpt-4.1-nano", options.modelName) + assertNull(options.modelSelectionCriteria) + } + } + + @Test + fun `models named under roles count as well known`() { + runner + .withPropertyValues( + "embabel.models.roles.cheapest.openai.model=gpt-4.1-nano", + "embabel.models.llms.best=gpt-4.1", + "embabel.models.default-llm=gpt-4.1-mini", + ) + .run { context -> + assertEquals( + setOf("gpt-4.1-nano", "gpt-4.1", "gpt-4.1-mini"), + context.getBean(ConfigurableModelProviderProperties::class.java).allWellKnownLlmNames(), + ) + } + } +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt new file mode 100644 index 000000000..09c3f7c6e --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -0,0 +1,445 @@ +/* + * 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.common.ai.model + +import com.embabel.agent.spi.LlmService +import com.embabel.agent.spi.support.springai.SpringAiLlmService +import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.springframework.ai.chat.model.ChatModel +import java.time.Duration + +/** + * Roles resolving against whichever provider is active, rather than against a fixed model name. + */ +class RoleResolutionTest { + + private fun llm(name: String, provider: String): LlmService<*> = + SpringAiLlmService(name, provider, mockk(), DefaultOptionsConverter) + + private val openAiModel = llm("gpt-4.1-nano", "openai") + private val anthropicModel = llm("claude-haiku-4-5", "anthropic") + private val defaultModel = llm("gpt-4.1-mini", "openai") + + /** + * One role, two providers, each with its own model and its own tuning. + */ + private val nestedRoles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano"), + "anthropic" to LlmOptions.withModel("claude-haiku-4-5").withTemperature(0.3), + ), + ) + + private fun provider( + models: List> = listOf(openAiModel, anthropicModel, defaultModel), + properties: ConfigurableModelProviderProperties = ConfigurableModelProviderProperties( + roles = nestedRoles, + defaultLlm = "gpt-4.1-mini", + ), + roleResolvers: List = emptyList(), + factories: List = emptyList(), + ) = ConfigurableModelProvider( + llms = models, + embeddingServices = emptyList(), + properties = properties, + roleResolvers = roleResolvers, + credentialLlmServiceFactories = factories, + ) + + @Nested + inner class ProviderDimension { + + @Test + fun `falls back to the provider of the default LLM when no key is active`() { + val resolved = provider().getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + assertEquals("gpt-4.1-nano", resolved.name) + } + + @Test + fun `resolves against the provider active for this call`() { + val built = mutableListOf() + val mp = provider( + factories = listOf( + CredentialLlmServiceFactory { _, model -> + built += model + anthropicModel + }, + ), + ) + val resolved = ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", "sk-test")), + ) { + mp.resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) + } + // The anthropic column, not the openai one the deployment default would have picked. + assertEquals(listOf("claude-haiku-4-5"), built) + assertEquals("claude-haiku-4-5", modelNameOf(resolved)) + } + + @Test + fun `provider match is case insensitive`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("OpenAI" to LlmOptions.withModel("gpt-4.1-nano"))), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertEquals("gpt-4.1-nano", mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)).name) + } + + @Test + fun `flat llms map still works and is used when the nested shape says nothing`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "claude-haiku-4-5"), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertEquals("claude-haiku-4-5", mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)).name) + } + + @Test + fun `nested shape takes precedence over the flat map for the active provider`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "claude-haiku-4-5"), + roles = nestedRoles, + defaultLlm = "gpt-4.1-mini", + ), + ) + assertEquals("gpt-4.1-nano", mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)).name) + } + } + + @Nested + inner class UnsatisfiableRoles { + + @Test + fun `a configured role whose model is not registered throws`() { + val mp = provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "a-model-nobody-registered"), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertThrows { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + + @Test + fun `a role configured only for another provider throws`() { + val mp = provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("anthropic" to LlmOptions.withModel("claude-haiku-4-5"))), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertThrows { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + + @Test + fun `an unsatisfiable role never silently resolves to the default LLM`() { + // "cheapest" quietly becoming the deployment's most capable model is the failure mode + // that motivates throwing here rather than falling back. + val expensiveDefault = llm("gpt-4.1-mini", "openai") + val mp = provider( + models = listOf(expensiveDefault), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-nano"), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertThrows { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + + @Test + fun `constructing the provider does not fail when a role names an unavailable model`() { + // Previously fatal at context refresh, which made a partially keyed deployment unbootable. + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "a-model-nobody-registered"), + defaultLlm = "gpt-4.1-mini", + ), + ) + } + + @Test + fun `a role nobody configured still throws`() { + assertThrows { + provider().getLlm(ByRoleModelSelectionCriteria("no-such-role")) + } + } + } + + @Nested + inner class RoleCarriedOptions { + + @Test + fun `hyperparameters configured against a role are applied`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + roles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano") + .withTemperature(0.2) + .withTimeout(Duration.ofSeconds(90)), + ), + ), + defaultLlm = "gpt-4.1-mini", + ), + ) + val resolved = mp.resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) + assertEquals(0.2, resolved.temperature) + assertEquals(Duration.ofSeconds(90), resolved.timeout) + } + + @Test + fun `what the caller set explicitly beats what the role configures`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + roles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano").withTemperature(0.2), + ), + ), + defaultLlm = "gpt-4.1-mini", + ), + ) + val resolved = mp.resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE).withTemperature(0.9)) + assertEquals(0.9, resolved.temperature) + } + + @Test + fun `options naming no role are returned untouched`() { + val asked = LlmOptions.withModel("gpt-4.1-mini").withTemperature(0.5) + assertEquals(asked, provider().resolveLlmOptions(asked)) + } + + @Test + fun `resolved options carry the chosen service so it is not looked up twice`() { + val resolved = provider().resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) + val criteria = resolved.criteria + assertTrue(criteria is PreResolvedModelSelectionCriteria<*>) + assertSame(openAiModel, (criteria as PreResolvedModelSelectionCriteria<*>).resolved) + } + } + + @Nested + inner class ApplicationResolvers { + + @Test + fun `an application resolver wins over configuration`() { + val mp = provider( + roleResolvers = listOf( + RoleResolver { _, _ -> RoleResolution.Service(anthropicModel) }, + ), + ) + assertSame(anthropicModel, mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE))) + } + + @Test + fun `a resolver returning null delegates to the next one`() { + val mp = provider( + roleResolvers = listOf( + RoleResolver { _, _ -> null }, + RoleResolver { _, _ -> RoleResolution.Service(anthropicModel) }, + ), + ) + assertSame(anthropicModel, mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE))) + } + + @Test + fun `a resolver sees the user and key active for the call`() { + var seen: ModelSelectionContext? = null + val mp = provider( + roleResolvers = listOf( + RoleResolver { _, context -> + seen = context + RoleResolution.Service(anthropicModel) + }, + ), + ) + val context = ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")) + ModelSelectionContextHolder.with(context) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + assertEquals(context, seen) + } + } + + @Nested + inner class BringYourOwnKey { + + private val userService = llm("claude-haiku-4-5", "anthropic") + private var buildCount = 0 + + private val factory = CredentialLlmServiceFactory { credential, model -> + if (credential.provider == "anthropic" && model == "claude-haiku-4-5") { + buildCount++ + userService + } else { + null + } + } + + @Test + fun `a key in context resolves the role through that provider`() { + val mp = provider(factories = listOf(factory)) + val resolved = ModelSelectionContextHolder.with( + ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + assertSame(userService, resolved) + } + + @Test + fun `the role's tuning for that provider travels with the user's key`() { + val mp = provider(factories = listOf(factory)) + val resolved = ModelSelectionContextHolder.with( + ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")), + ) { + mp.resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) + } + assertEquals(0.3, resolved.temperature) + } + + @Test + fun `a service built from a key is reused rather than rebuilt per call`() { + val mp = provider(factories = listOf(factory)) + buildCount = 0 + repeat(3) { + ModelSelectionContextHolder.with( + ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + assertEquals(1, buildCount) + } + + @Test + fun `two users with different keys do not share a service`() { + val built = mutableListOf() + val perKeyFactory = CredentialLlmServiceFactory { credential, _ -> + built += credential + llm("claude-haiku-4-5", "anthropic") + } + val mp = provider(factories = listOf(perKeyFactory)) + listOf("sk-ben", "sk-rod").forEach { key -> + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", key)), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + assertEquals(listOf("sk-ben", "sk-rod"), built.map { it.apiKey }) + } + + @Test + fun `a user key is never served from deployment credentials`() { + // The role has no mistral column, and the deployment's own models are keyed by the + // deployment. Serving them here would bill the deployment for this user's call. + val mp = provider(factories = listOf(factory)) + assertThrows { + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("mistral", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + } + + @Test + fun `a user key does not fall through to the flat llms map either`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-nano"), + defaultLlm = "gpt-4.1-mini", + ), + factories = listOf(factory), + ) + assertThrows { + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + } + + } + + @Nested + inner class ContextHolder { + + @Test + fun `defaults to empty`() { + assertEquals(ModelSelectionContext.EMPTY, ModelSelectionContextHolder.get()) + assertNull(ModelSelectionContextHolder.get().provider) + } + + @Test + fun `restores the previous context when nested`() { + val outer = ModelSelectionContext("outer") + val inner = ModelSelectionContext("inner") + ModelSelectionContextHolder.with(outer) { + ModelSelectionContextHolder.with(inner) { + assertEquals(inner, ModelSelectionContextHolder.get()) + } + assertEquals(outer, ModelSelectionContextHolder.get()) + } + assertEquals(ModelSelectionContext.EMPTY, ModelSelectionContextHolder.get()) + } + + @Test + fun `restores the previous context even when the body throws`() { + assertThrows { + ModelSelectionContextHolder.with(ModelSelectionContext("ben")) { + error("boom") + } + } + assertEquals(ModelSelectionContext.EMPTY, ModelSelectionContextHolder.get()) + } + + @Test + fun `a credential never prints its key`() { + val rendered = ProviderCredential("anthropic", "sk-very-secret").toString() + assertTrue(rendered.contains("anthropic")) + assertTrue(!rendered.contains("sk-very-secret")) + } + } + + private fun modelNameOf(options: LlmOptions): String = + (options.criteria as PreResolvedModelSelectionCriteria<*>).resolved + .let { it as LlmService<*> }.name +} diff --git a/embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/LlmOptions.kt b/embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/LlmOptions.kt index fdb32aae8..61f850b8f 100644 --- a/embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/LlmOptions.kt +++ b/embabel-agent-common/embabel-agent-ai/src/main/kotlin/com/embabel/common/ai/model/LlmOptions.kt @@ -21,6 +21,7 @@ import com.embabel.common.ai.model.ModelSelectionCriteria.Companion.byRole import com.embabel.common.ai.model.spi.InternalExtensionApi import com.embabel.common.core.types.HasInfoString import com.embabel.common.util.indent +import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonProperty import io.swagger.v3.oas.annotations.media.Schema import java.time.Duration @@ -189,6 +190,42 @@ data class LlmOptions @JvmOverloads constructor( return copy(timeout = timeout) } + /** + * The model these options name, however they name it. + * + * [withModel] records the choice as a [ByNameModelSelectionCriteria] while configuration + * binding sets the [model] field, so neither one alone answers the question. + */ + @get:JsonIgnore + val modelName: String? + get() = model ?: (criteria as? ByNameModelSelectionCriteria)?.name + + /** + * Fill in anything this instance leaves unset from [defaults], keeping every value set here. + * + * Used to apply the hyperparameters configured against a role without overriding what the + * caller asked for: a role may say `temperature: 0.3`, but a caller that passed its own + * temperature still gets that one. + * + * Model selection is taken from [defaults] wholesale, since the point of resolving a role is + * to decide which model it means. + */ + fun withDefaultsFrom(defaults: LlmOptions): LlmOptions = + copy( + modelSelectionCriteria = defaults.modelSelectionCriteria, + model = defaults.model, + role = defaults.role, + temperature = temperature ?: defaults.temperature, + frequencyPenalty = frequencyPenalty ?: defaults.frequencyPenalty, + maxTokens = maxTokens ?: defaults.maxTokens, + presencePenalty = presencePenalty ?: defaults.presencePenalty, + topK = topK ?: defaults.topK, + topP = topP ?: defaults.topP, + thinking = thinking ?: defaults.thinking, + timeout = timeout ?: defaults.timeout, + extensions = defaults.extensions + extensions, + ) + /** * Get a provider-specific extension value by key. * Returns null if the extension is not present or cannot be cast to type T. diff --git a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc index d9367ec2b..5efddf910 100644 --- a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc +++ b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc @@ -598,6 +598,90 @@ ai.withLlmByRole("best") NOTE: If no LLM is specified in `@LlmCall` or `withLlm()`, the `default-llm` from configuration is used. +===== Roles across providers + +The flat `llms` map above names one model per role, and a model belongs to one provider. That is the +right shape when a deployment will only ever be keyed for one provider. When it will not - a +bring-your-own-key application, or one that fails over between providers - give the role a provider +dimension with `roles`: + +[source,yaml] +---- +embabel: + models: + default-llm: gpt-4.1-mini + roles: + cheapest: + openai: + model: gpt-4.1-nano # <1> + anthropic: + model: claude-haiku-4-5 + temperature: 0.3 # <2> + best: + openai: + model: gpt-4.1 + anthropic: + model: claude-sonnet-4-6 +---- +<1> The model this role means when OpenAI is the active provider +<2> Tuning configured against the role, applied whenever this role resolves to this provider + +Call sites do not change: `ai.withLlmByRole("cheapest")` resolves against whichever provider is +active for the call. With no user key in play, that is the provider supplying the `default-llm`, so +a single-provider deployment needs nothing further. + +Anything the caller sets explicitly still wins over what the role configures, so +`ai.withLlmByRole("cheapest").withTemperature(0.9)` keeps `0.9`. + +A role that cannot be satisfied - because the active provider has no entry for it, or the model it +names is not registered - throws `NoSuitableModelException` at the point of use. It does not quietly +resolve to something else: falling back to the `default-llm` would let a role like `cheapest` become +the most capable, and most expensive, model in the deployment without anyone noticing. + +Startup is a separate question. An unsatisfiable role only logs a warning when the platform starts, +so a deployment keyed for one provider still boots and still serves every role that does work - you +find out about the broken role when something asks for it, and the message names the role, the +active provider, and the models that were available. + +===== Resolving roles in code + +To decide what a role means per user rather than per deployment, register a `RoleResolver` bean. +Resolvers are consulted in `Ordered` order, ahead of the configuration above, and returning `null` +delegates to the next one. + +[source,kotlin] +---- +@Component +class ByokRoleResolver( + private val userKeyStore: UserKeyStore, +) : RoleResolver { + + override fun resolve(role: String, context: ModelSelectionContext): RoleResolution? = + context.userId + ?.let { userKeyStore.activeKeyFor(it) } + ?.let { RoleResolution.Credential(ProviderCredential(it.provider, it.apiKey)) } +} +---- + +`RoleResolution.Credential` hands back a key and lets the platform do the rest: it looks up what the +role means under that provider, builds the service through a `CredentialLlmServiceFactory`, and +caches it so the service is not rebuilt on every call. Return `RoleResolution.Options` to name a +model and its tuning directly, or `RoleResolution.Service` to supply an `LlmService` you built +yourself. + +The `ModelSelectionContext` a resolver receives comes from `ModelSelectionContextHolder`, which the +application sets at its request boundary: + +[source,kotlin] +---- +ModelSelectionContextHolder.with(ModelSelectionContext(userId = currentUserId)) { + agentPlatform.runAgent(...) +} +---- + +The context does not propagate to threads you spawn yourself. Capture it with +`ModelSelectionContextHolder.get()` and re-establish it inside the new thread. + ===== Using Your Custom Implementation (Alternative) If you need more control over the LLM operations layer itself, you can extend `ToolLoopLlmOperations`: From 35bb4be995b2058b97423eaf0a9249b5b9f94b65 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 8 Aug 2026 15:35:51 +1000 Subject: [PATCH 02/12] Carry the model selection context across platform threads, and keep the Java constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three problems with role resolution as it stood. ModelSelectionContext is a ThreadLocal, and the platform moves work off the calling thread itself — AgentPlatform.start, parallel actions, OperationContext.parallelMap. Losing the context there does not fail: ConfigurableRoleResolver falls back to the deployment's own provider and serves a model the deployment is billed for, on a call the user brought their own key for. That is exactly the invariant this feature claims. ExecutorAsyncer now propagates it alongside AgentProcess, restoring the previous value so nothing leaks between tasks on a pooled thread. ConfigurableModelProvider gained two Kotlin default parameters, which Java callers cannot skip. @JvmOverloads keeps the three-argument constructor — without it the BYOK starter's boot test (#1890) fails to compile the moment these two branches meet. The startup warning promised a fallback to the default LLM that no longer happens: an unsatisfiable role throws at the point of use, deliberately. Say that instead. Tests: context propagation through async and parallelMap including pooled-thread isolation; role resolution end to end through ChatClientLlmOperations, so the wiring that makes a role carry hyperparameters is pinned, not just the provider that computes them; LlmOptions.withDefaultsFrom and modelName in their own module; the credential paths that fail rather than fall back. Signed-off-by: jasper blues --- .../agent/spi/support/ExecutorAsyncer.kt | 34 ++-- .../ai/model/ConfigurableModelProvider.kt | 2 +- .../common/ai/model/ModelSelectionContext.kt | 7 +- .../ModelSelectionContextPropagationTest.kt | 88 ++++++++++ .../spi/support/RoleResolutionWiringTest.kt | 163 ++++++++++++++++++ .../common/ai/model/RoleResolutionTest.kt | 81 +++++++++ .../common/ai/model/LlmOptionsDefaultsTest.kt | 122 +++++++++++++ .../main/asciidoc/reference/llms/page.adoc | 4 +- 8 files changed, 486 insertions(+), 15 deletions(-) create mode 100644 embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt create mode 100644 embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt create mode 100644 embabel-agent-common/embabel-agent-ai/src/test/kotlin/com/embabel/common/ai/model/LlmOptionsDefaultsTest.kt diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ExecutorAsyncer.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ExecutorAsyncer.kt index 7ce67b1ed..62b00c17c 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ExecutorAsyncer.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ExecutorAsyncer.kt @@ -16,6 +16,7 @@ package com.embabel.agent.spi.support import com.embabel.agent.api.common.Asyncer +import com.embabel.common.ai.model.ModelSelectionContextHolder import io.micrometer.context.ContextSnapshotFactory import javax.annotation.concurrent.ThreadSafe import java.util.concurrent.CompletableFuture @@ -24,9 +25,16 @@ import java.util.concurrent.Semaphore /** * Asyncer implementation that uses an Executor for async operations, propagating to worker - * threads the [AgentProcess] (a domain concern, via [AgentProcessAccessor]) and the current - * Micrometer Observation (via the official [ContextSnapshotFactory], so spans nest across - * threads; a no-op when no observation is current, e.g. a NOOP registry). + * threads the [AgentProcess] (a domain concern, via [AgentProcessAccessor]), the + * [com.embabel.common.ai.model.ModelSelectionContext] (so a user's own provider key still + * decides model selection off the request thread), and the current Micrometer Observation + * (via the official [ContextSnapshotFactory], so spans nest across threads; a no-op when no + * observation is current, e.g. a NOOP registry). + * + * The model selection context matters here because the platform itself moves work off the + * calling thread - `AgentPlatform.start`, parallel actions, `OperationContext.parallelMap`. + * Losing it does not fail: role resolution quietly falls back to deployment configuration and + * serves a model the deployment pays for, on a call the user brought their own key for. */ @ThreadSafe class ExecutorAsyncer( @@ -36,21 +44,25 @@ class ExecutorAsyncer( private val contextSnapshotFactory = ContextSnapshotFactory.builder().clearMissing(true).build() override fun async(block: () -> T): CompletableFuture { - // Capture AgentProcess and the current observation from the calling thread + // Capture AgentProcess, model selection context and the current observation from the calling thread val agentProcess = AgentProcessAccessor.getValue() + val modelSelectionContext = ModelSelectionContextHolder.get() val contextSnapshot = contextSnapshotFactory.captureAll() return CompletableFuture.supplyAsync({ contextSnapshot.setThreadLocals().use { - if (agentProcess != null) { - AgentProcessAccessor.setValue(agentProcess) - try { + // with() restores whatever the pooled thread held before, so nothing leaks between tasks + ModelSelectionContextHolder.with(modelSelectionContext) { + if (agentProcess != null) { + AgentProcessAccessor.setValue(agentProcess) + try { + block() + } finally { + AgentProcessAccessor.reset() // cleanup + } + } else { block() - } finally { - AgentProcessAccessor.reset() // cleanup } - } else { - block() } } }, executor) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index aa1383e19..d9c5c3e09 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -75,7 +75,7 @@ data class ConfigurableModelProviderProperties( /** * Take LLM definitions from configuration */ -class ConfigurableModelProvider( +class ConfigurableModelProvider @JvmOverloads constructor( private val llms: List>, private val embeddingServices: List, private val properties: ConfigurableModelProviderProperties, diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt index e4fc7876b..3f38dd6ed 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelSelectionContext.kt @@ -68,8 +68,11 @@ data class ModelSelectionContext( * threading it through every LLM API. Applications set it at their request boundary - a servlet * filter, an interceptor, or around the code that starts an agent process. * - * The context does not propagate to threads you spawn yourself. Capture it with [get] and re-establish - * it with [with] inside the new thread. + * The platform propagates the context across the threads it starts itself - a background agent run, + * parallel actions, `OperationContext.parallelMap` - because losing it there does not fail loudly: + * role resolution would quietly fall back to deployment configuration and serve a model the + * deployment is billed for. It does not reach threads the application spawns on its own. Capture it + * with [get] and re-establish it with [with] inside such a thread. */ object ModelSelectionContextHolder { diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt new file mode 100644 index 000000000..5a929ce4c --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt @@ -0,0 +1,88 @@ +/* + * 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 + +import com.embabel.common.ai.model.ModelSelectionContext +import com.embabel.common.ai.model.ModelSelectionContextHolder +import com.embabel.common.ai.model.ProviderCredential +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +/** + * The model selection context has to survive the platform moving work off the calling thread - + * `AgentPlatform.start`, parallel actions, `OperationContext.parallelMap` all go through + * [Asyncer]. + * + * Losing it does not fail loudly: role resolution falls back to deployment configuration and + * serves a model the deployment is keyed and billed for, on a call the user brought their own + * key for. That is why this is pinned rather than left to the application. + */ +class ModelSelectionContextPropagationTest { + + private val executor = Executors.newFixedThreadPool(2) + private val asyncer = ExecutorAsyncer(executor) + + @AfterEach + fun cleanup() { + executor.shutdown() + executor.awaitTermination(5, TimeUnit.SECONDS) + } + + @Test + fun `the user's credential reaches the worker thread`() { + val context = ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")) + + val seen = ModelSelectionContextHolder.with(context) { + asyncer.async { ModelSelectionContextHolder.get() } + }.get(5, TimeUnit.SECONDS) + + assertEquals(context, seen) + } + + @Test + fun `parallelMap carries the context to every worker`() { + val context = ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")) + + val seen = ModelSelectionContextHolder.with(context) { + asyncer.parallelMap(listOf(1, 2, 3, 4), maxConcurrency = 2) { + ModelSelectionContextHolder.get() + } + } + + assertEquals(List(4) { context }, seen) + } + + @Test + fun `a task leaves no context behind for the next task on the same thread`() { + val single = Executors.newSingleThreadExecutor() + try { + val singleAsyncer = ExecutorAsyncer(single) + ModelSelectionContextHolder.with(ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test"))) { + singleAsyncer.async { ModelSelectionContextHolder.get() } + }.get(5, TimeUnit.SECONDS) + + // Same pooled thread, no context on the caller this time: it must not inherit ben's key + val seen = singleAsyncer.async { ModelSelectionContextHolder.get() }.get(5, TimeUnit.SECONDS) + assertEquals(ModelSelectionContext.EMPTY, seen) + } finally { + single.shutdown() + single.awaitTermination(5, TimeUnit.SECONDS) + } + } +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt new file mode 100644 index 000000000..be50d44a8 --- /dev/null +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt @@ -0,0 +1,163 @@ +/* + * 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. + */ +@file:OptIn(InternalObservabilityApi::class) + +package com.embabel.agent.spi.support + +import com.embabel.agent.api.common.InteractionId +import com.embabel.agent.api.event.observation.InternalObservabilityApi +import com.embabel.agent.core.AgentProcess +import com.embabel.agent.core.ProcessContext +import com.embabel.agent.core.ProcessOptions +import com.embabel.agent.core.support.LlmInteraction +import com.embabel.agent.spi.LlmService +import com.embabel.agent.spi.support.springai.ChatClientLlmOperations +import com.embabel.agent.spi.support.springai.SpringAiLlmService +import com.embabel.agent.spi.validation.DefaultValidationPromptGenerator +import com.embabel.agent.support.SimpleTestAgent +import com.embabel.agent.test.common.EventSavingAgenticEventListener +import com.embabel.chat.UserMessage +import com.embabel.common.ai.model.ConfigurableModelProvider +import com.embabel.common.ai.model.ConfigurableModelProviderProperties +import com.embabel.common.ai.model.DefaultOptionsConverter +import com.embabel.common.ai.model.LlmOptions +import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE +import com.embabel.common.textio.template.JinjavaTemplateRenderer +import io.mockk.every +import io.mockk.mockk +import jakarta.validation.Validation +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import tools.jackson.module.kotlin.jacksonObjectMapper +import java.util.concurrent.Executors + +/** + * Roles are resolved by [ConfigurableModelProvider], but the thing that makes a role able to + * carry hyperparameters is [AbstractLlmOperations] asking it to, once per operation, before + * anything reads the options. Unit-testing the provider alone would leave that wiring untested, + * and its absence looks exactly like "the role's temperature is ignored". + */ +class RoleResolutionWiringTest { + + private data class Dog(val name: String) + + private fun llm(name: String, chatModel: FakeChatModel): LlmService<*> = + SpringAiLlmService(name, "openai", chatModel, DefaultOptionsConverter) + + @Test + fun `a by-role interaction reaches the model the role names, with the tuning it carries`() { + val cheap = FakeChatModel(jacksonObjectMapper().writeValueAsString(Dog("Duke"))) + val expensive = FakeChatModel(jacksonObjectMapper().writeValueAsString(Dog("Rex"))) + + val modelProvider = ConfigurableModelProvider( + llms = listOf(llm("gpt-4.1-nano", cheap), llm("gpt-4.1-mini", expensive)), + embeddingServices = emptyList(), + properties = ConfigurableModelProviderProperties( + roles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano").withTemperature(0.2), + ), + ), + defaultLlm = "gpt-4.1-mini", + ), + ) + + val (llmOperations, agentProcess) = setup(modelProvider) + val dog = llmOperations.createObject( + messages = listOf(UserMessage("Name a dog")), + interaction = LlmInteraction( + id = InteractionId("role"), + llm = LlmOptions.withLlmForRole(CHEAPEST_ROLE), + ), + outputClass = Dog::class.java, + action = SimpleTestAgent.actions.first(), + agentProcess = agentProcess, + ) + + assertEquals(Dog("Duke"), dog, "the role's model answered, not the default") + assertEquals(0, expensive.promptsPassed.size, "the default LLM must not have been called") + assertEquals(0.2, cheap.optionsPassed.single().temperature, "the role's temperature travelled with it") + } + + @Test + fun `what the caller sets explicitly still beats the role`() { + val cheap = FakeChatModel(jacksonObjectMapper().writeValueAsString(Dog("Duke"))) + val modelProvider = ConfigurableModelProvider( + llms = listOf(llm("gpt-4.1-nano", cheap)), + embeddingServices = emptyList(), + properties = ConfigurableModelProviderProperties( + roles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano").withTemperature(0.2), + ), + ), + defaultLlm = "gpt-4.1-nano", + ), + ) + + val (llmOperations, agentProcess) = setup(modelProvider) + llmOperations.createObject( + messages = listOf(UserMessage("Name a dog")), + interaction = LlmInteraction( + id = InteractionId("role"), + llm = LlmOptions.withLlmForRole(CHEAPEST_ROLE).withTemperature(0.9), + ), + outputClass = Dog::class.java, + action = SimpleTestAgent.actions.first(), + agentProcess = agentProcess, + ) + + assertEquals(0.9, cheap.optionsPassed.single().temperature) + } + + private data class Wiring( + val llmOperations: ChatClientLlmOperations, + val agentProcess: AgentProcess, + ) + + private fun setup(modelProvider: ConfigurableModelProvider): Wiring { + val ese = EventSavingAgenticEventListener() + val processContext = mockk() + every { processContext.platformServices } returns mockk() + every { processContext.platformServices.agentPlatform } returns mockk() + every { processContext.platformServices.agentPlatform.toolGroupResolver } returns + RegistryToolGroupResolver("mt", emptyList()) + every { processContext.platformServices.eventListener } returns ese + every { processContext.processOptions } returns ProcessOptions() + + val agentProcess = mockk() + every { agentProcess.recordLlmInvocation(any()) } answers { } + every { processContext.onProcessEvent(any()) } answers { ese.onProcessEvent(firstArg()) } + every { processContext.agentProcess } returns agentProcess + every { agentProcess.agent } returns SimpleTestAgent + every { agentProcess.processContext } returns processContext + every { agentProcess.blackboard } returns mockk(relaxed = true) + + return Wiring( + ChatClientLlmOperations( + modelProvider = modelProvider, + toolDecorator = DefaultToolDecorator(), + validator = Validation.buildDefaultValidatorFactory().validator, + validationPromptGenerator = DefaultValidationPromptGenerator(), + templateRenderer = JinjavaTemplateRenderer(), + objectMapper = jacksonObjectMapper(), + dataBindingProperties = LlmDataBindingProperties(), + asyncer = ExecutorAsyncer(Executors.newCachedThreadPool()), + ), + agentProcess, + ) + } +} diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index 09c3f7c6e..459b2dce4 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -27,6 +27,7 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.springframework.ai.chat.model.ChatModel +import org.springframework.core.Ordered import java.time.Duration /** @@ -397,6 +398,86 @@ class RoleResolutionTest { } } + @Test + fun `a provider no factory handles fails rather than falling back`() { + // The role names a model for openai, but the only factory speaks anthropic. Serving + // the deployment's own openai model here would bill the deployment for the user's call. + val mp = provider(factories = listOf(factory)) + assertThrows { + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("openai", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + } + + @Test + fun `a role column with tuning but no model cannot be satisfied by a key`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("anthropic" to LlmOptions().withTemperature(0.3))), + defaultLlm = "gpt-4.1-mini", + ), + factories = listOf(factory), + ) + assertThrows { + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", "sk-test")), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + } + } + + @Nested + inner class PlatformResolver { + + private val resolver = ConfigurableRoleResolver( + ConfigurableModelProviderProperties(roles = nestedRoles, defaultLlm = "gpt-4.1-mini"), + ) { "openai" } + + @Test + fun `runs last, so an application resolver can always take precedence`() { + assertEquals(Ordered.LOWEST_PRECEDENCE, resolver.order) + } + + @Test + fun `says nothing when there is no provider to resolve against`() { + assertNull(resolver.optionsFor(CHEAPEST_ROLE, provider = null)) + } + + @Test + fun `says nothing about a role it has never heard of`() { + assertNull(resolver.optionsFor("no-such-role", provider = "openai")) + assertNull(resolver.resolve("no-such-role", ModelSelectionContext.EMPTY)) + } + } + + @Nested + inner class DefaultSpiBehaviour { + + /** + * A [ModelProvider] that does not override role resolution: the default is to leave + * options alone, so an implementation predating roles keeps working. + */ + private val minimal = object : ModelProvider { + override fun getLlm(criteria: ModelSelectionCriteria): LlmService<*> = openAiModel + override fun getEmbeddingService(criteria: ModelSelectionCriteria): EmbeddingService = + throw UnsupportedOperationException() + + override fun listRoles(modelClass: Class<*>): List = emptyList() + override fun listModels(): List = emptyList() + override fun listModelNames(modelClass: Class<*>): List = emptyList() + override fun infoString(verbose: Boolean?, indent: Int): String = "minimal" + } + + @Test + fun `the default resolveLlmOptions returns what it was given`() { + val asked = LlmOptions.withLlmForRole(CHEAPEST_ROLE) + assertSame(asked, minimal.resolveLlmOptions(asked)) + } } @Nested diff --git a/embabel-agent-common/embabel-agent-ai/src/test/kotlin/com/embabel/common/ai/model/LlmOptionsDefaultsTest.kt b/embabel-agent-common/embabel-agent-ai/src/test/kotlin/com/embabel/common/ai/model/LlmOptionsDefaultsTest.kt new file mode 100644 index 000000000..63e816fd7 --- /dev/null +++ b/embabel-agent-common/embabel-agent-ai/src/test/kotlin/com/embabel/common/ai/model/LlmOptionsDefaultsTest.kt @@ -0,0 +1,122 @@ +/* + * 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.common.ai.model + +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 java.time.Duration + +/** + * [LlmOptions.modelName] and [LlmOptions.withDefaultsFrom] are what let a role carry + * hyperparameters as well as a model name, so they are pinned here rather than only through + * the resolution path that uses them. + */ +class LlmOptionsDefaultsTest { + + @Nested + inner class ModelName { + + @Test + fun `reads the model field, which is what configuration binding sets`() { + assertEquals("gpt-4.1-nano", LlmOptions(model = "gpt-4.1-nano").modelName) + } + + @Test + fun `reads by-name selection criteria, which is what withModel sets`() { + assertEquals("gpt-4.1-nano", LlmOptions.withModel("gpt-4.1-nano").modelName) + } + + @Test + fun `the model field wins when both are set`() { + assertEquals( + "from-field", + LlmOptions.withModel("from-criteria").copy(model = "from-field").modelName, + ) + } + + @Test + fun `options naming no model report none`() { + assertNull(LlmOptions().modelName) + assertNull(LlmOptions.withLlmForRole("cheapest").modelName) + assertNull(LlmOptions.withAutoLlm().modelName) + } + } + + @Nested + inner class WithDefaultsFrom { + + private val defaults = LlmOptions.withModel("gpt-4.1-nano") + .withTemperature(0.2) + .withMaxTokens(1000) + .withTopK(10) + .withTopP(0.8) + .withFrequencyPenalty(0.1) + .withPresencePenalty(0.2) + .withThinking(Thinking.withTokenBudget(512)) + .withTimeout(Duration.ofSeconds(90)) + .copy(role = "cheapest") + + @Test + fun `fills in everything the caller left unset`() { + val merged = LlmOptions().withDefaultsFrom(defaults) + + assertEquals("gpt-4.1-nano", merged.modelName) + assertEquals("cheapest", merged.role) + assertEquals(0.2, merged.temperature) + assertEquals(1000, merged.maxTokens) + assertEquals(10, merged.topK) + assertEquals(0.8, merged.topP) + assertEquals(0.1, merged.frequencyPenalty) + assertEquals(0.2, merged.presencePenalty) + assertEquals(512, merged.thinking?.tokenBudget) + assertEquals(Duration.ofSeconds(90), merged.timeout) + } + + @Test + fun `keeps every value the caller set explicitly`() { + val asked = LlmOptions() + .withTemperature(0.9) + .withMaxTokens(50) + .withTopK(1) + .withTopP(0.1) + .withFrequencyPenalty(0.9) + .withPresencePenalty(0.8) + .withThinking(Thinking.NONE) + .withTimeout(Duration.ofSeconds(5)) + + val merged = asked.withDefaultsFrom(defaults) + + assertEquals(0.9, merged.temperature) + assertEquals(50, merged.maxTokens) + assertEquals(1, merged.topK) + assertEquals(0.1, merged.topP) + assertEquals(0.9, merged.frequencyPenalty) + assertEquals(0.8, merged.presencePenalty) + assertNull(merged.thinking?.tokenBudget) + assertEquals(Duration.ofSeconds(5), merged.timeout) + } + + @Test + fun `model selection is taken from the defaults wholesale, since that is the point`() { + // Resolving a role decides which model it means, so the caller's own model choice + // (there is none when a role was named) must not survive the merge. + val merged = LlmOptions.withModel("caller-choice").withDefaultsFrom(defaults) + assertEquals("gpt-4.1-nano", merged.modelName) + } + } +} diff --git a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc index 5efddf910..0bd62c7fe 100644 --- a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc +++ b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc @@ -679,7 +679,9 @@ ModelSelectionContextHolder.with(ModelSelectionContext(userId = currentUserId)) } ---- -The context does not propagate to threads you spawn yourself. Capture it with +The platform carries the context across the threads it starts itself — a background agent run, +parallel actions, `OperationContext.parallelMap` — so setting it once at the request boundary is +enough. It does not reach threads your own code spawns: capture it there with `ModelSelectionContextHolder.get()` and re-establish it inside the new thread. ===== Using Your Custom Implementation (Alternative) From 3166ace5f5ad171359252779951c1ad54b975c55 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 8 Aug 2026 16:06:18 +1000 Subject: [PATCH 03/12] Resolve roles on the streaming path, keep the role name, stop caching keys in the clear Streaming went straight to chooseLlm, so a role picked the right model and then streamed with none of the tuning configured against it. Streaming is the chat path, which is where that tuning matters most. Resolve once and stream with the same options; the capability queries resolve too, so they answer about the model the role actually means. Resolution dropped the role name, because withDefaultsFrom takes model selection from the defaults wholesale and the configured options name no role. Selection no longer needs it, but events, logs and cost attribution all want to know a call was made "as cheapest" rather than as a bare model name. Put it back explicitly. The credential cache keyed on ProviderCredential, so every cached service also parked a plaintext key in a map living as long as the platform. Key on a SHA-256 digest instead. The service itself still holds the key it was built from, so this narrows the exposure rather than removing it - but it removes the copy that existed only for lookup. Tests cover each: streaming carries the role's temperature and timeout and never touches the default model; resolved options report the role; and the digest change does not alter what the cache means - same key reuses, different key rebuilds. Signed-off-by: jasper blues --- .../spi/support/AbstractLlmOperations.kt | 29 ++++++++---- .../ai/model/ConfigurableModelProvider.kt | 44 ++++++++++++++++--- .../spi/support/RoleResolutionWiringTest.kt | 41 +++++++++++++++++ .../common/ai/model/RoleResolutionTest.kt | 28 ++++++++++++ 4 files changed, 128 insertions(+), 14 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt index dafe66964..47ab0c318 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt @@ -428,11 +428,20 @@ abstract class AbstractLlmOperations( * * Interactions naming no role are returned untouched, so the common path costs nothing. */ - private fun withRoleResolved(interaction: LlmInteraction): LlmInteraction = - if (interaction.llm.criteria is ByRoleModelSelectionCriteria) { - interaction.copy(llm = modelProvider.resolveLlmOptions(interaction.llm)) + private fun withRoleResolved(interaction: LlmInteraction): LlmInteraction { + val resolved = withRoleResolved(interaction.llm) + return if (resolved === interaction.llm) interaction else interaction.copy(llm = resolved) + } + + /** + * As above, for the paths that carry options rather than a whole interaction - streaming, + * and the capability queries that pick a model without running a prompt. + */ + private fun withRoleResolved(options: LlmOptions): LlmOptions = + if (options.criteria is ByRoleModelSelectionCriteria) { + modelProvider.resolveLlmOptions(options) } else { - interaction + options } protected fun chooseLlm( @@ -452,18 +461,22 @@ abstract class AbstractLlmOperations( } override fun supportsStreaming(options: LlmOptions): Boolean { - val llmService = chooseLlm(options) + val llmService = chooseLlm(withRoleResolved(options)) return llmService.supportsStreaming() } override fun supportsThinking(options: LlmOptions): Boolean { - val llmService = chooseLlm(options) + val llmService = chooseLlm(withRoleResolved(options)) return llmService.supportsThinking() } override fun createStreamingOperations(options: LlmOptions): StreamingLlmOperations { - val llmService = chooseLlm(options) - val messageStreamer = llmService.createMessageStreamer(options) + // Resolve once and stream with the SAME options. The streamer reads hyperparameters, so + // resolving only far enough to pick a model would silently drop the tuning a role carries - + // and streaming is the chat path, where that tuning matters most. + val resolved = withRoleResolved(options) + val llmService = chooseLlm(resolved) + val messageStreamer = llmService.createMessageStreamer(resolved) return StreamingLlmOperationsImpl( messageStreamer = messageStreamer, objectMapper = objectMapper, diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index d9c5c3e09..45f3a8ae9 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -21,6 +21,8 @@ import com.embabel.common.util.indent import com.embabel.common.util.loggerFor import org.springframework.boot.context.properties.ConfigurationProperties import org.springframework.validation.annotation.Validated +import java.nio.charset.StandardCharsets +import java.security.MessageDigest /** * Configuration properties for the model provider @@ -254,7 +256,13 @@ class ConfigurableModelProvider @JvmOverloads constructor( val resolved = resolveRole(criteria.role, ModelSelectionContextHolder.get()) return llmOptions .withDefaultsFrom(resolved.llmOptions) - .copy(modelSelectionCriteria = ModelSelectionCriteria.preResolved(resolved.llmService)) + .copy( + modelSelectionCriteria = ModelSelectionCriteria.preResolved(resolved.llmService), + // Keep the role that was asked for. Selection no longer consults it - the + // pre-resolved criteria decide - but events, logs and cost attribution all want to + // know a call was made "as cheapest", which is otherwise lost the moment it resolves. + role = criteria.role, + ) } /** @@ -309,7 +317,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( // Read then put rather than computeIfAbsent: building a service can validate the key over // the network, and computeIfAbsent would hold the map's lock for the duration. A race here // costs one redundant build, never a wrong service. - val key = CredentialModelKey(credential, model) + val key = CredentialModelKey.of(credential, model) val llmService = credentialLlmServices[key] ?: credentialLlmServiceFactories .firstNotNullOfOrNull { it.createLlmService(credential, model) } @@ -423,13 +431,37 @@ class ConfigurableModelProvider @JvmOverloads constructor( ) /** - * Cache key for a service built from a user's key. Holds the credential itself rather than a - * hash of it, so two distinct keys can never collide onto one another's service. + * Cache key for a service built from a user's key. + * + * Identifies the key by SHA-256 rather than holding it, so the plaintext is not duplicated + * into a map that lives as long as the platform does. The cached [LlmService] was built from + * the key and still holds it, so this narrows the exposure rather than removing it - but it + * removes the copy that exists purely for lookup, and keeps keys out of any heap dump taken + * of the cache itself. + * + * A digest cannot collide in practice, and two users would have to share a provider AND a + * model AND a SHA-256 collision to reach one another's service. */ private data class CredentialModelKey( - val credential: ProviderCredential, + val provider: String, + val apiKeyDigest: String, val model: String, - ) + ) { + + companion object { + + fun of(credential: ProviderCredential, model: String) = CredentialModelKey( + provider = credential.provider.lowercase(), + apiKeyDigest = digest(credential.apiKey), + model = model, + ) + + private fun digest(apiKey: String): String = + MessageDigest.getInstance("SHA-256") + .digest(apiKey.toByteArray(StandardCharsets.UTF_8)) + .joinToString("") { "%02x".format(it) } + } + } companion object { diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt index be50d44a8..a471e4a94 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt @@ -38,10 +38,13 @@ import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE import com.embabel.common.textio.template.JinjavaTemplateRenderer import io.mockk.every import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify import jakarta.validation.Validation import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import tools.jackson.module.kotlin.jacksonObjectMapper +import java.time.Duration import java.util.concurrent.Executors /** @@ -123,6 +126,44 @@ class RoleResolutionWiringTest { assertEquals(0.9, cheap.optionsPassed.single().temperature) } + @Test + fun `streaming gets the role's tuning too, not just the role's model`() { + // Streaming is the chat path. Resolving only far enough to choose a model would leave the + // streamer running on whatever the caller happened to pass, which is nothing. + val streamed = slot() + val roleModel = mockk>(relaxed = true) + every { roleModel.name } returns "gpt-4.1-nano" + every { roleModel.provider } returns "openai" + every { roleModel.createMessageStreamer(capture(streamed)) } returns mockk(relaxed = true) + + val defaultModel = mockk>(relaxed = true) + every { defaultModel.name } returns "gpt-4.1-mini" + every { defaultModel.provider } returns "openai" + + val modelProvider = ConfigurableModelProvider( + llms = listOf(roleModel, defaultModel), + embeddingServices = emptyList(), + properties = ConfigurableModelProviderProperties( + roles = mapOf( + CHEAPEST_ROLE to mapOf( + "openai" to LlmOptions.withModel("gpt-4.1-nano") + .withTemperature(0.2) + .withTimeout(Duration.ofSeconds(90)), + ), + ), + defaultLlm = "gpt-4.1-mini", + ), + ) + + setup(modelProvider).llmOperations.createStreamingOperations( + LlmOptions.withLlmForRole(CHEAPEST_ROLE), + ) + + assertEquals(0.2, streamed.captured.temperature) + assertEquals(Duration.ofSeconds(90), streamed.captured.timeout) + verify(exactly = 0) { defaultModel.createMessageStreamer(any()) } + } + private data class Wiring( val llmOperations: ChatClientLlmOperations, val agentProcess: AgentProcess, diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index 459b2dce4..cee06a4a1 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -245,6 +245,14 @@ class RoleResolutionTest { assertEquals(asked, provider().resolveLlmOptions(asked)) } + @Test + fun `resolved options still say which role was asked for`() { + // Selection no longer consults it, but events, logs and cost attribution all want to + // know the call was made "as cheapest" rather than as a bare model name. + val resolved = provider().resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) + assertEquals(CHEAPEST_ROLE, resolved.role) + } + @Test fun `resolved options carry the chosen service so it is not looked up twice`() { val resolved = provider().resolveLlmOptions(LlmOptions.withLlmForRole(CHEAPEST_ROLE)) @@ -398,6 +406,26 @@ class RoleResolutionTest { } } + @Test + fun `the same key and model reuse one service, a different key does not`() { + // The cache is keyed by a digest of the key rather than the key itself. That must not + // change what the cache MEANS: same key reuses, different key rebuilds. + val built = mutableListOf() + val perKeyFactory = CredentialLlmServiceFactory { credential, _ -> + built += credential.apiKey + llm("claude-haiku-4-5", "anthropic") + } + val mp = provider(factories = listOf(perKeyFactory)) + listOf("sk-ben", "sk-ben", "sk-rod", "sk-ben").forEach { key -> + ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", key)), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + } + assertEquals(listOf("sk-ben", "sk-rod"), built) + } + @Test fun `a provider no factory handles fails rather than falling back`() { // The role names a model for openai, but the only factory speaks anthropic. Serving From d2d47f634d66c7ebd4aba48faa11817699bbd0dc Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sat, 8 Aug 2026 19:41:27 +1000 Subject: [PATCH 04/12] Warn at startup about unsatisfiable roles under the nested shape too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The startup check only walked embabel.models.llms, so the shape this feature adds got no validation at all — while the docs it ships say an unsatisfiable role warns at startup. That gap matters most precisely here. The nested shape is the one place configuration can take a role AWAY: with an entry present for the active provider, resolution uses it and throws when its model is not registered, instead of falling back to the flat map. So a typo under roles was silent until something asked for the role, and then failed a request. Only entries for the deployment's own provider are checked. An entry for a provider it is not keyed for is the entire point of the provider dimension — its model is not expected to be registered, and warning about it would teach people to ignore the warning. Signed-off-by: jasper blues --- .../ai/model/ConfigurableModelProvider.kt | 37 +++++++++ .../common/ai/model/RoleResolutionTest.kt | 78 +++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 45f3a8ae9..92c56854e 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -174,6 +174,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( } } } + warnAboutUnsatisfiableNestedRoles() logger.info(infoString(verbose = true)) properties.embeddingServices.forEach { (role, model) -> @@ -199,6 +200,42 @@ class ConfigurableModelProvider @JvmOverloads constructor( } } + /** + * Warn about `embabel.models.roles` entries that cannot be satisfied, on the same terms as + * the flat map above. + * + * Only entries for the deployment's own provider are checked. An entry for a provider this + * deployment is not keyed for is the point of the nested shape - it applies when a user + * brings a key for that provider, and its model is not expected to be registered here, so + * warning about it would train people to ignore the warning. + * + * Without this, a typo under `roles` is silent until something asks for the role: the entry + * is found, its model is not registered, and resolution throws rather than falling back to + * the flat map - which is the one case where the nested shape can take a role AWAY. + */ + private fun warnAboutUnsatisfiableNestedRoles() { + val deploymentProvider = defaultLlm.provider + properties.roles.forEach { (role, byProvider) -> + byProvider + .filterKeys { it.equals(deploymentProvider, ignoreCase = true) } + .forEach { (provider, options) -> + val model = options.modelName + if (model == null) { + logger.warn( + "Role '{}' under provider '{}' names no model, so anything asking for that role will fail", + role, provider, + ) + } else if (llms.none { it.name == model }) { + logger.warn( + "LLM '{}' for role '{}' under provider '{}' - this deployment's own provider - is not " + + "available, so anything asking for that role will fail. Available: {}", + model, role, provider, llms.map { it.name }, + ) + } + } + } + } + private fun showModel(model: LlmService<*>): String { val roles = properties.llms.filter { it.value == model.name }.keys val maybeRoles = if (roles.isNotEmpty()) " - Roles: ${roles.joinToString(", ")}" else "" diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index cee06a4a1..139b8e022 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -15,6 +15,10 @@ */ package com.embabel.common.ai.model +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender import com.embabel.agent.spi.LlmService import com.embabel.agent.spi.support.springai.SpringAiLlmService import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE @@ -26,6 +30,7 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows +import org.slf4j.LoggerFactory import org.springframework.ai.chat.model.ChatModel import org.springframework.core.Ordered import java.time.Duration @@ -193,6 +198,63 @@ class RoleResolutionTest { ) } + @Test + fun `a nested role naming an unregistered model for our own provider warns at startup`() { + // The nested shape is the one case where configuration can take a role AWAY: the entry + // is found, its model is not registered, and resolution throws rather than falling back + // to the flat map. A typo there was silent until something asked for the role. + val warnings = captureWarnings { + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-mini"), + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nanoo"))), + defaultLlm = "gpt-4.1-mini", + ), + ) + } + assertTrue( + warnings.any { it.contains("gpt-4.1-nanoo") && it.contains(CHEAPEST_ROLE) }, + "the typo must be reported at startup: $warnings", + ) + } + + @Test + fun `a nested role for another provider is not warned about`() { + // Its model is not expected to be registered here — that is the whole point of the + // provider dimension. Warning would train people to ignore the warning. + val warnings = captureWarnings { + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("anthropic" to LlmOptions.withModel("claude-haiku-4-5"))), + defaultLlm = "gpt-4.1-mini", + ), + ) + } + assertTrue( + warnings.none { it.contains("claude-haiku-4-5") }, + "a model for a provider we are not keyed for is not a misconfiguration: $warnings", + ) + } + + @Test + fun `a nested role naming no model at all warns`() { + val warnings = captureWarnings { + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions().withTemperature(0.3))), + defaultLlm = "gpt-4.1-mini", + ), + ) + } + assertTrue( + warnings.any { it.contains("names no model") }, + "tuning without a model cannot satisfy a role: $warnings", + ) + } + @Test fun `a role nobody configured still throws`() { assertThrows { @@ -548,6 +610,22 @@ class RoleResolutionTest { } } + /** + * Warnings emitted by [ConfigurableModelProvider] while [block] runs. Startup diagnostics are + * the whole product of the checks above, so asserting on them is asserting on the behaviour. + */ + private fun captureWarnings(block: () -> Unit): List { + val logger = LoggerFactory.getLogger(ConfigurableModelProvider::class.java) as Logger + val appender = ListAppender().apply { start() } + logger.addAppender(appender) + try { + block() + } finally { + logger.detachAppender(appender) + } + return appender.list.filter { it.level == Level.WARN }.map { it.formattedMessage } + } + private fun modelNameOf(options: LlmOptions): String = (options.criteria as PreResolvedModelSelectionCriteria<*>).resolved .let { it as LlmService<*> }.name From 453c9e838640f65538b531fccd5979d0c20b40f5 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 01:34:17 +1000 Subject: [PATCH 05/12] Give the nested role shape a read API, and stop renumbering the properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things this feature was missing for the people who have to consume it. It added a config shape with no supported way to read it. listRoles returns role NAMES, and the resolver that can answer "what model does this role mean" is constructed privately inside ConfigurableModelProvider — so an application with a settings screen has to read the raw properties and reimplement the precedence between the flat and nested shapes. That is exactly how a settings screen ends up disagreeing with what actually runs. ModelProvider gains configuredOptionsForRole, sharing one lookup with the resolver so the two cannot drift, and deliberately narrower: deployment provider only, no user key, no application resolvers, so the answer does not depend on who asks or on which thread. And `roles` went in as the second constructor parameter of a data class, renumbering the three after it for anyone constructing it positionally. It reads better next to `llms`, but that is not worth a source break; moved to the end with a comment saying why it sits away from its sibling. Signed-off-by: jasper blues --- .../ai/model/ConfigurableModelProvider.kt | 30 ++++++----- .../ai/model/ConfigurableRoleResolver.kt | 22 +++++++- .../embabel/common/ai/model/ModelProvider.kt | 18 +++++++ .../common/ai/model/RoleResolutionTest.kt | 52 +++++++++++++++++++ .../main/asciidoc/reference/llms/page.adoc | 18 +++++++ 5 files changed, 127 insertions(+), 13 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 92c56854e..cabf1ebc7 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -34,6 +34,18 @@ data class ConfigurableModelProviderProperties( * Map of role to LLM name. Each entry will require an LLM to be registered with the same name. May not include the default LLM. */ var llms: Map = emptyMap(), + /** + * Map of role to embedding service name. May not include the default embedding service. + */ + var embeddingServices: Map = emptyMap(), + /** + * Default LLM name. Must be an LLM name. It's good practice to override this in configuration. + */ + var defaultLlm: String = "gpt-4.1-mini", + /** + * Default embedding model name. Must be an embedding model name. Need not be set, in which case it defaults to null. + */ + var defaultEmbeddingModel: String? = null, /** * Map of role to provider to options, for deployments whose provider is not fixed - a * bring-your-own-key application, or one configured for failover across providers. @@ -49,20 +61,11 @@ data class ConfigurableModelProviderProperties( * * Takes precedence over [llms] for the active provider. Unlike [llms], an entry naming a model * that is not registered is not an error: it applies only when that provider is the active one. + * + * Declared last, despite belonging with [llms], so that adding it does not renumber the + * existing parameters for anyone constructing this positionally. */ var roles: Map> = emptyMap(), - /** - * Map of role to embedding service name. May not include the default embedding service. - */ - var embeddingServices: Map = emptyMap(), - /** - * Default LLM name. Must be an LLM name. It's good practice to override this in configuration. - */ - var defaultLlm: String = "gpt-4.1-mini", - /** - * Default embedding model name. Must be an embedding model name. Need not be set, in which case it defaults to null. - */ - var defaultEmbeddingModel: String? = null, ) { fun allWellKnownLlmNames(): Set { @@ -302,6 +305,9 @@ class ConfigurableModelProvider @JvmOverloads constructor( ) } + override fun configuredOptionsForRole(role: String): LlmOptions? = + configurableRoleResolver.configuredOptionsFor(role, defaultLlm.provider) + /** * Ask each resolver in turn what the role means, and materialize the answer. * diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt index 523514fee..19bab6f72 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableRoleResolver.kt @@ -65,9 +65,26 @@ class ConfigurableRoleResolver( // pay for, so leave it alone and let the role fail for this user. return null } - return properties.llms[role]?.let { RoleResolution.Options(LlmOptions.withModel(it)) } + return flatOptionsFor(role)?.let { RoleResolution.Options(it) } } + /** + * What configuration says [role] means for [provider] - the nested entry if there is one, + * otherwise the flat map. + * + * This is the read side of [resolve], for callers that want to *show* a role's model rather + * than use it: a settings UI, a diagnostic endpoint, an application with its own per-user + * override layer on top. Sharing the lookup means such a caller cannot drift from what + * resolution would actually pick. + * + * It is deliberately narrower than resolution in two ways. It does not consult application + * [RoleResolver] beans, so a resolver that overrides a role per user is not reflected here. + * And it takes the provider as an argument rather than reading the active + * [ModelSelectionContext], so the answer does not depend on which thread asks. + */ + fun configuredOptionsFor(role: String, provider: String?): LlmOptions? = + optionsFor(role, provider) ?: flatOptionsFor(role) + /** * Options configured for [role] under [provider], or null if the role says nothing about it. */ @@ -80,4 +97,7 @@ class ConfigurableRoleResolver( ?.firstOrNull { it.key.equals(provider, ignoreCase = true) } ?.value } + + private fun flatOptionsFor(role: String): LlmOptions? = + properties.llms[role]?.let { LlmOptions.withModel(it) } } diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt index b1eabc9cb..eefc12ab6 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt @@ -38,6 +38,24 @@ interface ModelProvider : HasInfoString { */ fun resolveLlmOptions(llmOptions: LlmOptions): LlmOptions = llmOptions + /** + * What configuration says [role] means for this deployment, or null if it says nothing. + * + * The read side of role configuration, for callers that want to *show* a role's model rather + * than use it - a settings screen, a diagnostic endpoint, or an application layering its own + * per-user overrides on top. Without it such a caller has to read the raw properties and + * reimplement the precedence between the flat and nested shapes, which is how a settings + * screen ends up disagreeing with what actually runs. + * + * Narrower than [resolveLlmOptions] on purpose: it answers for the deployment's own provider + * and ignores both user keys and application [RoleResolver] beans, so the answer does not + * depend on who is asking or on which thread. Use [resolveLlmOptions] to find out what a + * particular call will really do. + * + * Returns null by default, meaning "this implementation cannot say". + */ + fun configuredOptionsForRole(role: String): LlmOptions? = null + /** * List the roles available for this class of model */ diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index 139b8e022..36e93574e 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -538,6 +538,53 @@ class RoleResolutionTest { assertNull(resolver.optionsFor(CHEAPEST_ROLE, provider = null)) } + @Test + fun `the configured view answers with the nested entry for the provider`() { + assertEquals("gpt-4.1-nano", provider().configuredOptionsForRole(CHEAPEST_ROLE)?.modelName) + } + + @Test + fun `the configured view falls back to the flat map`() { + val mp = provider( + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "claude-haiku-4-5"), + defaultLlm = "gpt-4.1-mini", + ), + ) + assertEquals("claude-haiku-4-5", mp.configuredOptionsForRole(CHEAPEST_ROLE)?.modelName) + } + + @Test + fun `the configured view agrees with what resolution actually picks`() { + // The reason this API exists: a settings screen that reads config itself drifts from + // what runs. Both shapes configured, nested wins — and both answers must say so. + val mp = provider( + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "claude-haiku-4-5"), + roles = nestedRoles, + defaultLlm = "gpt-4.1-mini", + ), + ) + assertEquals( + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)).name, + mp.configuredOptionsForRole(CHEAPEST_ROLE)?.modelName, + ) + } + + @Test + fun `the configured view ignores application resolvers, which are per-call`() { + val mp = provider( + roleResolvers = listOf(RoleResolver { _, _ -> RoleResolution.Service(anthropicModel) }), + ) + // The resolver decides what RUNS; configuration is what the deployment DECLARES. + assertEquals("gpt-4.1-nano", mp.configuredOptionsForRole(CHEAPEST_ROLE)?.modelName) + } + + @Test + fun `the configured view says nothing about an unconfigured role`() { + assertNull(provider().configuredOptionsForRole("no-such-role")) + } + @Test fun `says nothing about a role it has never heard of`() { assertNull(resolver.optionsFor("no-such-role", provider = "openai")) @@ -568,6 +615,11 @@ class RoleResolutionTest { val asked = LlmOptions.withLlmForRole(CHEAPEST_ROLE) assertSame(asked, minimal.resolveLlmOptions(asked)) } + + @Test + fun `the default configuredOptionsForRole says it cannot say`() { + assertNull(minimal.configuredOptionsForRole(CHEAPEST_ROLE)) + } } @Nested diff --git a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc index 0bd62c7fe..b867afda8 100644 --- a/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc +++ b/embabel-agent-docs/src/main/asciidoc/reference/llms/page.adoc @@ -684,6 +684,24 @@ parallel actions, `OperationContext.parallelMap` — so setting it once at the r enough. It does not reach threads your own code spawns: capture it there with `ModelSelectionContextHolder.get()` and re-establish it inside the new thread. +===== Reading what a role is configured to mean + +Resolution answers "what will this call use", which depends on the active key and on any +`RoleResolver` beans. A settings screen or diagnostic endpoint wants a different question — "what +does this deployment declare this role to be" — and answering it by reading `embabel.models` +directly means reimplementing the precedence between the flat and nested shapes, which is how a +settings screen ends up disagreeing with what actually runs. + +`ModelProvider.configuredOptionsForRole` answers that question against the deployment's own +provider, ignoring user keys and application resolvers: + +[source,kotlin] +---- +val declared: String? = modelProvider.configuredOptionsForRole("cheapest")?.modelName +---- + +Use `resolveLlmOptions` instead when you need to know what a specific call will really do. + ===== Using Your Custom Implementation (Alternative) If you need more control over the LLM operations layer itself, you can extend `ToolLoopLlmOperations`: From e8f22144b608910b70352009e0af7d9d5d1e2cef Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 01:39:33 +1000 Subject: [PATCH 06/12] Cover the capability queries that resolve a role supportsStreaming and supportsThinking now resolve the role before choosing a model, but nothing asserted it. The two models in this test disagree, so answering from the default would be visible rather than merely uncovered. Signed-off-by: jasper blues --- .../spi/support/RoleResolutionWiringTest.kt | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt index a471e4a94..e780277fd 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt @@ -42,6 +42,7 @@ import io.mockk.slot import io.mockk.verify import jakarta.validation.Validation import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import tools.jackson.module.kotlin.jacksonObjectMapper import java.time.Duration @@ -164,6 +165,37 @@ class RoleResolutionWiringTest { verify(exactly = 0) { defaultModel.createMessageStreamer(any()) } } + @Test + fun `capability queries answer about the model the role means, not the default`() { + val roleModel = mockk>(relaxed = true) + every { roleModel.name } returns "gpt-4.1-nano" + every { roleModel.provider } returns "openai" + every { roleModel.supportsStreaming() } returns true + every { roleModel.supportsThinking() } returns true + + val defaultModel = mockk>(relaxed = true) + every { defaultModel.name } returns "gpt-4.1-mini" + every { defaultModel.provider } returns "openai" + every { defaultModel.supportsStreaming() } returns false + every { defaultModel.supportsThinking() } returns false + + val modelProvider = ConfigurableModelProvider( + llms = listOf(roleModel, defaultModel), + embeddingServices = emptyList(), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nano"))), + defaultLlm = "gpt-4.1-mini", + ), + ) + + val ops = setup(modelProvider).llmOperations + val asRole = LlmOptions.withLlmForRole(CHEAPEST_ROLE) + + // The two models disagree, so answering from the default would be visible here. + assertTrue(ops.supportsStreaming(asRole)) + assertTrue(ops.supportsThinking(asRole)) + } + private data class Wiring( val llmOperations: ChatClientLlmOperations, val agentProcess: AgentProcess, From 88dca4444edf0ad7974457f9c42c0a4369748901 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 07:02:33 +1000 Subject: [PATCH 07/12] Use an import and a multi-line string, per house convention java.util.Collections inline where an import was needed, and a warning built by concatenation where this repo uses raw strings everywhere else. The startup hunk for the flat map is now byte-identical to the one on #1888, so whichever lands first the other merges clean. Signed-off-by: jasper blues --- .../embabel/common/ai/model/ConfigurableModelProvider.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index cabf1ebc7..718a0e494 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -23,6 +23,7 @@ import org.springframework.boot.context.properties.ConfigurationProperties import org.springframework.validation.annotation.Validated import java.nio.charset.StandardCharsets import java.security.MessageDigest +import java.util.Collections /** * Configuration properties for the model provider @@ -108,7 +109,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( object : LinkedHashMap>(16, 0.75f, true) { override fun removeEldestEntry(eldest: Map.Entry>) = size > MAX_CACHED_CREDENTIAL_SERVICES - }.let { java.util.Collections.synchronizedMap(it) } + }.let { Collections.synchronizedMap(it) } private val defaultLlm = if (llms.isNotEmpty()) @@ -230,8 +231,10 @@ class ConfigurableModelProvider @JvmOverloads constructor( ) } else if (llms.none { it.name == model }) { logger.warn( - "LLM '{}' for role '{}' under provider '{}' - this deployment's own provider - is not " + - "available, so anything asking for that role will fail. Available: {}", + """ + LLM '{}' for role '{}' under provider '{}' - this deployment's own provider - + is not available, so anything asking for that role will fail. Available: {} + """.trimIndent(), model, role, provider, llms.map { it.name }, ) } From b04dff32e3f6fcb5b93ed2981519b57900c84d6a Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 09:17:18 +1000 Subject: [PATCH 08/12] Make the credential cache bound configurable, and cover context propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review points. The cache bound was a magic 500 with a hand-wave for a comment. It is an operational trade — memory against how often a deployment rebuilds a service for a key it has seen — and the right number depends on how many distinct keys are concurrently active, which only the deployment knows. Now embabel.models.credential-service-cache-size, with a test that pins LRU eviction at a configured bound of 2 so "exceeding it costs construction, never correctness" is asserted rather than claimed. Propagation coverage, as asked: the AgentProcess and model-selection ThreadLocals now have a test that they propagate TOGETHER, since one wrapper restores both and a badly nested try/finally would drop the outer one silently. Plus nested parallelMap, because the parallel tool loop fans out from work already on a worker thread and one-hop propagation would look fine in a single-level test. Writing that second one turned up a trap worth recording: parallelMap submits to its executor and then blocks on join(), so nesting it on a BOUNDED pool deadlocks - the outer tasks hold every thread waiting for inner tasks that can never be scheduled. The test now uses an unbounded executor and says why, and carries a @Timeout so a regression fails in seconds rather than hanging a build. Signed-off-by: jasper blues --- .../ai/model/ConfigurableModelProvider.kt | 50 ++++++---- .../ModelSelectionContextPropagationTest.kt | 51 +++++++++- .../common/ai/model/RoleResolutionTest.kt | 95 +++++++++++++++++-- 3 files changed, 172 insertions(+), 24 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 718a0e494..695ff5f87 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -67,6 +67,19 @@ data class ConfigurableModelProviderProperties( * existing parameters for anyone constructing this positionally. */ var roles: Map> = emptyMap(), + /** + * Upper bound on LLM services built from user-supplied keys and held for reuse. + * + * A cache bound is an operational concern: it trades memory against how often a deployment + * rebuilds a service for a key it has seen before, and the right number depends on how many + * distinct keys are concurrently active — which only the deployment knows. The default suits + * a deployment with tens to low hundreds of concurrent users; raise it if yours has more, and + * expect roughly one thin chat-client wrapper per entry. + * + * Exceeding it is not an error. Least-recently-used entries are dropped and rebuilt on next + * use, so the only cost of setting it too low is repeated construction. + */ + var credentialServiceCacheSize: Int = 500, ) { fun allWellKnownLlmNames(): Set { @@ -108,7 +121,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( private val credentialLlmServices: MutableMap> = object : LinkedHashMap>(16, 0.75f, true) { override fun removeEldestEntry(eldest: Map.Entry>) = - size > MAX_CACHED_CREDENTIAL_SERVICES + size > properties.credentialServiceCacheSize }.let { Collections.synchronizedMap(it) } private val defaultLlm = @@ -160,13 +173,11 @@ class ConfigurableModelProvider @JvmOverloads constructor( init { properties.llms.forEach { (role, model) -> if (llms.none { it.name == model }) { - /* - * Fatal, unless this deployment is waiting for a key. A name that resolves to - * nothing is a typo in a deployment that has one, and letting it start would move - * the failure to whichever unrelated call first asks for that role. A deployment in - * setup-required mode has no models registered yet by definition, so the same name - * is expected there and only worth reporting. - */ + // Fatal, unless this deployment is waiting for a key. A name that resolves to + // nothing is a typo in a deployment that has one, and letting it start would move + // the failure to whichever unrelated call first asks for that role. A deployment in + // setup-required mode has no models registered yet by definition, so the same name + // is expected there and only worth reporting. if (setupRequired) { logger.warn( "LLM '{}' for role '{}' is not registered. This deployment is awaiting a key, so that is expected; " + @@ -337,6 +348,21 @@ class ConfigurableModelProvider @JvmOverloads constructor( if (resolved != null) { return resolved } + if (setupRequired) { + // No key has arrived yet, so no role can name a registered model and this is not a + // misconfiguration. Hand back the placeholder rather than throwing: the caller then + // fails with the same actionable "no LLM configured" error that the default LLM already + // gives, instead of a NoSuitableModelException listing the placeholder as a choice. + // + // Not a silent substitution of the kind this method otherwise refuses. The objection to + // falling back is that "cheapest" would quietly become a real, expensive model; the + // placeholder answers nothing and bills nothing. + logger.debug( + "Role '{}' has no registered model and this deployment is awaiting a key; using the placeholder", + role, + ) + return ResolvedRole(defaultLlm, LlmOptions.withDefaults()) + } logger.warn( "No model available for role '{}' (provider: {})", role, context.provider ?: "deployment default", @@ -509,12 +535,4 @@ class ConfigurableModelProvider @JvmOverloads constructor( } } - companion object { - - /** - * Upper bound on services built from user keys. Generous: each is a thin wrapper around a - * chat client, and a deployment with more concurrent keys than this will simply rebuild. - */ - const val MAX_CACHED_CREDENTIAL_SERVICES = 500 - } } diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt index 5a929ce4c..442271fac 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/ModelSelectionContextPropagationTest.kt @@ -15,12 +15,16 @@ */ package com.embabel.agent.spi.support +import com.embabel.agent.core.AgentProcess import com.embabel.common.ai.model.ModelSelectionContext import com.embabel.common.ai.model.ModelSelectionContextHolder import com.embabel.common.ai.model.ProviderCredential +import io.mockk.mockk import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertSame import org.junit.jupiter.api.Test +import org.junit.jupiter.api.Timeout import java.util.concurrent.Executors import java.util.concurrent.TimeUnit @@ -33,9 +37,14 @@ import java.util.concurrent.TimeUnit * serves a model the deployment is keyed and billed for, on a call the user brought their own * key for. That is why this is pinned rather than left to the application. */ +@Timeout(30) class ModelSelectionContextPropagationTest { - private val executor = Executors.newFixedThreadPool(2) + // Unbounded on purpose. parallelMap submits to this executor and then blocks on join(), so a + // bounded pool deadlocks as soon as one parallelMap runs inside another: the outer tasks hold + // every thread while waiting for inner tasks that can never be scheduled. Production wiring + // uses a cached or virtual-thread executor for the same reason. + private val executor = Executors.newCachedThreadPool() private val asyncer = ExecutorAsyncer(executor) @AfterEach @@ -68,6 +77,46 @@ class ModelSelectionContextPropagationTest { assertEquals(List(4) { context }, seen) } + @Test + fun `the model selection context and the AgentProcess propagate together`() { + // These are two separate ThreadLocals restored by the same wrapper. A change that + // established one inside the other's try/finally could drop the outer on the way out, + // and nothing else in the suite would notice. + val process = mockk(relaxed = true) + val context = ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")) + AgentProcess.set(process) + try { + val seen = ModelSelectionContextHolder.with(context) { + asyncer.async { AgentProcess.get() to ModelSelectionContextHolder.get() } + }.get(5, TimeUnit.SECONDS) + + assertSame(process, seen.first, "AgentProcess must still propagate") + assertEquals(context, seen.second, "model selection context must propagate alongside it") + } finally { + AgentProcess.remove() + } + assertEquals(ModelSelectionContext.EMPTY, ModelSelectionContextHolder.get()) + } + + @Test + fun `nested parallelMap keeps the context on every level`() { + // The parallel tool loop fans out from work that is itself already on a worker thread. + // Propagation that only survives one hop would look fine in a single-level test. + // + // Needs the unbounded executor above: nesting parallelMap on a bounded pool deadlocks. + val context = ModelSelectionContext("ben", ProviderCredential("anthropic", "sk-test")) + + val seen = ModelSelectionContextHolder.with(context) { + asyncer.parallelMap(listOf(1, 2), maxConcurrency = 2) { + asyncer.parallelMap(listOf(1, 2), maxConcurrency = 2) { + ModelSelectionContextHolder.get() + } + } + } + + assertEquals(List(2) { List(2) { context } }, seen) + } + @Test fun `a task leaves no context behind for the next task on the same thread`() { val single = Executors.newSingleThreadExecutor() diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index 36e93574e..7e1fcd581 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -20,6 +20,7 @@ import ch.qos.logback.classic.Logger import ch.qos.logback.classic.spi.ILoggingEvent import ch.qos.logback.core.read.ListAppender import com.embabel.agent.spi.LlmService +import com.embabel.agent.spi.PlaceholderLlmService import com.embabel.agent.spi.support.springai.SpringAiLlmService import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE import io.mockk.mockk @@ -47,6 +48,17 @@ class RoleResolutionTest { private val anthropicModel = llm("claude-haiku-4-5", "anthropic") private val defaultModel = llm("gpt-4.1-mini", "openai") + /** + * Stands in for `SetupRequiredLlm`, which lives in the BYOK module this one cannot depend on. + * [SpringAiLlmService] is a data class and so final; the real placeholder carries the marker by + * delegation in the same way. + */ + private val placeholderModel: LlmService<*> = object : + LlmService by SpringAiLlmService( + "setup-required", "none", mockk(), DefaultOptionsConverter, + ), + PlaceholderLlmService {} + /** * One role, two providers, each with its own model and its own tuning. */ @@ -143,10 +155,12 @@ class RoleResolutionTest { @Test fun `a configured role whose model is not registered throws`() { + // Configured through the nested shape, which warns rather than failing at startup: the + // flat map is fatal at construction in a keyed deployment, so it cannot reach here. val mp = provider( models = listOf(defaultModel), properties = ConfigurableModelProviderProperties( - llms = mapOf(CHEAPEST_ROLE to "a-model-nobody-registered"), + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("a-model-nobody-registered"))), defaultLlm = "gpt-4.1-mini", ), ) @@ -177,7 +191,7 @@ class RoleResolutionTest { val mp = provider( models = listOf(expensiveDefault), properties = ConfigurableModelProviderProperties( - llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-nano"), + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nano"))), defaultLlm = "gpt-4.1-mini", ), ) @@ -187,15 +201,52 @@ class RoleResolutionTest { } @Test - fun `constructing the provider does not fail when a role names an unavailable model`() { - // Previously fatal at context refresh, which made a partially keyed deployment unbootable. - provider( - models = listOf(defaultModel), + fun `constructing the provider DOES fail when a keyed deployment names an unavailable model`() { + // A deployment whose default-llm resolves to a real model has a key, so a name nothing + // registers is a typo. Booting anyway would move the failure to whichever unrelated + // call first asked for the role. + assertThrows { + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "a-model-nobody-registered"), + defaultLlm = "gpt-4.1-mini", + ), + ) + } + } + + @Test + fun `a deployment awaiting a key starts, and its roles report that rather than failing`() { + // The BYOK case: no key has arrived, so NO role can name a registered model. The + // placeholder answers, so the caller gets the same actionable "no LLM configured" error + // the default LLM already gives instead of a NoSuitableModelException. + val mp = provider( + models = listOf(placeholderModel), + properties = ConfigurableModelProviderProperties( + llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-nano"), + defaultLlm = "setup-required", + ), + ) + + assertSame(placeholderModel, mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE))) + } + + @Test + fun `the placeholder is not handed out once a real default is registered`() { + // The hybrid deployment - BYOK starter alongside a provider starter. It has a key, so + // an unsatisfiable role is still an error rather than "you have not set a key". + val mp = provider( + models = listOf(defaultModel, placeholderModel), properties = ConfigurableModelProviderProperties( - llms = mapOf(CHEAPEST_ROLE to "a-model-nobody-registered"), + roles = mapOf(CHEAPEST_ROLE to mapOf("anthropic" to LlmOptions.withModel("claude-haiku-4-5"))), defaultLlm = "gpt-4.1-mini", ), ) + + assertThrows { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } } @Test @@ -488,6 +539,36 @@ class RoleResolutionTest { assertEquals(listOf("sk-ben", "sk-rod"), built) } + @Test + fun `the cache is bounded by configuration, and evicted entries rebuild`() { + val built = mutableListOf() + val mp = provider( + properties = ConfigurableModelProviderProperties( + roles = nestedRoles, + defaultLlm = "gpt-4.1-mini", + credentialServiceCacheSize = 2, + ), + factories = listOf( + CredentialLlmServiceFactory { credential, _ -> + built += credential.apiKey + llm("claude-haiku-4-5", "anthropic") + }, + ), + ) + fun resolveFor(key: String) = ModelSelectionContextHolder.with( + ModelSelectionContext(credential = ProviderCredential("anthropic", key)), + ) { + mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + } + + listOf("sk-a", "sk-b", "sk-c").forEach { resolveFor(it) } + resolveFor("sk-a") + + // sk-a was the least recently used when sk-c arrived, so it is gone and rebuilds. + // Exceeding the bound costs construction, never correctness. + assertEquals(listOf("sk-a", "sk-b", "sk-c", "sk-a"), built) + } + @Test fun `a provider no factory handles fails rather than falling back`() { // The role names a model for openai, but the only factory speaks anthropic. Serving From badb5c657eafff88f466585f0b8ff3c1565fcc59 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 09:27:45 +1000 Subject: [PATCH 09/12] Say why the shadowing is deliberate, and state the resolver ordering contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review points, both fair: the code was relying on the reader inferring intent. The bare @Suppress("NAME_SHADOWING") looked like silencing a warning. It is a safety property. After that line the resolved interaction IS the interaction for the rest of the method, and shadowing makes the unresolved one unreachable — a distinct name would leave both in scope, differing only in whether a role has become a concrete model plus its hyperparameters, and reaching for the wrong one is not a compile error but a call that silently skips role resolution. Said so, at all four sites. The ordering contract was only discoverable by reading AgentPlatformConfiguration. It is now on RoleResolver, where someone writing one will find it: application resolvers always precede the platform's own, Ordered applies between them, and ties fall back to registration order and should not be relied on. Signed-off-by: jasper blues --- .../spi/support/AbstractLlmOperations.kt | 20 +++++++++++ .../ai/model/ConfigurableModelProvider.kt | 33 ++++++++++++------- .../embabel/common/ai/model/RoleResolver.kt | 19 ++++++++--- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt index 47ab0c318..9c5593ee1 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt @@ -145,6 +145,11 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): O { + // Shadowed deliberately. After this line the resolved interaction IS the interaction for + // the rest of the method, and shadowing makes the unresolved one unreachable. A distinct + // name would leave both in scope, differing only in whether a role has become a concrete + // model plus its hyperparameters - and picking the wrong one is not a compile error, it is + // a call that silently skips role resolution and runs on the default model. @Suppress("NAME_SHADOWING") val interaction = withRoleResolved(interaction) @@ -264,6 +269,11 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): Result { + // Shadowed deliberately. After this line the resolved interaction IS the interaction for + // the rest of the method, and shadowing makes the unresolved one unreachable. A distinct + // name would leave both in scope, differing only in whether a role has become a concrete + // model plus its hyperparameters - and picking the wrong one is not a compile error, it is + // a call that silently skips role resolution and runs on the default model. @Suppress("NAME_SHADOWING") val interaction = withRoleResolved(interaction) @@ -319,6 +329,11 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): ThinkingResponse { + // Shadowed deliberately. After this line the resolved interaction IS the interaction for + // the rest of the method, and shadowing makes the unresolved one unreachable. A distinct + // name would leave both in scope, differing only in whether a role has become a concrete + // model plus its hyperparameters - and picking the wrong one is not a compile error, it is + // a call that silently skips role resolution and runs on the default model. @Suppress("NAME_SHADOWING") val interaction = withRoleResolved(interaction) @@ -374,6 +389,11 @@ abstract class AbstractLlmOperations( agentProcess: AgentProcess, action: Action?, ): Result> { + // Shadowed deliberately. After this line the resolved interaction IS the interaction for + // the rest of the method, and shadowing makes the unresolved one unreachable. A distinct + // name would leave both in scope, differing only in whether a role has become a concrete + // model plus its hyperparameters - and picking the wrong one is not a compile error, it is + // a call that silently skips role resolution and runs on the default model. @Suppress("NAME_SHADOWING") val interaction = withRoleResolved(interaction) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 695ff5f87..7a5d14b53 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -189,7 +189,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( } } } - warnAboutUnsatisfiableNestedRoles() + checkNestedRoles() logger.info(infoString(verbose = true)) properties.embeddingServices.forEach { (role, model) -> @@ -228,7 +228,7 @@ class ConfigurableModelProvider @JvmOverloads constructor( * is found, its model is not registered, and resolution throws rather than falling back to * the flat map - which is the one case where the nested shape can take a role AWAY. */ - private fun warnAboutUnsatisfiableNestedRoles() { + private fun checkNestedRoles() { val deploymentProvider = defaultLlm.provider properties.roles.forEach { (role, byProvider) -> byProvider @@ -236,23 +236,34 @@ class ConfigurableModelProvider @JvmOverloads constructor( .forEach { (provider, options) -> val model = options.modelName if (model == null) { - logger.warn( - "Role '{}' under provider '{}' names no model, so anything asking for that role will fail", - role, provider, + reportUnsatisfiableRole( + "Role '$role' under provider '$provider' names no model, so anything asking for that role will fail", ) } else if (llms.none { it.name == model }) { - logger.warn( - """ - LLM '{}' for role '{}' under provider '{}' - this deployment's own provider - - is not available, so anything asking for that role will fail. Available: {} - """.trimIndent(), - model, role, provider, llms.map { it.name }, + reportUnsatisfiableRole( + "LLM '$model' for role '$role' under provider '$provider' - this deployment's own " + + "provider - is not available. Available: ${llms.map { it.name }}", ) } } } } + /** + * Report a role this deployment cannot satisfy, on the same terms as the flat map: fatal in a + * deployment that holds a key, expected in one that is waiting for one. + * + * Shared so the two shapes cannot drift. Applying the rule to only one of them was the original + * defect here - a typo under `roles` warned and booted while the same typo under `llms` did not. + */ + private fun reportUnsatisfiableRole(message: String) { + if (setupRequired) { + logger.warn("{}. This deployment is awaiting a key, so that is expected", message) + } else { + error(message) + } + } + private fun showModel(model: LlmService<*>): String { val roles = properties.llms.filter { it.value == model.name }.keys val maybeRoles = if (roles.isNotEmpty()) " - Roles: ${roles.joinToString(", ")}" else "" diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt index 8be0d9a24..46532491d 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt @@ -52,12 +52,21 @@ sealed interface RoleResolution { /** * Decides what a role means for a given call. * - * Register as many as you like: the platform consults them in Spring [org.springframework.core.Ordered] - * order and takes the first non-null answer, so a resolver can handle the roles it cares about and - * delegate the rest. The platform's own [ConfigurableRoleResolver] runs last and reads - * `embabel.models.roles` and `embabel.models.llms`. + * Register as many as you like. The platform takes the first non-null answer, so a resolver can + * handle the roles it cares about and delegate the rest by returning null. * - * Implementations must be thread-safe. + * Ordering, in full, because it decides who wins: + * + * - **Application resolvers always precede the platform's own.** [ConfigurableRoleResolver] is not + * in the bean stream at all - it is appended after the ordered beans - so configuration is + * always the last word, whatever an application registers. + * - **Between application resolvers**, Spring's [org.springframework.core.Ordered] applies: + * lowest value first, via `@Order` or by implementing `Ordered`. A resolver that does neither is + * `LOWEST_PRECEDENCE` and sorts after any that do. + * - **Ties** fall back to bean registration order, which is not something to depend on. If two of + * your resolvers can answer the same role, order them explicitly. + * + * Implementations must be thread-safe: one instance serves every call, on any thread. */ fun interface RoleResolver { From 75707dc23aa04da4c489afe8f5fafdd91efe9e5c Mon Sep 17 00:00:00 2001 From: jasper blues Date: Sun, 9 Aug 2026 13:32:26 +1000 Subject: [PATCH 10/12] Resolve roles on the low-level transform paths, and fix four stale role tests doTransform and its variants are entry points on LlmOperations in their own right, not only reachable through createObject, so a role named there silently ran on the default model. Resolution is idempotent, so the createObject path pays nothing. The four RoleResolutionTest failures were left by the commit that made checkNestedRoles fatal: they still asserted the older warn-at-startup behaviour and so died in the constructor. They now assert the rule the code enforces, and one is repurposed to cover the awaiting-a-key side of the gate. Also: the embedding-services check gets the same gate; Asyncer states the context it must propagate; CredentialLlmServiceFactory no longer claims provider modules ship an implementation, since none does yet. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/embabel/agent/api/common/Asyncer.kt | 12 +++ .../spi/support/AbstractLlmOperations.kt | 7 +- .../spi/support/ToolLoopLlmOperations.kt | 32 ++++++++ .../ai/model/ConfigurableModelProvider.kt | 14 ++-- .../embabel/common/ai/model/ModelProvider.kt | 7 +- .../embabel/common/ai/model/RoleResolver.kt | 24 +++++- .../common/ai/model/RoleResolutionTest.kt | 78 ++++++++++--------- 7 files changed, 125 insertions(+), 49 deletions(-) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/common/Asyncer.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/common/Asyncer.kt index ad03799eb..d1b4cf826 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/common/Asyncer.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/api/common/Asyncer.kt @@ -19,6 +19,18 @@ import java.util.concurrent.CompletableFuture /** * Simple Java-friendly async interface. + * + * An implementation is a supported extension point, and one obligation comes with it: **capture + * the calling thread's context and re-establish it on the worker**, restoring whatever the worker + * held before. Three things travel that way today - the [com.embabel.agent.core.AgentProcess], the + * current Micrometer observation, and the + * [com.embabel.common.ai.model.ModelSelectionContext] - and + * [com.embabel.agent.spi.support.ExecutorAsyncer] is the reference for how. + * + * Dropping them does not fail loudly, which is what makes this worth stating. A lost model + * selection context means role resolution quietly falls back to deployment configuration and + * serves a model the deployment is billed for, on a call the user brought their own key for. A + * lost `AgentProcess` breaks the platform's own bookkeeping just as silently. */ interface Asyncer { diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt index 9c5593ee1..ac08305d4 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/AbstractLlmOperations.kt @@ -447,8 +447,13 @@ abstract class AbstractLlmOperations( * carry hyperparameters, and which model it means depends on the provider active for this call. * * Interactions naming no role are returned untouched, so the common path costs nothing. + * + * Idempotent, so a subclass may call it on a path this class has already resolved: resolution + * replaces the role criteria with a pre-resolved one, and a second call sees no role and does + * nothing. That is what lets the low-level `doTransform` entry points resolve for themselves + * without double-resolving the `createObject` path that reaches them. */ - private fun withRoleResolved(interaction: LlmInteraction): LlmInteraction { + protected fun withRoleResolved(interaction: LlmInteraction): LlmInteraction { val resolved = withRoleResolved(interaction.llm) return if (resolved === interaction.llm) interaction else interaction.copy(llm = resolved) } diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ToolLoopLlmOperations.kt b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ToolLoopLlmOperations.kt index cdaa080a8..d91975364 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ToolLoopLlmOperations.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/agent/spi/support/ToolLoopLlmOperations.kt @@ -151,6 +151,14 @@ open class ToolLoopLlmOperations( outputClass: Class, llmRequestEvent: LlmRequestEvent?, ): O { + // Shadowed deliberately, on the same terms as AbstractLlmOperations: after this line the + // resolved interaction IS the interaction. These are the low-level entry points on + // LlmOperations - reachable directly, not only through createObject - so a role named here + // has to resolve here too, or it silently runs on the default model. Idempotent, so the + // createObject path that already resolved pays nothing. + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val llm = chooseLlm(interaction.llm) val promptContributions = buildPromptContributions(interaction, llm) @@ -239,6 +247,14 @@ open class ToolLoopLlmOperations( outputClass: Class, llmRequestEvent: LlmRequestEvent, ): Result { + // Shadowed deliberately, on the same terms as AbstractLlmOperations: after this line the + // resolved interaction IS the interaction. These are the low-level entry points on + // LlmOperations - reachable directly, not only through createObject - so a role named here + // has to resolve here too, or it silently runs on the default model. Idempotent, so the + // createObject path that already resolved pays nothing. + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val llm = chooseLlm(interaction.llm) val promptContributions = buildPromptContributions(interaction, llm) @@ -344,6 +360,14 @@ open class ToolLoopLlmOperations( outputClass: Class, llmRequestEvent: LlmRequestEvent?, ): ThinkingResponse { + // Shadowed deliberately, on the same terms as AbstractLlmOperations: after this line the + // resolved interaction IS the interaction. These are the low-level entry points on + // LlmOperations - reachable directly, not only through createObject - so a role named here + // has to resolve here too, or it silently runs on the default model. Idempotent, so the + // createObject path that already resolved pays nothing. + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + val llm = chooseLlm(interaction.llm) val promptContributions = buildPromptContributions(interaction, llm) @@ -446,6 +470,14 @@ open class ToolLoopLlmOperations( outputClass: Class, llmRequestEvent: LlmRequestEvent?, ): Result> { + // Shadowed deliberately, on the same terms as AbstractLlmOperations: after this line the + // resolved interaction IS the interaction. These are the low-level entry points on + // LlmOperations - reachable directly, not only through createObject - so a role named here + // has to resolve here too, or it silently runs on the default model. Idempotent, so the + // createObject path that already resolved pays nothing. + @Suppress("NAME_SHADOWING") + val interaction = withRoleResolved(interaction) + return try { val llm = chooseLlm(interaction.llm) val promptContributions = buildPromptContributions(interaction, llm) diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt index 7a5d14b53..b6b488b6c 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ConfigurableModelProvider.kt @@ -194,14 +194,12 @@ class ConfigurableModelProvider @JvmOverloads constructor( properties.embeddingServices.forEach { (role, model) -> if (embeddingServices.none { it.name == model }) { - /* - * The same gate as the LLM roles above, and for the same reason: an unresolvable - * name is a typo in a deployment that holds a key, and expected in one still - * waiting for it. There is no fallback here, though, and there should not be - - * an embedding model is a schema commitment and nothing can stand in for one. - * The gate decides only whether the deployment STARTS; asking for the service - * still throws. - */ + // The same gate as the LLM roles above, and for the same reason: an unresolvable + // name is a typo in a deployment that holds a key, and expected in one still + // waiting for it. There is no fallback here, though, and there should not be - + // an embedding model is a schema commitment and nothing can stand in for one. + // The gate decides only whether the deployment STARTS; asking for the service + // still throws. if (setupRequired) { logger.warn( "Embedding model '{}' for role '{}' is not registered. This deployment is awaiting a key, " + diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt index eefc12ab6..512fcab1d 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/ModelProvider.kt @@ -31,7 +31,12 @@ interface ModelProvider : HasInfoString { /** * Resolve any role in these options to a concrete model, applying whatever hyperparameters are - * configured against that role. Values the caller set explicitly are kept. + * configured against that role. + * + * Hyperparameters the caller set explicitly are kept - a role may say `temperature: 0.3`, but a + * caller that passed its own temperature still gets that one. Model selection is the exception + * and comes from the role wholesale: deciding which model a role means is the entire point of + * resolving it, so a `model` set alongside a role is replaced rather than preserved. * * Called once per LLM operation, before the model is chosen, so that a role can carry more than * a model name. Options naming no role are returned unchanged. diff --git a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt index 46532491d..7ba24f447 100644 --- a/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt +++ b/embabel-agent-api/src/main/kotlin/com/embabel/common/ai/model/RoleResolver.kt @@ -79,8 +79,28 @@ fun interface RoleResolver { } /** - * Builds an [LlmService] from a user-supplied key. Provider modules contribute implementations; - * a bring-your-own-key application does not need to write one. + * Builds an [LlmService] from a user-supplied key. + * + * No implementation ships with the platform yet, so an application returning + * [RoleResolution.Credential] must register one - otherwise the role fails with + * [NoSuitableModelException] and a log line naming the provider nothing handled. It is a one-liner + * over the provider's own BYOK factory: + * + * ```kotlin + * @Bean + * fun openAiCredentialFactory() = CredentialLlmServiceFactory { credential, model -> + * if (!credential.provider.equals(OpenAiModels.PROVIDER, ignoreCase = true)) null + * else OpenAiCompatibleModelFactory(baseUrl = null, apiKey = credential.apiKey) + * .openAiCompatibleLlm(model = model, provider = OpenAiModels.PROVIDER) + * } + * ``` + * + * The platform caches what this returns, per (provider, key, model), so an implementation should + * build rather than maintain a cache of its own. + * + * Provider-supplied implementations are follow-up work: they belong with the provider modules, and + * a pure bring-your-own-key deployment deliberately has no provider autoconfiguration on the + * classpath to carry them. */ fun interface CredentialLlmServiceFactory { diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt index 7e1fcd581..71971328a 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/common/ai/model/RoleResolutionTest.kt @@ -154,19 +154,23 @@ class RoleResolutionTest { inner class UnsatisfiableRoles { @Test - fun `a configured role whose model is not registered throws`() { - // Configured through the nested shape, which warns rather than failing at startup: the - // flat map is fatal at construction in a keyed deployment, so it cannot reach here. - val mp = provider( - models = listOf(defaultModel), - properties = ConfigurableModelProviderProperties( - roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("a-model-nobody-registered"))), - defaultLlm = "gpt-4.1-mini", - ), - ) - assertThrows { - mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + fun `a nested role naming an unregistered model for our own provider is fatal at startup`() { + // Both shapes get the same rule, which is the point of routing them through one place: + // an earlier revision made the nested shape warn while the flat map died, so the same + // typo was fatal or silent depending on which shape you happened to write it in. + val e = assertThrows { + provider( + models = listOf(defaultModel), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("a-model-nobody-registered"))), + defaultLlm = "gpt-4.1-mini", + ), + ) } + assertTrue( + e.message!!.contains("a-model-nobody-registered") && e.message!!.contains(CHEAPEST_ROLE), + "the failure must name the model and the role: ${e.message}", + ) } @Test @@ -186,18 +190,20 @@ class RoleResolutionTest { @Test fun `an unsatisfiable role never silently resolves to the default LLM`() { // "cheapest" quietly becoming the deployment's most capable model is the failure mode - // that motivates throwing here rather than falling back. + // this whole area exists to prevent. A keyed deployment now never even boots into that + // state, so the guard is asserted at startup - and the message has to name the role, + // since "some model is missing" is what gets fixed by adding a fallback. val expensiveDefault = llm("gpt-4.1-mini", "openai") - val mp = provider( - models = listOf(expensiveDefault), - properties = ConfigurableModelProviderProperties( - roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nano"))), - defaultLlm = "gpt-4.1-mini", - ), - ) - assertThrows { - mp.getLlm(ByRoleModelSelectionCriteria(CHEAPEST_ROLE)) + val e = assertThrows { + provider( + models = listOf(expensiveDefault), + properties = ConfigurableModelProviderProperties( + roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nano"))), + defaultLlm = "gpt-4.1-mini", + ), + ) } + assertTrue(e.message!!.contains(CHEAPEST_ROLE), "the failure must name the role: ${e.message}") } @Test @@ -250,23 +256,22 @@ class RoleResolutionTest { } @Test - fun `a nested role naming an unregistered model for our own provider warns at startup`() { - // The nested shape is the one case where configuration can take a role AWAY: the entry - // is found, its model is not registered, and resolution throws rather than falling back - // to the flat map. A typo there was silent until something asked for the role. + fun `a nested role awaiting a key warns instead of failing, like the flat map`() { + // The other side of the gate for the nested shape. Both shapes route through + // reportUnsatisfiableRole precisely so this half cannot drift apart from the flat + // map's - keyed deployments die, deployments awaiting a key boot and report. val warnings = captureWarnings { provider( - models = listOf(defaultModel), + models = listOf(placeholderModel), properties = ConfigurableModelProviderProperties( - llms = mapOf(CHEAPEST_ROLE to "gpt-4.1-mini"), - roles = mapOf(CHEAPEST_ROLE to mapOf("openai" to LlmOptions.withModel("gpt-4.1-nanoo"))), - defaultLlm = "gpt-4.1-mini", + roles = mapOf(CHEAPEST_ROLE to mapOf("none" to LlmOptions.withModel("gpt-4.1-nanoo"))), + defaultLlm = "setup-required", ), ) } assertTrue( warnings.any { it.contains("gpt-4.1-nanoo") && it.contains(CHEAPEST_ROLE) }, - "the typo must be reported at startup: $warnings", + "the entry must still be reported, just not fatally: $warnings", ) } @@ -290,8 +295,10 @@ class RoleResolutionTest { } @Test - fun `a nested role naming no model at all warns`() { - val warnings = captureWarnings { + fun `a nested role naming no model at all is fatal too`() { + // Tuning without a model cannot satisfy a role, so it is the same misconfiguration as + // naming a model nothing registers and gets the same treatment. + val e = assertThrows { provider( models = listOf(defaultModel), properties = ConfigurableModelProviderProperties( @@ -300,10 +307,7 @@ class RoleResolutionTest { ), ) } - assertTrue( - warnings.any { it.contains("names no model") }, - "tuning without a model cannot satisfy a role: $warnings", - ) + assertTrue(e.message!!.contains("names no model"), e.message) } @Test From a9e0ab0ac064a61c0ad49aa5294c075a02c30b50 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Tue, 11 Aug 2026 07:29:27 +1000 Subject: [PATCH 11/12] Take the wrapped ObjectMapper in the new role-wiring test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1887 replaced ChatClientLlmOperations' `objectMapper` parameter with an `EmbabelObjectMapperHolder` and updated every call site that existed then. RoleResolutionWiringTest arrives on this branch, so it was not there to be updated, and neither PR conflicts with the other textually — the break only appears in the merge CI builds, which is where it did. Use the holder's default, as the other ChatClientLlmOperations tests do. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/embabel/agent/spi/support/RoleResolutionWiringTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt index e780277fd..ecf577b54 100644 --- a/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt +++ b/embabel-agent-api/src/test/kotlin/com/embabel/agent/spi/support/RoleResolutionWiringTest.kt @@ -36,6 +36,7 @@ import com.embabel.common.ai.model.DefaultOptionsConverter import com.embabel.common.ai.model.LlmOptions import com.embabel.common.ai.model.ModelProvider.Companion.CHEAPEST_ROLE import com.embabel.common.textio.template.JinjavaTemplateRenderer +import com.embabel.common.util.EmbabelObjectMapperHolder import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -226,7 +227,7 @@ class RoleResolutionWiringTest { validator = Validation.buildDefaultValidatorFactory().validator, validationPromptGenerator = DefaultValidationPromptGenerator(), templateRenderer = JinjavaTemplateRenderer(), - objectMapper = jacksonObjectMapper(), + embabelObjectMapperHolder = EmbabelObjectMapperHolder.createDefault(), dataBindingProperties = LlmDataBindingProperties(), asyncer = ExecutorAsyncer(Executors.newCachedThreadPool()), ), From bc80c2169a69db29f9f92d1a81cc78dce3808450 Mon Sep 17 00:00:00 2001 From: jasper blues Date: Wed, 12 Aug 2026 08:09:43 +1000 Subject: [PATCH 12/12] Let a role awaiting a key resolve to the placeholder, as the default LLM does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1890 asserted that an unsatisfiable role in a pure BYOK deployment throws NoSuitableModelException; this branch makes it resolve to the placeholder. The two do not conflict textually, so both merged clean and the merge failed. Resolving wins, on the ground #1890 itself argues from. Its concern is that "no key configured" must never become an empty or broken answer later — but the exception does not serve that concern well. It names the role and lists what IS registered, which in a pure BYOK deployment is the placeholder alone: "no model for role best, available: setup-required". The reader is told the wrong problem. And `default-llm` already resolves to the placeholder in exactly this deployment, so a role that throws is the odd one out. The half of the old assertion that mattered is kept as its own test: resolution is tolerant, USE is not. A prompt reaching the placeholder still fails with the message naming withLlmService, so nothing is silent. Deployments holding a key are untouched — an unregistered role name is still fatal at startup, covered above and unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../models/byok/PureByokWithRolesTest.kt | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/embabel-agent-autoconfigure/models/embabel-agent-byok-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/byok/PureByokWithRolesTest.kt b/embabel-agent-autoconfigure/models/embabel-agent-byok-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/byok/PureByokWithRolesTest.kt index 4287bfef2..7b255fb90 100644 --- a/embabel-agent-autoconfigure/models/embabel-agent-byok-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/byok/PureByokWithRolesTest.kt +++ b/embabel-agent-autoconfigure/models/embabel-agent-byok-autoconfigure/src/test/kotlin/com/embabel/agent/config/models/byok/PureByokWithRolesTest.kt @@ -16,6 +16,7 @@ package com.embabel.agent.config.models.byok import com.embabel.agent.spi.support.springai.SpringAiLlmService +import com.embabel.common.ai.model.AiModel import com.embabel.common.ai.model.ByRoleModelSelectionCriteria import com.embabel.common.ai.model.ConfigurableModelProvider import com.embabel.common.ai.model.ConfigurableModelProviderProperties @@ -27,6 +28,9 @@ import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatCode import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test +import org.springframework.ai.chat.messages.UserMessage +import org.springframework.ai.chat.model.ChatModel +import org.springframework.ai.chat.prompt.Prompt /** * A pure BYOK deployment holds no provider key, so no model a role names is registered — and @@ -139,12 +143,39 @@ class PureByokWithRolesTest { } @Test - fun `an unsatisfiable role fails when asked for, rather than resolving to the placeholder`() { - // Silently handing back the placeholder would turn "no key configured" into an empty or - // broken answer at some unrelated point later. The role has no model; say so. + fun `a role awaiting a key resolves to the placeholder, like the default LLM already does`() { + /* + * This test used to require NoSuitableModelException. The concern behind it stands - "no key + * configured" must never become an empty or broken answer somewhere later - but throwing + * here is the wrong way to serve it, for two reasons. + * + * The exception reports the wrong problem. It names the role and lists what IS registered, + * which in a pure BYOK deployment is the placeholder alone: "no model for role best, + * available: setup-required". The actual problem is that no key has been set, and that is + * not what the reader is told. + * + * And it makes a role behave differently from the default LLM in the one deployment where + * they are in the same position. `default-llm` already resolves to the placeholder here - + * see the two tests above - so a role that throws is the odd one out. + * + * Nothing is silent either way: the placeholder is not a working model, and the next test + * pins what happens when a prompt actually reaches it. + */ + val modelProvider = pureByok(mapOf(BEST_ROLE to "gpt-4.1")) + + assertThat(modelProvider.getLlm(ByRoleModelSelectionCriteria(BEST_ROLE)).name) + .isEqualTo(SetupRequiredLlm.NAME) + } + + @Test + fun `and using that role fails with the message that says to add a key`() { + // The half of the old assertion that mattered: resolving is tolerant, USING it is not. The + // deployment gets the actionable error rather than a plausible-looking answer. val modelProvider = pureByok(mapOf(BEST_ROLE to "gpt-4.1")) + val llm = modelProvider.getLlm(ByRoleModelSelectionCriteria(BEST_ROLE)) - assertThatThrownBy { modelProvider.getLlm(ByRoleModelSelectionCriteria(BEST_ROLE)) } - .isInstanceOf(NoSuitableModelException::class.java) + assertThatThrownBy { ((llm as AiModel<*>).model as ChatModel).call(Prompt(listOf(UserMessage("hello")))) } + .isInstanceOf(NoLlmConfiguredException::class.java) + .hasMessageContaining("withLlmService") } }