diff --git a/lib/crates/fabro-cli/src/commands/model.rs b/lib/crates/fabro-cli/src/commands/model.rs index d5d41bb20..4e5bd6f39 100644 --- a/lib/crates/fabro-cli/src/commands/model.rs +++ b/lib/crates/fabro-cli/src/commands/model.rs @@ -45,6 +45,10 @@ struct CompletedModelTest { status: String, } +fn model_matches_selector(model: &Model, selector: &str) -> bool { + model.id == selector || model.aliases.iter().any(|alias| alias == selector) +} + fn find_model_by_id_or_alias( models: &[Model], id: &str, @@ -54,7 +58,7 @@ fn find_model_by_id_or_alias( .iter() .find(|model| { provider.is_none_or(|provider| &model.provider == provider) - && (model.id == id || model.aliases.iter().any(|alias| alias == id)) + && model_matches_selector(model, id) }) .cloned() } @@ -275,10 +279,9 @@ async fn test_models_via_server( if !json_output { eprint!("Testing {model_id}..."); } - let has_configured_match = listed_models.iter().any(|model| { - model.configured - && (model.id == model_id || model.aliases.iter().any(|alias| alias == model_id)) - }); + let has_configured_match = listed_models + .iter() + .any(|model| model.configured && model_matches_selector(model, model_id)); let result = if requested_provider.is_none() && listed_info.is_some() && !has_configured_match { None diff --git a/lib/crates/fabro-config/src/layers/llm.rs b/lib/crates/fabro-config/src/layers/llm.rs index 32169b980..ca29d2bbd 100644 --- a/lib/crates/fabro-config/src/layers/llm.rs +++ b/lib/crates/fabro-config/src/layers/llm.rs @@ -25,7 +25,7 @@ //! Resolution against the static adapter registry happens in `fabro-model` //! when the resolved [`Catalog`](fabro_model::Catalog) is built. -use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::collections::{BTreeMap, HashMap}; use fabro_model::catalog::deserialize_knowledge_cutoff; use fabro_model::{ @@ -160,90 +160,16 @@ pub struct ModelSettings { pub costs: Option, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum LlmNormalizationError { - BuiltinCatalog { - message: String, - }, - RetiredModel { - identifier: String, - provider: ProviderId, - model: ModelId, - }, - UnknownLegacyModel { - model: String, - }, - AmbiguousLegacyModel { - model: String, - candidates: Vec<(ProviderId, ModelId)>, - }, - AmbiguousProviderModel { - provider: ProviderId, - selector: String, - models: Vec, - }, - DuplicateModelDefinition { - provider: ProviderId, - model: ModelId, - }, - ScopedModelDeclaresProvider { - provider: ProviderId, - model: ModelId, - }, -} - -impl std::fmt::Display for LlmNormalizationError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::BuiltinCatalog { message } => { - write!(f, "failed to inspect the built-in model catalog: {message}") - } - Self::RetiredModel { - identifier, - provider, - model, - } => write!( - f, - "model identifier '{identifier}' has been retired; use '{provider}/{model}' instead" - ), - Self::UnknownLegacyModel { model } => write!( - f, - "legacy model row '{model}' omits provider and does not match a unique known offering" - ), - Self::AmbiguousLegacyModel { model, candidates } => write!( - f, - "legacy model row '{model}' omits provider and matches multiple offerings: {candidates:?}" - ), - Self::AmbiguousProviderModel { - provider, - selector, - models, - } => write!( - f, - "legacy model selector '{selector}' is ambiguous on provider '{provider}': {models:?}" - ), - Self::DuplicateModelDefinition { provider, model } => write!( - f, - "provider '{provider}' model '{model}' is defined through both provider-scoped and legacy top-level syntax" - ), - Self::ScopedModelDeclaresProvider { provider, model } => write!( - f, - "provider-scoped model '{provider}/{model}' must not declare a provider field" - ), - } - } -} - -impl std::error::Error for LlmNormalizationError {} - impl LlmLayer { /// Normalize the temporary legacy model table before this source is - /// combined with any other settings source. - pub(crate) fn normalize_legacy_models(&mut self) -> Result<(), LlmNormalizationError> { + /// combined with any other settings source. Resolution runs against this + /// layer's own providers plus the built-in catalog, because a single + /// source may reference built-in offerings that merge in later. + pub(crate) fn normalize_legacy_models(&mut self) -> Result<(), catalog::LegacyModelError> { for (provider, settings) in self.providers.iter() { for (model, settings) in settings.models.iter() { if settings.provider.is_some() { - return Err(LlmNormalizationError::ScopedModelDeclaresProvider { + return Err(catalog::LegacyModelError::ScopedModelDeclaresProvider { provider: ProviderId::new(provider.clone()), model: ModelId::new(model.clone()), }); @@ -251,46 +177,45 @@ impl LlmLayer { } } - let mut legacy_models = std::mem::take(&mut self.models.0) - .into_iter() - .collect::>(); + let legacy_models = std::mem::take(&mut self.models.0); + if legacy_models.is_empty() { + return Ok(()); + } + let mut legacy_models = legacy_models.into_iter().collect::>(); legacy_models.sort_by(|(left, _), (right, _)| left.cmp(right)); - for (legacy_id, mut settings) in legacy_models { - if let Some((provider, model)) = catalog::retired_model_replacement(&legacy_id) { - return Err(LlmNormalizationError::RetiredModel { - identifier: legacy_id, - provider, - model, - }); - } + let mut index = catalog::LegacyModelIndex::default(); + let mut provider_ids = self.providers.keys().cloned().collect::>(); + provider_ids.sort_unstable(); + for provider_id in &provider_ids { + let settings = self + .providers + .get(provider_id) + .expect("provider ID came from provider map keys"); + let mut model_ids = settings.models.keys().cloned().collect::>(); + model_ids.sort_unstable(); + index.add_provider( + ProviderId::new(provider_id.clone()), + settings.aliases.clone().unwrap_or_default(), + model_ids.into_iter().map(|model_id| { + let model = settings + .models + .get(&model_id) + .expect("model ID came from model map keys"); + let aliases = model.aliases.clone().unwrap_or_default(); + (ModelId::new(model_id), aliases) + }), + ); + } + let index = index.with_builtin()?; + for (legacy_id, mut settings) in legacy_models { let explicit_provider = settings.provider.take(); - let (provider, model) = if let Some(provider) = explicit_provider { - let provider = self.canonical_provider(&provider)?; - let model = self - .canonical_model_on_provider(&provider, &legacy_id)? - .unwrap_or_else(|| ModelId::new(legacy_id.clone())); - (provider, model) - } else { - let candidates = self.model_candidates(&legacy_id)?; - match candidates.as_slice() { - [(provider, model)] => (provider.clone(), model.clone()), - [] => { - return Err(LlmNormalizationError::UnknownLegacyModel { model: legacy_id }); - } - _ => { - return Err(LlmNormalizationError::AmbiguousLegacyModel { - model: legacy_id, - candidates, - }); - } - } - }; + let (provider, model) = index.resolve(&legacy_id, explicit_provider.as_deref())?; let provider_settings = self.providers.entry(provider.to_string()).or_default(); if provider_settings.models.contains_key(model.as_str()) { - return Err(LlmNormalizationError::DuplicateModelDefinition { provider, model }); + return Err(catalog::LegacyModelError::DuplicateModel { provider, model }); } provider_settings .models @@ -298,139 +223,6 @@ impl LlmLayer { } Ok(()) } - - fn canonical_provider(&self, selector: &str) -> Result { - if self.providers.contains_key(selector) { - return Ok(ProviderId::new(selector)); - } - let mut aliases = self - .providers - .iter() - .filter(|(_, settings)| { - settings - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - .map(|(provider, _)| ProviderId::new(provider.clone())) - .collect::>(); - aliases.sort(); - if let Some(provider) = aliases.into_iter().next() { - return Ok(provider); - } - catalog::builtin_provider_id(selector) - .map_err(|error| normalization_catalog_error(&error)) - .map(|provider| provider.unwrap_or_else(|| ProviderId::new(selector))) - } - - fn canonical_model_on_provider( - &self, - provider: &ProviderId, - selector: &str, - ) -> Result, LlmNormalizationError> { - let current = self - .providers - .get(provider.as_str()) - .map(|settings| &settings.models); - if current.is_some_and(|models| models.contains_key(selector)) { - return Ok(Some(ModelId::new(selector))); - } - - let builtin_canonical = catalog::builtin_canonical_model_offerings(selector) - .map_err(|error| normalization_catalog_error(&error))? - .into_iter() - .find_map(|(candidate_provider, model)| { - (candidate_provider == *provider).then_some(model) - }); - if builtin_canonical.is_some() { - return Ok(builtin_canonical); - } - - let mut aliases = current - .into_iter() - .flat_map(|models| models.iter()) - .filter(|(_, settings)| { - settings - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - .map(|(model, _)| ModelId::new(model.clone())) - .collect::>(); - aliases.extend( - catalog::builtin_alias_model_offerings(selector) - .map_err(|error| normalization_catalog_error(&error))? - .into_iter() - .filter_map(|(candidate_provider, model)| { - (candidate_provider == *provider).then_some(model) - }), - ); - match aliases.len() { - 0 => Ok(None), - 1 => Ok(aliases.into_iter().next()), - _ => Err(LlmNormalizationError::AmbiguousProviderModel { - provider: provider.clone(), - selector: selector.to_string(), - models: aliases.into_iter().collect(), - }), - } - } - - fn model_candidates( - &self, - selector: &str, - ) -> Result, LlmNormalizationError> { - let mut canonical = self - .providers - .iter() - .filter(|(_, settings)| settings.models.contains_key(selector)) - .map(|(provider, _)| { - ( - ProviderId::new(provider.clone()), - ModelId::new(selector.to_string()), - ) - }) - .collect::>(); - canonical.extend( - catalog::builtin_canonical_model_offerings(selector) - .map_err(|error| normalization_catalog_error(&error))?, - ); - if !canonical.is_empty() { - return Ok(canonical.into_iter().collect()); - } - - let mut aliases = - self.providers - .iter() - .flat_map(|(provider, settings)| { - settings - .models - .iter() - .filter(move |(_, settings)| { - settings.aliases.as_ref().is_some_and(|aliases| { - aliases.iter().any(|alias| alias == selector) - }) - }) - .map(move |(model, _)| { - ( - ProviderId::new(provider.clone()), - ModelId::new(model.clone()), - ) - }) - }) - .collect::>(); - aliases.extend( - catalog::builtin_alias_model_offerings(selector) - .map_err(|error| normalization_catalog_error(&error))?, - ); - Ok(aliases.into_iter().collect()) - } -} - -fn normalization_catalog_error(error: &catalog::CatalogBuildError) -> LlmNormalizationError { - LlmNormalizationError::BuiltinCatalog { - message: error.to_string(), - } } #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] diff --git a/lib/crates/fabro-config/src/layers/mod.rs b/lib/crates/fabro-config/src/layers/mod.rs index c25838781..c3fa1632c 100644 --- a/lib/crates/fabro-config/src/layers/mod.rs +++ b/lib/crates/fabro-config/src/layers/mod.rs @@ -21,9 +21,9 @@ pub use environment::{ EnvironmentNetworkLayer, EnvironmentResourcesLayer, RunEnvironmentLayer, }; pub use llm::{ - CostRates, CredentialRef, CredentialRefParseError, LlmLayer, LlmNormalizationError, - ModelControls, ModelCostTable, ModelFeatures as LlmModelFeatures, - ModelLimits as LlmModelLimits, ModelSettings, ProviderSettings, ReasoningEffortFeature, + CostRates, CredentialRef, CredentialRefParseError, LlmLayer, ModelControls, ModelCostTable, + ModelFeatures as LlmModelFeatures, ModelLimits as LlmModelLimits, ModelSettings, + ProviderSettings, ReasoningEffortFeature, }; pub use log_filter::LogFilter; pub use maps::{MergeMap, ReplaceMap, StickyMap}; diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 694ab1773..f6097b9be 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -46,19 +46,18 @@ pub use layers::{ CredentialRefParseError, EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, EnvironmentLifecycleLayer, EnvironmentNetworkLayer, EnvironmentResourcesLayer, GitAuthorLayer, GithubIntegrationLayer, HookAgentMarker, HookEntry, HookTlsMode, IntegrationWebhooksLayer, - InterviewProviderLayer, InterviewsLayer, LlmLayer, LlmModelFeatures, LlmModelLimits, - LlmNormalizationError, LogFilter, McpEntryLayer, MergeMap, ModelControls, ModelCostTable, - ModelRefOrSplice, ModelSettings, NotificationProviderLayer, NotificationRouteLayer, - ObjectStoreLocalLayer, ObjectStoreS3Layer, PrepareStep, ProjectLayer, ProviderSettings, - ReasoningEffortFeature, ReplaceMap, RunAgentLayer, RunArtifactsLayer, RunCheckpointLayer, - RunCloneLayer, RunEnvironmentLayer, RunExecutionLayer, RunGitLayer, RunGoalLayer, - RunIntegrationsGithubLayer, RunIntegrationsLayer, RunLayer, RunMetaBranchLayer, - RunModelControlsLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunRunBranchLayer, - RunScmLayer, ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer, ServerAuthGithubLayer, - ServerAuthLayer, ServerIntegrationsLayer, ServerLayer, ServerListenLayer, ServerLoggingLayer, - ServerSandboxLayer, ServerSandboxProviderLayer, ServerSandboxProvidersLayer, - ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer, SettingsLayer, - SlackIntegrationLayer, StickyMap, StringOrSplice, WorkflowLayer, + InterviewProviderLayer, InterviewsLayer, LlmLayer, LlmModelFeatures, LlmModelLimits, LogFilter, + McpEntryLayer, MergeMap, ModelControls, ModelCostTable, ModelRefOrSplice, ModelSettings, + NotificationProviderLayer, NotificationRouteLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, + PrepareStep, ProjectLayer, ProviderSettings, ReasoningEffortFeature, ReplaceMap, RunAgentLayer, + RunArtifactsLayer, RunCheckpointLayer, RunCloneLayer, RunEnvironmentLayer, RunExecutionLayer, + RunGitLayer, RunGoalLayer, RunIntegrationsGithubLayer, RunIntegrationsLayer, RunLayer, + RunMetaBranchLayer, RunModelControlsLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, + RunRunBranchLayer, RunScmLayer, ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer, + ServerAuthGithubLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerLayer, + ServerListenLayer, ServerLoggingLayer, ServerSandboxLayer, ServerSandboxProviderLayer, + ServerSandboxProvidersLayer, ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, + ServerWebLayer, SettingsLayer, SlackIntegrationLayer, StickyMap, StringOrSplice, WorkflowLayer, }; pub use logging::{resolve_log_destination, resolve_log_destination_with_env}; pub use parse::ParseError; diff --git a/lib/crates/fabro-config/src/parse.rs b/lib/crates/fabro-config/src/parse.rs index 025e0b130..fb72c35af 100644 --- a/lib/crates/fabro-config/src/parse.rs +++ b/lib/crates/fabro-config/src/parse.rs @@ -1,7 +1,8 @@ use std::fmt; +use fabro_model::catalog::LegacyModelError; + use crate::SettingsLayer; -use crate::layers::LlmNormalizationError; const CURRENT_VERSION: u32 = 1; @@ -31,7 +32,7 @@ const LEGACY_LLM_KEYS: &[&str] = &[ #[derive(Debug, Clone, PartialEq, Eq)] pub enum ParseError { Toml(String), - LlmCatalog(LlmNormalizationError), + LlmCatalog(LegacyModelError), Version(VersionError), UnknownTopLevelKey { key: String, @@ -325,7 +326,7 @@ provider = "openai" assert!(matches!( error, - ParseError::LlmCatalog(LlmNormalizationError::ScopedModelDeclaresProvider { + ParseError::LlmCatalog(LegacyModelError::ScopedModelDeclaresProvider { provider, model, }) if provider.as_str() == "openai" && model.as_str() == "gpt-5.4" @@ -398,7 +399,7 @@ display_name = "Ambiguous" assert!(matches!( error, - ParseError::LlmCatalog(LlmNormalizationError::AmbiguousLegacyModel { + ParseError::LlmCatalog(LegacyModelError::AmbiguousModel { model, candidates, }) if model == "gpt-5.6-sol" && candidates.len() >= 2 @@ -420,7 +421,7 @@ display_name = "Legacy" assert!(matches!( error, - ParseError::LlmCatalog(LlmNormalizationError::DuplicateModelDefinition { + ParseError::LlmCatalog(LegacyModelError::DuplicateModel { provider, model, }) if provider.as_str() == "openai" && model.as_str() == "gpt-5.4" @@ -438,7 +439,7 @@ provider = "openrouter" assert!(matches!( error, - ParseError::LlmCatalog(LlmNormalizationError::RetiredModel { + ParseError::LlmCatalog(LegacyModelError::Retired { identifier, provider, model, diff --git a/lib/crates/fabro-llm/src/client.rs b/lib/crates/fabro-llm/src/client.rs index e40a8ede2..446b0b77a 100644 --- a/lib/crates/fabro-llm/src/client.rs +++ b/lib/crates/fabro-llm/src/client.rs @@ -309,20 +309,19 @@ impl Client { let eligible = self.eligible_provider_ids(); if let Some(explicit) = request.provider.as_deref() { let explicit = ProviderId::new(explicit); - if let Some(catalog_provider) = catalog.provider(&explicit) { + if catalog.provider(&explicit).is_some() { + let selected = catalog + .resolve_selection(Some(&request.model), Some(&explicit), &eligible) + .map_err(selection_error)?; let provider = self - .provider_adapter(catalog_provider.id.as_str()) + .provider_adapter(selected.provider.as_str()) .ok_or_else(|| { selection_error(ModelSelectionError::ProviderUnavailable { - provider: catalog_provider.id.clone(), + provider: selected.provider.clone(), }) })?; - match catalog.resolve_on_provider(&catalog_provider.id, &request.model) { - Ok(model) => resolved.model = model.id.to_string(), - Err(ModelSelectionError::UnknownSelectorOnProvider { .. }) => {} - Err(error) => return Err(selection_error(error)), - } - resolved.provider = Some(catalog_provider.id.to_string()); + resolved.model = selected.model; + resolved.provider = Some(selected.provider.into_inner()); return Ok(ResolvedRequest { provider, request: resolved, diff --git a/lib/crates/fabro-llm/src/providers/anthropic.rs b/lib/crates/fabro-llm/src/providers/anthropic.rs index 539b2b0fc..83894e9a9 100644 --- a/lib/crates/fabro-llm/src/providers/anthropic.rs +++ b/lib/crates/fabro-llm/src/providers/anthropic.rs @@ -7,7 +7,7 @@ use crate::codec::anthropic_messages::{AnthropicMessages, anthropic_option}; use crate::codec::{AnthropicVersion, Codec, CodecCtx, CodecParams, EncodedRequest}; use crate::error::Error; use crate::provider::{self, ProviderAdapter, StreamEventStream}; -use crate::providers::common::{self as common}; +use crate::providers::common::{self as common, CatalogRoute}; use crate::token_count::{InputTokenCount, InputTokenCountMethod}; use crate::transport::{self, HttpTransport, SseFraming}; use crate::types::{AdapterTimeout, Request, Response, StreamEvent}; @@ -118,11 +118,7 @@ impl Adapter { request, provider_name: &self.provider_name, deployment_id, - model: common::catalog_model( - self.catalog.as_deref(), - &self.provider_name, - &request.model, - ), + model: self.catalog_model(&request.model), params, } } @@ -214,6 +210,16 @@ fn anthropic_thinking_type(provider_options: Option<&serde_json::Value>) -> Opti .and_then(serde_json::Value::as_str) } +impl common::CatalogRoute for Adapter { + fn catalog(&self) -> Option<&Catalog> { + self.catalog.as_deref() + } + + fn provider_name(&self) -> &str { + &self.provider_name + } +} + #[async_trait::async_trait] impl ProviderAdapter for Adapter { fn name(&self) -> &str { @@ -232,11 +238,7 @@ impl ProviderAdapter for Adapter { self.validate_request(request)?; let resolved = self.resolve_request(request).await; let codec = AnthropicMessages; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let ctx = self.codec_ctx(&resolved, &deployment_id, &route.codec_params); let Some(encoded) = codec.encode_count_tokens(&ctx).transpose()? else { @@ -272,11 +274,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = AnthropicMessages; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let ctx = self.codec_ctx(&resolved, &deployment_id, &route.codec_params); let encoded = codec.encode(&ctx, false)?; @@ -293,11 +291,7 @@ impl ProviderAdapter for Adapter { let route = self.route_config(); let resolved = self.resolve_request(request).await; let codec = AnthropicMessages; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let ctx = self.codec_ctx(&resolved, &deployment_id, &route.codec_params); let encoded = codec.encode(&ctx, true)?; @@ -323,8 +317,7 @@ impl ProviderAdapter for Adapter { // Always-adaptive models reject manual enabled/disabled thinking // configs at the API, so fail them locally with a clear message // instead. - let model_info = - common::catalog_model(self.catalog.as_deref(), &self.provider_name, &request.model); + let model_info = self.catalog_model(&request.model); if let Some(model) = model_info .filter(|m| m.features.reasoning_effort == ReasoningEffortFeature::AlwaysAdaptive) { diff --git a/lib/crates/fabro-llm/src/providers/bedrock/mod.rs b/lib/crates/fabro-llm/src/providers/bedrock/mod.rs index 97c15dc02..deca69bd0 100644 --- a/lib/crates/fabro-llm/src/providers/bedrock/mod.rs +++ b/lib/crates/fabro-llm/src/providers/bedrock/mod.rs @@ -29,7 +29,7 @@ use crate::codec::bedrock_converse::BedrockConverse; use crate::codec::{Codec, CodecCtx, CodecParams, EncodedRequest, RawEvent, StreamDecoder}; use crate::error::Error; use crate::provider::{self, ProviderAdapter, StreamEventStream}; -use crate::providers::common::{self as common}; +use crate::providers::common::{self as common, CatalogRoute}; use crate::transport::{self, HttpTransport}; use crate::types::{AdapterTimeout, Request, Response, StreamEvent}; @@ -169,11 +169,7 @@ impl Adapter { request, provider_name: &self.provider_name, deployment_id, - model: common::catalog_model( - self.catalog.as_deref(), - &self.provider_name, - &request.model, - ), + model: self.catalog_model(&request.model), params, } } @@ -232,6 +228,16 @@ impl Adapter { } } +impl common::CatalogRoute for Adapter { + fn catalog(&self) -> Option<&Catalog> { + self.catalog.as_deref() + } + + fn provider_name(&self) -> &str { + &self.provider_name + } +} + #[async_trait::async_trait] impl ProviderAdapter for Adapter { fn name(&self) -> &str { @@ -243,11 +249,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = BedrockConverse; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = CodecParams::default(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); @@ -261,11 +263,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = BedrockConverse; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = CodecParams::default(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); diff --git a/lib/crates/fabro-llm/src/providers/common.rs b/lib/crates/fabro-llm/src/providers/common.rs index f85397935..6eff3b727 100644 --- a/lib/crates/fabro-llm/src/providers/common.rs +++ b/lib/crates/fabro-llm/src/providers/common.rs @@ -20,6 +20,23 @@ pub fn api_model_id(catalog: Option<&Catalog>, provider: &str, model: &str) -> S .map_or_else(|| model.to_string(), |settings| settings.api_id.clone()) } +/// Adapters that route models through an optional catalog scoped to one +/// provider name. +pub trait CatalogRoute { + fn catalog(&self) -> Option<&Catalog>; + fn provider_name(&self) -> &str; + + /// Catalog offering for a canonical ID or alias on this provider. + fn catalog_model(&self, model: &str) -> Option<&Model> { + catalog_model(self.catalog(), self.provider_name(), model) + } + + /// Identifier sent to the provider API for a model. + fn api_model_id(&self, model: &str) -> String { + api_model_id(self.catalog(), self.provider_name(), model) + } +} + /// Check if a URL string looks like a local file path. #[must_use] pub fn is_file_path(url: &str) -> bool { diff --git a/lib/crates/fabro-llm/src/providers/gemini.rs b/lib/crates/fabro-llm/src/providers/gemini.rs index a9e2fab0f..e2a33337b 100644 --- a/lib/crates/fabro-llm/src/providers/gemini.rs +++ b/lib/crates/fabro-llm/src/providers/gemini.rs @@ -9,7 +9,7 @@ use crate::error::Error; use crate::provider::{ ProviderAdapter, StreamEventStream, validate_standard_speed, validate_tool_choice, }; -use crate::providers::common::{self as common}; +use crate::providers::common::{self as common, CatalogRoute}; use crate::token_count::{InputTokenCount, InputTokenCountMethod}; use crate::transport::{self, HttpTransport, SseFraming}; use crate::types::{AdapterTimeout, Request, Response}; @@ -102,11 +102,7 @@ impl Adapter { request, provider_name: &self.provider_name, deployment_id, - model: common::catalog_model( - self.catalog.as_deref(), - &self.provider_name, - &request.model, - ), + model: self.catalog_model(&request.model), params, } } @@ -129,6 +125,16 @@ impl Adapter { } } +impl common::CatalogRoute for Adapter { + fn catalog(&self) -> Option<&Catalog> { + self.catalog.as_deref() + } + + fn provider_name(&self) -> &str { + &self.provider_name + } +} + #[async_trait::async_trait] impl ProviderAdapter for Adapter { fn name(&self) -> &str { @@ -151,11 +157,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = GeminiGenerate; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = CodecParams::default(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); @@ -185,11 +187,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = GeminiGenerate; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = CodecParams::default(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); @@ -206,11 +204,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = GeminiGenerate; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = CodecParams::default(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index 185af1714..56eb76c3d 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -9,7 +9,7 @@ use crate::error::Error; use crate::provider::{ ProviderAdapter, StreamEventStream, validate_standard_speed, validate_tool_choice, }; -use crate::providers::common::{self as common}; +use crate::providers::common::{self as common, CatalogRoute}; use crate::token_count::{InputTokenCount, InputTokenCountMethod}; use crate::transport::{self, HttpTransport, SseFraming}; use crate::types::{AdapterTimeout, Request, Response, StreamEvent}; @@ -131,11 +131,7 @@ impl Adapter { request, provider_name: &self.provider_name, deployment_id, - model: common::catalog_model( - self.catalog.as_deref(), - &self.provider_name, - &request.model, - ), + model: self.catalog_model(&request.model), params, } } @@ -196,6 +192,16 @@ impl Adapter { } } +impl common::CatalogRoute for Adapter { + fn catalog(&self) -> Option<&Catalog> { + self.catalog.as_deref() + } + + fn provider_name(&self) -> &str { + &self.provider_name + } +} + #[async_trait::async_trait] impl ProviderAdapter for Adapter { fn name(&self) -> &str { @@ -218,11 +224,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = OpenAiResponses; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = self.codec_params(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); @@ -258,11 +260,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = OpenAiResponses; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = self.codec_params(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); @@ -279,11 +277,7 @@ impl ProviderAdapter for Adapter { let resolved = self.resolve_request(request).await; let codec = OpenAiResponses; - let deployment_id = common::api_model_id( - self.catalog.as_deref(), - &self.provider_name, - &resolved.model, - ); + let deployment_id = self.api_model_id(&resolved.model); let params = self.codec_params(); let ctx = self.codec_ctx(&resolved, &deployment_id, ¶ms); diff --git a/lib/crates/fabro-llm/src/providers/openai_compatible.rs b/lib/crates/fabro-llm/src/providers/openai_compatible.rs index 3feb0b577..b46444636 100644 --- a/lib/crates/fabro-llm/src/providers/openai_compatible.rs +++ b/lib/crates/fabro-llm/src/providers/openai_compatible.rs @@ -8,7 +8,7 @@ use crate::error::Error; use crate::provider::{ ProviderAdapter, StreamEventStream, validate_standard_speed, validate_tool_choice, }; -use crate::providers::common::{self as common}; +use crate::providers::common::{self as common, CatalogRoute}; use crate::transport::{self, HttpTransport, SseFraming}; use crate::types::{AdapterTimeout, Request, Response}; @@ -88,7 +88,7 @@ impl Adapter { /// Resolve the wire model id (catalog `api_id`, falling back to the /// requested model). fn deployment_id(&self, request: &Request) -> String { - common::api_model_id(self.catalog.as_deref(), &self.provider_name, &request.model) + self.api_model_id(&request.model) } /// Build the borrowed codec context. `deployment_id` and `params` are @@ -103,11 +103,7 @@ impl Adapter { request, provider_name: &self.provider_name, deployment_id, - model: common::catalog_model( - self.catalog.as_deref(), - &self.provider_name, - &request.model, - ), + model: self.catalog_model(&request.model), params, } } @@ -131,6 +127,16 @@ impl Adapter { } } +impl common::CatalogRoute for Adapter { + fn catalog(&self) -> Option<&Catalog> { + self.catalog.as_deref() + } + + fn provider_name(&self) -> &str { + &self.provider_name + } +} + #[async_trait::async_trait] impl ProviderAdapter for Adapter { fn name(&self) -> &str { diff --git a/lib/crates/fabro-model/src/catalog.rs b/lib/crates/fabro-model/src/catalog.rs index ee362d414..ea089cfba 100644 --- a/lib/crates/fabro-model/src/catalog.rs +++ b/lib/crates/fabro-model/src/catalog.rs @@ -544,33 +544,8 @@ pub enum CatalogBuildError { first: ModelId, second: ModelId, }, - #[error( - "provider '{provider}' model '{model}' is defined through both provider-scoped and legacy top-level syntax" - )] - DuplicateProviderModelDefinition { - provider: ProviderId, - model: ModelId, - }, - #[error("provider-scoped model '{provider}/{model}' must not declare a provider field")] - ScopedModelDeclaresProvider { - provider: ProviderId, - model: ModelId, - }, - #[error("legacy model row '{model}' omits provider and does not match a unique known offering")] - LegacyModelProviderUnknown { model: String }, - #[error( - "legacy model row '{model}' omits provider and matches multiple providers: {providers:?}" - )] - LegacyModelProviderAmbiguous { - model: String, - providers: Vec, - }, - #[error("model identifier '{identifier}' has been retired; use '{provider}/{model}' instead")] - RetiredModelIdentifier { - identifier: String, - provider: ProviderId, - model: ModelId, - }, + #[error(transparent)] + LegacyModel(#[from] LegacyModelError), #[error("provider '{provider}' model '{model}' has an empty api_id")] EmptyModelApiId { provider: ProviderId, @@ -637,6 +612,10 @@ pub enum ModelSelectionError { selector: String, providers: Vec, }, + #[error( + "no default model is available on an eligible provider; providers with defaults: {providers:?}" + )] + NoDefaultModel { providers: Vec }, #[error("model identifier '{identifier}' has been retired; use '{provider}/{model}' instead")] RetiredModelIdentifier { identifier: String, @@ -645,6 +624,15 @@ pub enum ModelSelectionError { }, } +/// One provider/model pair chosen by [`Catalog::resolve_selection`]. The +/// model is the canonical catalog ID when the selector matched an offering, +/// or the caller's selector passed through verbatim when it did not. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SelectedModel { + pub provider: ProviderId, + pub model: String, +} + /// Typed model catalog backed by a `Vec`. /// /// Use [`Catalog::builtin()`] for the embedded settings-backed catalog. @@ -724,13 +712,14 @@ impl Catalog { } if let Some((replacement_provider, replacement_model)) = - retired_model_address(&model_id) + retired_model_replacement(&model_id) { - return Err(CatalogBuildError::RetiredModelIdentifier { + return Err(LegacyModelError::Retired { identifier: model_id, provider: replacement_provider, model: replacement_model, - }); + } + .into()); } let (model, resolved_settings) = build_model(&model_id, model_settings, provider)?; @@ -905,7 +894,8 @@ impl Catalog { provider: &ProviderId, selector: &str, ) -> Result<&Model, ModelSelectionError> { - if let Some((replacement_provider, replacement_model)) = retired_model_address(selector) { + if let Some((replacement_provider, replacement_model)) = retired_model_replacement(selector) + { return Err(ModelSelectionError::RetiredModelIdentifier { identifier: selector.to_string(), provider: replacement_provider, @@ -937,7 +927,8 @@ impl Catalog { explicit_provider: Option<&ProviderId>, eligible_providers: &HashSet, ) -> Result<&'a Model, ModelSelectionError> { - if let Some((replacement_provider, replacement_model)) = retired_model_address(selector) { + if let Some((replacement_provider, replacement_model)) = retired_model_replacement(selector) + { return Err(ModelSelectionError::RetiredModelIdentifier { identifier: selector.to_string(), provider: replacement_provider, @@ -1042,10 +1033,82 @@ impl Catalog { .collect::>(); providers.sort(); providers.dedup(); - Err(ModelSelectionError::NoEligibleOffering { - selector: "".to_string(), - providers, - }) + Err(ModelSelectionError::NoDefaultModel { providers }) + } + + /// Canonicalize a provider ID or alias and require it to be in the + /// eligible snapshot. + pub fn ready_provider( + &self, + provider: &ProviderId, + eligible_providers: &HashSet, + ) -> Result { + let provider = + self.provider(provider) + .ok_or_else(|| ModelSelectionError::UnknownProvider { + provider: provider.clone(), + })?; + let ready = eligible_providers.iter().any(|eligible| { + self.provider(eligible) + .is_some_and(|eligible| eligible.id == provider.id) + }); + if !ready { + return Err(ModelSelectionError::ProviderUnavailable { + provider: provider.id.clone(), + }); + } + Ok(provider.id.clone()) + } + + /// Resolve an optional selector to one provider/model pair, applying the + /// passthrough policy shared by every dispatch boundary: + /// + /// - A selector known to the catalog resolves to its canonical offering. + /// - An unknown selector pinned to a provider passes through verbatim on + /// that provider. + /// - An unqualified unknown selector passes through on the default + /// provider. + /// - No selector picks the default offering (of the pinned provider, when + /// one is given). + pub fn resolve_selection( + &self, + selector: Option<&str>, + explicit_provider: Option<&ProviderId>, + eligible_providers: &HashSet, + ) -> Result { + let Some(selector) = selector else { + let eligible = match explicit_provider { + Some(provider) => { + HashSet::from([self.ready_provider(provider, eligible_providers)?]) + } + None => eligible_providers.clone(), + }; + let offering = self.select_default(&eligible)?; + return Ok(SelectedModel { + provider: offering.provider.clone(), + model: offering.id.to_string(), + }); + }; + match self.select(selector, explicit_provider, eligible_providers) { + Ok(offering) => Ok(SelectedModel { + provider: offering.provider.clone(), + model: offering.id.to_string(), + }), + Err(ModelSelectionError::UnknownSelectorOnProvider { provider, .. }) => { + Ok(SelectedModel { + provider, + model: selector.to_string(), + }) + } + Err(ModelSelectionError::UnknownSelector { .. }) => { + let default = self.select_default(eligible_providers)?; + Ok(SelectedModel { + provider: default.provider.clone(), + model: selector.to_string(), + }) + } + Err(error) => Err(error), + } } #[must_use] @@ -1411,52 +1474,24 @@ fn normalize_catalog_settings( mut settings: LlmCatalogSettings, known: Option<&LlmCatalogSettings>, ) -> Result { - for (provider, settings) in &settings.providers { - for (model, settings) in &settings.models { - if settings.provider.is_some() { - return Err(CatalogBuildError::ScopedModelDeclaresProvider { - provider: ProviderId::new(provider.clone()), - model: ModelId::new(model.clone()), - }); - } - } - } + reject_scoped_provider_fields(&settings)?; let legacy_models = std::mem::take(&mut settings.models); - for (legacy_id, mut model_settings) in legacy_models { - if let Some((provider, model)) = retired_model_address(&legacy_id) { - return Err(CatalogBuildError::RetiredModelIdentifier { - identifier: legacy_id, - provider, - model, - }); - } + if legacy_models.is_empty() { + return Ok(settings); + } + let mut legacy_models = legacy_models.into_iter().collect::>(); + legacy_models.sort_by(|(left, _), (right, _)| left.cmp(right)); + let mut index = LegacyModelIndex::default(); + index.add_settings(&settings); + if let Some(known) = known { + index.add_settings(known); + } + + for (legacy_id, mut model_settings) in legacy_models { let explicit_provider = model_settings.provider.take(); - let (provider, model_id) = if let Some(provider) = explicit_provider { - let provider = canonical_settings_provider(&provider, &settings, known) - .unwrap_or_else(|| ProviderId::new(provider)); - let model_id = canonical_settings_model(&provider, &legacy_id, &settings, known) - .unwrap_or_else(|| ModelId::new(legacy_id.clone())); - (provider, model_id) - } else { - let candidates = settings_model_candidates(&legacy_id, &settings, known); - match candidates.as_slice() { - [(provider, model)] => (provider.clone(), model.clone()), - [] => { - return Err(CatalogBuildError::LegacyModelProviderUnknown { model: legacy_id }); - } - _ => { - return Err(CatalogBuildError::LegacyModelProviderAmbiguous { - model: legacy_id, - providers: candidates - .into_iter() - .map(|(provider, _)| provider) - .collect(), - }); - } - } - }; + let (provider, model_id) = index.resolve(&legacy_id, explicit_provider.as_deref())?; if !settings.providers.contains_key(provider.as_str()) && !known.is_some_and(|known| known.providers.contains_key(provider.as_str())) @@ -1469,10 +1504,11 @@ fn normalize_catalog_settings( let provider_settings = settings.providers.entry(provider.to_string()).or_default(); if provider_settings.models.contains_key(model_id.as_str()) { - return Err(CatalogBuildError::DuplicateProviderModelDefinition { + return Err(LegacyModelError::DuplicateModel { provider, model: model_id, - }); + } + .into()); } provider_settings .models @@ -1481,154 +1517,242 @@ fn normalize_catalog_settings( Ok(settings) } -fn canonical_settings_provider( - selector: &str, - settings: &LlmCatalogSettings, - known: Option<&LlmCatalogSettings>, -) -> Option { - let providers = || { - settings - .providers - .iter() - .chain(known.into_iter().flat_map(|known| known.providers.iter())) - }; - providers() - .find(|(id, _)| id.as_str() == selector) - .or_else(|| { - providers().find(|(_, provider)| { - provider - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - }) - .map(|(id, _)| ProviderId::new(id.clone())) -} - -fn canonical_settings_model( - provider: &ProviderId, - selector: &str, - settings: &LlmCatalogSettings, - known: Option<&LlmCatalogSettings>, -) -> Option { - let models = || { - settings - .providers - .get(provider.as_str()) - .into_iter() - .chain(known.and_then(|known| known.providers.get(provider.as_str()))) - .flat_map(|provider| provider.models.iter()) - }; - models() - .find(|(id, _)| id.as_str() == selector) - .or_else(|| { - models().find(|(_, model)| { - model - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - }) - .map(|(id, _)| ModelId::new(id.clone())) -} - -fn settings_model_candidates( - selector: &str, - settings: &LlmCatalogSettings, - known: Option<&LlmCatalogSettings>, -) -> Vec<(ProviderId, ModelId)> { - let collect = |matches: &dyn Fn(&str, &ModelCatalogSettings) -> bool| { - let mut candidates = BTreeSet::<(ProviderId, ModelId)>::new(); - for (provider_id, provider) in settings - .providers - .iter() - .chain(known.into_iter().flat_map(|known| known.providers.iter())) - { - for (model_id, model) in &provider.models { - if matches(model_id, model) { - candidates.insert(( - ProviderId::new(provider_id.clone()), - ModelId::new(model_id.clone()), - )); - } +fn reject_scoped_provider_fields(settings: &LlmCatalogSettings) -> Result<(), LegacyModelError> { + for (provider, settings) in &settings.providers { + for (model, settings) in &settings.models { + if settings.provider.is_some() { + return Err(LegacyModelError::ScopedModelDeclaresProvider { + provider: ProviderId::new(provider.clone()), + model: ModelId::new(model.clone()), + }); } } - candidates.into_iter().collect::>() - }; - - let canonical = collect(&|model_id, _| model_id == selector); - if canonical.is_empty() { - collect(&|_, model| { - model - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - } else { - canonical } + Ok(()) } -/// Return every built-in offering whose canonical model ID is `selector`. -/// -/// This includes disabled providers because config compatibility -/// normalization happens before runtime availability is known. -pub fn builtin_canonical_model_offerings( - selector: &str, -) -> Result, CatalogBuildError> { - let settings = Catalog::builtin_settings()?; - Ok(settings - .providers - .iter() - .filter_map(|(provider, settings)| { - settings.models.get_key_value(selector).map(|(model, _)| { - ( - ProviderId::new(provider.clone()), - ModelId::new(model.clone()), - ) - }) - }) - .collect()) +/// Failure to resolve a legacy top-level `[models.]` row onto its +/// provider. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum LegacyModelError { + #[error("failed to inspect the built-in model catalog: {message}")] + BuiltinCatalog { message: String }, + #[error("model identifier '{identifier}' has been retired; use '{provider}/{model}' instead")] + Retired { + identifier: String, + provider: ProviderId, + model: ModelId, + }, + #[error("legacy model row '{model}' omits provider and does not match a unique known offering")] + UnknownModel { model: String }, + #[error( + "legacy model row '{model}' omits provider and matches multiple offerings: {candidates:?}" + )] + AmbiguousModel { + model: String, + candidates: Vec<(ProviderId, ModelId)>, + }, + #[error("legacy model selector '{selector}' is ambiguous on provider '{provider}': {models:?}")] + AmbiguousAlias { + provider: ProviderId, + selector: String, + models: Vec, + }, + #[error("provider-scoped model '{provider}/{model}' must not declare a provider field")] + ScopedModelDeclaresProvider { + provider: ProviderId, + model: ModelId, + }, + #[error( + "provider '{provider}' model '{model}' is defined through both provider-scoped and legacy top-level syntax" + )] + DuplicateModel { + provider: ProviderId, + model: ModelId, + }, } -/// Return every built-in offering that declares `selector` as an alias. +/// Identifier/alias view used to resolve legacy top-level `[models.]` +/// rows onto their provider before provider-scoped settings merge. /// -/// This includes disabled providers because config compatibility -/// normalization happens before runtime availability is known. -pub fn builtin_alias_model_offerings( - selector: &str, -) -> Result, CatalogBuildError> { - let settings = Catalog::builtin_settings()?; - let mut offerings = settings - .providers - .iter() - .flat_map(|(provider, settings)| { - settings - .models - .iter() - .filter(move |(_, settings)| { - settings - .aliases - .as_ref() - .is_some_and(|aliases| aliases.iter().any(|alias| alias == selector)) - }) - .map(move |(model, _)| { +/// Both the settings-layer normalization in `fabro-config` and catalog-build +/// normalization here feed this index: local entries first, lower-precedence +/// known entries (e.g. the built-in catalog) after. Canonical IDs always win +/// over aliases; alias ties resolve to the first entry added. +#[derive(Debug, Default)] +pub struct LegacyModelIndex { + providers: Vec, +} + +#[derive(Debug)] +struct LegacyProviderEntry { + id: ProviderId, + aliases: Vec, + models: Vec, +} + +#[derive(Debug)] +struct LegacyModelEntry { + id: ModelId, + aliases: Vec, +} + +impl LegacyModelIndex { + pub fn add_provider( + &mut self, + id: ProviderId, + aliases: Vec, + models: impl IntoIterator)>, + ) { + self.providers.push(LegacyProviderEntry { + id, + aliases, + models: models + .into_iter() + .map(|(id, aliases)| LegacyModelEntry { id, aliases }) + .collect(), + }); + } + + fn add_settings(&mut self, settings: &LlmCatalogSettings) { + let mut provider_ids = settings.providers.keys().collect::>(); + provider_ids.sort_unstable(); + for provider_id in provider_ids { + let provider = &settings.providers[provider_id]; + let mut model_ids = provider.models.keys().collect::>(); + model_ids.sort_unstable(); + self.add_provider( + ProviderId::new(provider_id.clone()), + provider.aliases.clone().unwrap_or_default(), + model_ids.into_iter().map(|model_id| { + let model = &provider.models[model_id]; ( - ProviderId::new(provider.clone()), - ModelId::new(model.clone()), + ModelId::new(model_id.clone()), + model.aliases.clone().unwrap_or_default(), ) - }) - }) - .collect::>(); - offerings.sort(); - offerings.dedup(); - Ok(offerings) -} + }), + ); + } + } -/// Resolve a built-in provider ID or alias without filtering disabled rows. -pub fn builtin_provider_id(selector: &str) -> Result, CatalogBuildError> { - let settings = Catalog::builtin_settings()?; - Ok(canonical_settings_provider(selector, &settings, None)) + /// Append the built-in catalog as the lowest-precedence tier. Includes + /// disabled providers because config compatibility normalization happens + /// before runtime availability is known. + pub fn with_builtin(mut self) -> Result { + let builtin = + Catalog::builtin_settings().map_err(|error| LegacyModelError::BuiltinCatalog { + message: error.to_string(), + })?; + self.add_settings(&builtin); + Ok(self) + } + + /// Resolve one legacy row to its provider-scoped address. An unknown + /// explicit provider or model selector passes through verbatim; rows + /// without an explicit provider must match exactly one known offering. + pub fn resolve( + &self, + legacy_id: &str, + explicit_provider: Option<&str>, + ) -> Result<(ProviderId, ModelId), LegacyModelError> { + if let Some((provider, model)) = retired_model_replacement(legacy_id) { + return Err(LegacyModelError::Retired { + identifier: legacy_id.to_string(), + provider, + model, + }); + } + if let Some(explicit) = explicit_provider { + let provider = self + .canonical_provider(explicit) + .unwrap_or_else(|| ProviderId::new(explicit)); + let model = self + .canonical_model_on(&provider, legacy_id)? + .unwrap_or_else(|| ModelId::new(legacy_id)); + return Ok((provider, model)); + } + let candidates = self.candidates(legacy_id); + match candidates.as_slice() { + [(provider, model)] => Ok((provider.clone(), model.clone())), + [] => Err(LegacyModelError::UnknownModel { + model: legacy_id.to_string(), + }), + _ => Err(LegacyModelError::AmbiguousModel { + model: legacy_id.to_string(), + candidates, + }), + } + } + + fn canonical_provider(&self, selector: &str) -> Option { + self.providers + .iter() + .find(|provider| provider.id.as_str() == selector) + .or_else(|| { + self.providers + .iter() + .find(|provider| provider.aliases.iter().any(|alias| alias == selector)) + }) + .map(|provider| provider.id.clone()) + } + + fn canonical_model_on( + &self, + provider: &ProviderId, + selector: &str, + ) -> Result, LegacyModelError> { + let models = || { + self.providers + .iter() + .filter(|entry| entry.id == *provider) + .flat_map(|entry| entry.models.iter()) + }; + if models().any(|model| model.id.as_str() == selector) { + return Ok(Some(ModelId::new(selector))); + } + let matches = models() + .filter(|model| model.aliases.iter().any(|alias| alias == selector)) + .map(|model| model.id.clone()) + .collect::>(); + match matches.len() { + 0 => Ok(None), + 1 => Ok(matches.into_iter().next()), + _ => Err(LegacyModelError::AmbiguousAlias { + provider: provider.clone(), + selector: selector.to_string(), + models: matches.into_iter().collect(), + }), + } + } + + fn candidates(&self, selector: &str) -> Vec<(ProviderId, ModelId)> { + let canonical = self + .providers + .iter() + .filter(|entry| { + entry + .models + .iter() + .any(|model| model.id.as_str() == selector) + }) + .map(|entry| (entry.id.clone(), ModelId::new(selector))) + .collect::>(); + if !canonical.is_empty() { + return canonical.into_iter().collect(); + } + self.providers + .iter() + .flat_map(|entry| { + entry + .models + .iter() + .filter(|model| model.aliases.iter().any(|alias| alias == selector)) + .map(|model| (entry.id.clone(), model.id.clone())) + }) + .collect::>() + .into_iter() + .collect() + } } /// Built-in catalog keys retired when provider API identifiers stopped being @@ -1718,10 +1842,6 @@ pub fn retired_model_replacement(identifier: &str) -> Option<(ProviderId, ModelI .map(|(_, provider, model)| (ProviderId::new(*provider), ModelId::new(*model))) } -fn retired_model_address(identifier: &str) -> Option<(ProviderId, ModelId)> { - retired_model_replacement(identifier) -} - fn merge_catalog_settings( higher: LlmCatalogSettings, mut fallback: LlmCatalogSettings, @@ -3676,8 +3796,10 @@ provider = "test" assert!(matches!( error, - CatalogBuildError::ScopedModelDeclaresProvider { provider, model } - if provider == ProviderId::new("test") && model == "one" + CatalogBuildError::LegacyModel(LegacyModelError::ScopedModelDeclaresProvider { + provider, + model, + }) if provider == ProviderId::new("test") && model == "one" )); } diff --git a/lib/crates/fabro-model/src/ids.rs b/lib/crates/fabro-model/src/ids.rs index 3429ae094..02d6db0d7 100644 --- a/lib/crates/fabro-model/src/ids.rs +++ b/lib/crates/fabro-model/src/ids.rs @@ -150,14 +150,6 @@ impl From for ModelId { } } -impl FromStr for ModelId { - type Err = std::convert::Infallible; - - fn from_str(s: &str) -> Result { - Ok(Self::from(s)) - } -} - impl AsRef for ModelId { fn as_ref(&self) -> &str { &self.0 diff --git a/lib/crates/fabro-model/src/lib.rs b/lib/crates/fabro-model/src/lib.rs index 337c00fdd..5f3511096 100644 --- a/lib/crates/fabro-model/src/lib.rs +++ b/lib/crates/fabro-model/src/lib.rs @@ -19,7 +19,7 @@ pub use billing::{ }; pub use catalog::{ ApiKeyHeaderPolicy, BillingPolicy, Catalog, CredentialRef, CredentialRefParseError, - FallbackTarget, ModelSelectionError, ProviderAuthConfig, + FallbackTarget, ModelSelectionError, ProviderAuthConfig, SelectedModel, }; pub use codec::CodecKind; pub use ids::{ModelId, ProviderId}; diff --git a/lib/crates/fabro-server/src/server/handler/completions.rs b/lib/crates/fabro-server/src/server/handler/completions.rs index 3d36f9036..b53e56d31 100644 --- a/lib/crates/fabro-server/src/server/handler/completions.rs +++ b/lib/crates/fabro-server/src/server/handler/completions.rs @@ -53,7 +53,7 @@ async fn create_completion( let (model_id, selected_provider) = match resolve_request_model( catalog.as_ref(), &client.provider_ids(), - req.model, + req.model.as_deref(), req.provider, ) { Ok(selection) => selection, @@ -114,20 +114,8 @@ async fn create_completion( metadata: None, provider_options: req.provider_options, }; - let request = match client.resolve_request(&request) { - Ok(request) => request, - Err(error) => return ApiError::bad_request(error.to_string()).into_response(), - }; - let selected_provider = ProviderId::new( - request - .provider - .as_deref() - .expect("resolved LLM request always has a provider"), - ); - let selected_model = request.model.clone(); - info!( - model = %selected_model, + model = %model_id, provider = %selected_provider, "Completion request received" ); @@ -177,7 +165,7 @@ async fn create_completion( let stop_reason = finish_reason_to_api_stop_reason(&response.finish_reason); Json(CompletionResponse { id: msg_id, - model: selected_model, + model: model_id, provider: selected_provider, message: response.message, stop_reason, @@ -224,44 +212,11 @@ async fn create_completion( pub(super) fn resolve_request_model( catalog: &Catalog, eligible: &HashSet, - requested_model: Option, + requested_model: Option<&str>, explicit_provider: Option, ) -> Result<(String, ProviderId), ModelSelectionError> { let explicit_provider = explicit_provider.map(ProviderId::new); - if let Some(model) = requested_model { - return match catalog.select(&model, explicit_provider.as_ref(), eligible) { - Ok(offering) => Ok((offering.id.to_string(), offering.provider.clone())), - Err(ModelSelectionError::UnknownSelectorOnProvider { provider, .. }) => { - Ok((model, provider)) - } - Err(ModelSelectionError::UnknownSelector { .. }) => { - let default = catalog.select_default(eligible)?; - Ok((model, default.provider.clone())) - } - Err(error) => Err(error), - }; - } - - let eligible = if let Some(requested_provider) = explicit_provider { - let provider = catalog.provider(&requested_provider).ok_or_else(|| { - ModelSelectionError::UnknownProvider { - provider: requested_provider.clone(), - } - })?; - let provider_is_ready = eligible.iter().any(|eligible_provider| { - catalog - .provider(eligible_provider) - .is_some_and(|eligible_provider| eligible_provider.id == provider.id) - }); - if !provider_is_ready { - return Err(ModelSelectionError::ProviderUnavailable { - provider: provider.id.clone(), - }); - } - HashSet::from([provider.id.clone()]) - } else { - eligible.clone() - }; - let default = catalog.select_default(&eligible)?; - Ok((default.id.to_string(), default.provider.clone())) + let selected = + catalog.resolve_selection(requested_model, explicit_provider.as_ref(), eligible)?; + Ok((selected.model, selected.provider)) } diff --git a/lib/crates/fabro-server/src/server/handler/models.rs b/lib/crates/fabro-server/src/server/handler/models.rs index 9b639af99..59b6b8623 100644 --- a/lib/crates/fabro-server/src/server/handler/models.rs +++ b/lib/crates/fabro-server/src/server/handler/models.rs @@ -272,7 +272,8 @@ fn model_selection_response(error: &ModelSelectionError) -> Response { } ModelSelectionError::RetiredModelIdentifier { .. } | ModelSelectionError::ProviderUnavailable { .. } - | ModelSelectionError::NoEligibleOffering { .. } => { + | ModelSelectionError::NoEligibleOffering { .. } + | ModelSelectionError::NoDefaultModel { .. } => { ApiError::bad_request(error.to_string()).into_response() } } diff --git a/lib/crates/fabro-server/src/server/handler/playground.rs b/lib/crates/fabro-server/src/server/handler/playground.rs index 046cf2ef3..d404c2b58 100644 --- a/lib/crates/fabro-server/src/server/handler/playground.rs +++ b/lib/crates/fabro-server/src/server/handler/playground.rs @@ -130,7 +130,7 @@ async fn create_playground_chat( let (model_id, selected_provider) = match super::completions::resolve_request_model( catalog.as_ref(), &client.provider_ids(), - req.model, + req.model.as_deref(), req.provider.map(|provider| provider.to_string()), ) { Ok(selection) => selection, @@ -146,7 +146,7 @@ async fn create_playground_chat( let request = LlmRequest { model: model_id, messages, - provider: Some(selected_provider.to_string()), + provider: Some(selected_provider.into_inner()), tools: Some(playground_tools()), tool_choice: Some(ToolChoice::Auto), response_format: None, @@ -159,10 +159,6 @@ async fn create_playground_chat( metadata: None, provider_options: None, }; - let request = match client.resolve_request(&request) { - Ok(request) => request, - Err(error) => return ApiError::bad_request(error.to_string()).into_response(), - }; info!( model = %request.model, provider = request.provider.as_deref().unwrap_or(""), diff --git a/lib/crates/fabro-server/src/server/handler/sessions.rs b/lib/crates/fabro-server/src/server/handler/sessions.rs index eb72bd386..936b8e582 100644 --- a/lib/crates/fabro-server/src/server/handler/sessions.rs +++ b/lib/crates/fabro-server/src/server/handler/sessions.rs @@ -805,37 +805,22 @@ fn selected_session_model( .provider_ids() .into_iter() .collect::>(); - let (provider_id, model) = if let Some(model_id) = session.record.model.as_deref() { - match catalog.select(model_id, session.record.provider.as_ref(), &eligible) { - Ok(selected) => (selected.provider.clone(), selected.id.to_string()), - Err(ModelSelectionError::UnknownSelectorOnProvider { provider, .. }) => { - (provider, model_id.to_string()) - } - Err(ModelSelectionError::UnknownSelector { .. }) - if session.record.provider.is_none() => + let record = &session.record; + let selected = catalog + .resolve_selection(record.model.as_deref(), record.provider.as_ref(), &eligible) + .map_err(|error| { + // A missing default with no provider pin means no LLM is + // configured at all; every other failure is about the requested + // model/provider. + if record.provider.is_none() + && matches!(error, ModelSelectionError::NoDefaultModel { .. }) { - let selected = catalog - .select_default(&eligible) - .map_err(|error| AskFabroBuildError::LlmUnconfigured(error.to_string()))?; - (selected.provider.clone(), model_id.to_string()) + AskFabroBuildError::LlmUnconfigured(error.to_string()) + } else { + AskFabroBuildError::ModelUnavailable(error.to_string()) } - Err(error) => { - return Err(AskFabroBuildError::ModelUnavailable(error.to_string())); - } - } - } else { - let selected = if let Some(provider) = session.record.provider.as_ref() { - let provider_only = std::collections::HashSet::from([provider.clone()]); - catalog - .select_default(&provider_only) - .map_err(|error| AskFabroBuildError::ModelUnavailable(error.to_string()))? - } else { - catalog - .select_default(&eligible) - .map_err(|error| AskFabroBuildError::LlmUnconfigured(error.to_string()))? - }; - (selected.provider.clone(), selected.id.to_string()) - }; + })?; + let (provider_id, model) = (selected.provider, selected.model); let profile_kind = catalog .effective_agent_profile(&provider_id, Some(&model)) .ok_or_else(|| { @@ -865,27 +850,10 @@ fn canonical_session_model( }) .transpose()?; let Some(requested) = requested else { - let eligible = if let Some(provider) = explicit_provider.as_ref() { - let provider_is_ready = eligible.iter().any(|eligible_provider| { - catalog - .provider(eligible_provider) - .is_some_and(|eligible_provider| eligible_provider.id == *provider) - }); - if !provider_is_ready { - return Err(session_selection_error( - &ModelSelectionError::ProviderUnavailable { - provider: provider.clone(), - }, - )); - } - std::collections::HashSet::from([provider.clone()]) - } else { - eligible.clone() - }; - let model = catalog - .select_default(&eligible) + let selected = catalog + .resolve_selection(None, explicit_provider.as_ref(), eligible) .map_err(|error| session_selection_error(&error))?; - return Ok((model.provider.clone(), model.id.to_string())); + return Ok((selected.provider, selected.model)); }; let requested = requested.trim(); if requested.is_empty() { @@ -942,19 +910,10 @@ fn canonical_session_model( } }; let provider = qualified_provider.as_ref().or(explicit_provider.as_ref()); - match catalog.select(&model, provider, eligible) { - Ok(selected) => Ok((selected.provider.clone(), selected.id.to_string())), - Err(ModelSelectionError::UnknownSelectorOnProvider { provider, .. }) => { - Ok((provider, model)) - } - Err(ModelSelectionError::UnknownSelector { .. }) if provider.is_none() => { - let selected = catalog - .select_default(eligible) - .map_err(|error| session_selection_error(&error))?; - Ok((selected.provider.clone(), model)) - } - Err(error) => Err(session_selection_error(&error)), - } + let selected = catalog + .resolve_selection(Some(&model), provider, eligible) + .map_err(|error| session_selection_error(&error))?; + Ok((selected.provider, selected.model)) } fn session_selection_error(error: &ModelSelectionError) -> ApiError { diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index ca29053a9..74095bbf5 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -3414,7 +3414,7 @@ async fn create_run_without_ready_llm_provider_rejects_implicit_model_selection( assert!( body["errors"][0]["detail"] .as_str() - .is_some_and(|detail| detail.contains("no offering on an eligible provider")), + .is_some_and(|detail| detail.contains("no default model is available")), "unexpected response: {body}" ); assert!(state.runs.lock().expect("runs lock poisoned").is_empty()); diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index 5972d6cc4..b1e33f4c7 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -35,7 +35,6 @@ use crate::event::{ Emitter, Event, EventBody, RunEventLogger, RunEventSink, RunNoticeLevel, append_event_to_sink, }; use crate::handler::HandlerRegistry; -use crate::handler::llm::routing; use crate::outcome::{Outcome, StageOutcome}; use crate::pipeline::{ self, FinalizeOptions, Finalized, InitOptions, LlmSpec, Persisted, PullRequestOptions, @@ -618,22 +617,12 @@ fn resolve_start_llm( settings: &ResolvedRunSettings, ) -> Result { let eligible = configured.iter().cloned().collect::>(); - let (model, provider) = resolve_run_model( + let (model, provider_id) = resolve_run_model( catalog, &eligible, settings.model.name.as_deref(), settings.model.provider.as_deref(), )?; - let provider_id = ProviderId::new( - provider.expect("catalog-backed run model resolution always selects a provider"), - ); - let provider_context = routing::resolve_provider_context( - catalog, - &provider_id, - &model, - Some(provider_id.as_str()), - )?; - let provider_id = provider_context.provider_id; let fallback_chain = resolve_fallback_chain(catalog, &provider_id, &model, &settings.model, &eligible)?; diff --git a/lib/crates/fabro-workflow/src/run_materialization.rs b/lib/crates/fabro-workflow/src/run_materialization.rs index 14ffa735a..7e82964ac 100644 --- a/lib/crates/fabro-workflow/src/run_materialization.rs +++ b/lib/crates/fabro-workflow/src/run_materialization.rs @@ -34,7 +34,7 @@ pub fn materialize_run( resolve_run_model(catalog, &eligible, model.as_deref(), provider.as_deref())?; settings.run.model.name = Some(resolved_model); - settings.run.model.provider = resolved_provider; + settings.run.model.provider = Some(resolved_provider.into_inner()); let goal = graph.goal().to_string(); settings.run.goal = if goal.is_empty() { @@ -60,55 +60,10 @@ pub(crate) fn resolve_run_model( eligible: &HashSet, model: Option<&str>, provider: Option<&str>, -) -> Result<(String, Option), ModelSelectionError> { - if let Some(provider) = provider.filter(|provider| !provider.is_empty()) { - let requested = ProviderId::new(provider); - let provider = - catalog - .provider(&requested) - .ok_or_else(|| ModelSelectionError::UnknownProvider { - provider: requested.clone(), - })?; - let canonical_provider = provider.id.clone(); - let canonical_eligible = eligible.iter().any(|eligible| { - catalog - .provider(eligible) - .is_some_and(|provider| provider.id == canonical_provider) - }); - if !canonical_eligible { - return Err(ModelSelectionError::ProviderUnavailable { - provider: canonical_provider, - }); - } - if let Some(model) = model { - return match catalog.resolve_on_provider(&provider.id, model) { - Ok(offering) => Ok((offering.id.to_string(), Some(offering.provider.to_string()))), - Err(ModelSelectionError::UnknownSelectorOnProvider { .. }) => { - Ok((model.to_string(), Some(provider.id.to_string()))) - } - Err(error) => Err(error), - }; - } - let offering = catalog.default_for_provider(&provider.id).ok_or_else(|| { - ModelSelectionError::UnknownSelectorOnProvider { - selector: "".to_string(), - provider: provider.id.clone(), - } - })?; - return Ok((offering.id.to_string(), Some(offering.provider.to_string()))); - } - - if let Some(model) = model { - return match catalog.select(model, None, eligible) { - Ok(offering) => Ok((offering.id.to_string(), Some(offering.provider.to_string()))), - Err(ModelSelectionError::UnknownSelector { .. }) => { - let default = catalog.select_default(eligible)?; - Ok((model.to_string(), Some(default.provider.to_string()))) - } - Err(error) => Err(error), - }; - } - - let default = catalog.select_default(eligible)?; - Ok((default.id.to_string(), Some(default.provider.to_string()))) +) -> Result<(String, ProviderId), ModelSelectionError> { + let provider = provider + .filter(|provider| !provider.is_empty()) + .map(ProviderId::new); + let selected = catalog.resolve_selection(model, provider.as_ref(), eligible)?; + Ok((selected.model, selected.provider)) } diff --git a/lib/crates/fabro-workflow/src/transforms/model_resolution.rs b/lib/crates/fabro-workflow/src/transforms/model_resolution.rs index fc130bfb1..7eecbcfd1 100644 --- a/lib/crates/fabro-workflow/src/transforms/model_resolution.rs +++ b/lib/crates/fabro-workflow/src/transforms/model_resolution.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use std::sync::Arc; use fabro_graphviz::graph::{AttrValue, Graph}; -use fabro_model::{Catalog, ModelSelectionError, ProviderId}; +use fabro_model::{Catalog, ProviderId}; use super::Transform; use crate::error::Error; @@ -46,24 +46,12 @@ impl ModelResolutionTransform { model: &str, explicit_provider: Option<&ProviderId>, ) -> Result<(String, ProviderId), Error> { - match self - .catalog - .select(model, explicit_provider, &self.eligible_providers) - { - Ok(info) => Ok((info.id.to_string(), info.provider.clone())), - Err(ModelSelectionError::UnknownSelectorOnProvider { provider, .. }) => { - Ok((model.to_string(), provider)) - } - Err(ModelSelectionError::UnknownSelector { .. }) => { - let provider = self - .catalog - .select_default(&self.eligible_providers)? - .provider - .clone(); - Ok((model.to_string(), provider)) - } - Err(error) => Err(error.into()), - } + let selected = self.catalog.resolve_selection( + Some(model), + explicit_provider, + &self.eligible_providers, + )?; + Ok((selected.model, selected.provider)) } }