From 33b94d850e960f2ed3ef395cc5bd2badaddeba9c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 28 Jul 2026 11:48:20 -0400 Subject: [PATCH] Support provider-qualified fallback selectors --- docs/public/api-reference/fabro-api.yaml | 14 +- docs/public/core-concepts/models.mdx | 2 +- docs/public/execution/failures.mdx | 4 +- docs/public/execution/run-configuration.mdx | 14 +- docs/public/reference/user-configuration.mdx | 4 +- .../src/server/handler/sessions.rs | 37 +++-- .../fabro-workflow/src/operations/start.rs | 96 ++++++++++- lib/foundation/fabro-config/src/layers/run.rs | 7 +- .../fabro-config/src/tests/combine.rs | 17 +- .../src/commands/docs_options_reference.rs | 2 +- lib/foundation/fabro-model/src/catalog.rs | 151 ++++++++++++++++-- .../fabro-types/src/settings/model_ref.rs | 124 ++++++++++---- .../src/models/create-run-session-request.ts | 2 +- 13 files changed, 397 insertions(+), 77 deletions(-) diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index b028d89c0..9ded5ee70 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -7941,8 +7941,11 @@ components: model: type: string description: | - Catalog model ID or alias. The server selects among ready - providers and stores the canonical model ID. + Catalog model ID or alias, optionally qualified as + `provider:selector`. A provider-qualified selector may be a + canonical model ID, alias, or provider API ID. Legacy + `provider/model` references remain accepted. The server stores the + canonical model ID. provider: $ref: "#/components/schemas/ProviderId" description: Optional provider pin. Provider-qualified model references remain accepted for compatibility. @@ -14119,6 +14122,13 @@ components: ModelRef: type: string + description: | + A fallback model reference. Bare values name a provider, canonical + model ID, or alias. Provider-qualified values use + `provider:selector`; the selector may be a canonical model ID, alias, + or provider API ID and may contain `/` or additional colons. Legacy + `provider/model` references remain accepted. + example: openrouter:moonshotai/kimi-k3 RunModelSettings: type: object diff --git a/docs/public/core-concepts/models.mdx b/docs/public/core-concepts/models.mdx index 1a6f094b3..2eb14340a 100644 --- a/docs/public/core-concepts/models.mdx +++ b/docs/public/core-concepts/models.mdx @@ -277,7 +277,7 @@ Then launch with: fabro run run.toml ``` -The `fallbacks` array is optional. Each entry may be a bare provider token (like `"gemini"`), a bare model alias (like `"gpt-5.4"`), or a qualified `"provider/model"` reference. Fabro tries them in order when the primary provider is unavailable. In this field, qualified references keep their established provider-pin meaning: `"openai/gpt-5.6-sol"` selects the direct OpenAI offering. +The `fallbacks` array is optional. Each entry may be a bare provider token (like `"gemini"`), a bare model ID or alias (like `"gpt-terra"`), or a qualified `"provider:selector"` reference. A qualified selector may be the provider's canonical model ID, alias, or API ID, including API IDs with slashes such as `"openrouter:moonshotai/kimi-k3"`. Fabro tries entries in order when the primary provider is unavailable, and qualified references remain provider pins. Legacy `provider/model` references remain accepted for compatibility. The precedence order is: node-level stylesheet > run config TOML > CLI flags > server defaults. More specific settings always win. diff --git a/docs/public/execution/failures.mdx b/docs/public/execution/failures.mdx index a74eefa9f..bddd0dc7b 100644 --- a/docs/public/execution/failures.mdx +++ b/docs/public/execution/failures.mdx @@ -124,10 +124,10 @@ fallbacks = ["gemini", "openai"] When Anthropic fails, Fabro tries Gemini first, then OpenAI. Fallback resolution is provider-aware: - A bare provider token such as `"gemini"` selects that provider's closest compatible model. -- A qualified selector such as `"openrouter/gpt-56-sol"` resolves only within that provider. +- A qualified selector such as `"openrouter:gpt-56-sol"` resolves only within that provider. The selector may be a canonical model ID, alias, or provider API ID such as `"openrouter:moonshotai/kimi-k3"`. - A bare model slug or alias considers ready providers and uses provider priority. -Qualified fallback references always remain provider pins, including strings that were historical built-in API IDs. For example, `"openai/gpt-5.6-sol"` pins the direct OpenAI offering. +Qualified fallback references always remain provider pins. For example, `"openai:gpt-5.6-sol"` pins the direct OpenAI offering. Legacy `provider/model` fallback references remain accepted for compatibility. The primary provider and model were already resolved and persisted when the run was created; resuming does not re-run primary selection. Fallbacks are only considered after an eligible runtime failure. diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 300f0a802..1f1a2dd8e 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -140,10 +140,22 @@ name = "claude-sonnet-4-5" |---|---| | `name` | Canonical model slug or alias (e.g. `claude-sonnet-4-5`, `opus`, `gemini-pro`). See [Models](/core-concepts/models). | | `provider` | Optional provider pin. When omitted, Fabro selects among ready offerings by provider priority. When present, an unavailable provider is an error rather than permission to switch. | -| `fallbacks` | Ordered list of model references to try when the primary is unavailable. Entries can be bare provider tokens (`"openai"`), bare model aliases, or qualified `"provider/model"` references. | +| `fallbacks` | Ordered list of model references to try when the primary is unavailable. Entries can be bare provider tokens (`"openai"`), bare model IDs or aliases, or qualified `"provider:selector"` references. | Provider values are catalog provider ID strings. Built-in IDs like `anthropic` and `openai` work, and settings-defined IDs like `proxy` work after they are added under `[llm.providers.]`. +For a qualified fallback, the selector may be that provider's canonical model ID, alias, or API ID. Fabro splits on the first `:`, so provider API IDs may contain `/` or additional colons: + +```toml title="run.toml" +[run.model] +fallbacks = [ + "openrouter:kimi-k3", + "gpt-terra", +] +``` + +The first entry could equivalently be written as `"openrouter:moonshotai/kimi-k3"` using OpenRouter's API ID; both forms resolve to its canonical `kimi-k3` offering. The unqualified `gpt-terra` alias uses normal ready-provider priority selection. Legacy `provider/model` fallback references remain accepted but are normalized to `provider:model`. + At run creation, Fabro resolves the primary selector and every node selector against the ready-provider snapshot. It persists the selected canonical model slug and provider, so resuming the run does not choose a different provider just because credentials or priorities changed. The configured fallback chain remains available for failures that occur while the materialized run is executing. Historical built-in provider API IDs are accepted for compatibility and normalize before this selection. For example, `name = "openai/gpt-5.6-sol"` is treated as the canonical `gpt-5.6-sol` selector; omit `provider` to use readiness and priority, or set `provider` separately to pin an offering. diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index 8ae378442..bf9ca0bd6 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -381,12 +381,12 @@ permissions = "read-write" [run.model] provider = "anthropic" name = "claude-sonnet-4-5" -fallbacks = ["openai", "gpt-5.4"] +fallbacks = ["openrouter:kimi-k3", "gpt-terra"] ``` | Key | Type / values | Default | Description | |---|---|---|---| -| `fallbacks` | array | [] | Ordered list of fallback model references. Supports `...` splice marker
at layering time — see [`super::splice_array`]. | +| `fallbacks` | array | [] | Ordered fallback references: bare providers, bare model IDs or aliases,
or provider-qualified `provider:selector` values. A qualified selector
may be a model ID, alias, or provider API ID. Legacy `provider/model`
values remain accepted. Supports the `...` splice marker at layering
time — see [`super::splice_array`]. | | `name` | string | None | Model name for workflow runs. | | `provider` | string | None | Provider name for workflow model selection. | diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index c25d7ca27..2cc9c8168 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -858,8 +858,8 @@ fn canonical_session_model( let model_ref = requested .parse::() .map_err(|err| ApiError::bad_request(err.to_string()))?; - let (qualified_provider, model) = match model_ref { - SettingsModelRef::Qualified { provider, model } => { + let (qualified_provider, selector) = match model_ref { + SettingsModelRef::Qualified { provider, selector } => { let requested_provider = ProviderId::new(provider); let provider = catalog .provider(&requested_provider) @@ -877,28 +877,30 @@ fn canonical_session_model( ))); } } - (Some(provider), model) + (Some(provider), selector) } - SettingsModelRef::Bare(model) => { - if explicit_provider.is_none() && catalog.provider(&ProviderId::new(&model)).is_some() { - let detail = if catalog.is_model_selector(&model) { + SettingsModelRef::Bare(selector) => { + if explicit_provider.is_none() + && catalog.provider(&ProviderId::new(&selector)).is_some() + { + let detail = if catalog.is_model_selector(&selector) { format!( - "Session model reference '{model}' is ambiguous between a provider and a \ - model selector; supply `provider` or use `provider/model`." + "Session model reference '{selector}' is ambiguous between a provider and \ + a model selector; supply `provider` or use `provider:model`." ) } else { format!( - "Session model reference '{model}' names a provider; include a model ID." + "Session model reference '{selector}' names a provider; include a model ID." ) }; return Err(ApiError::bad_request(detail)); } - (None, model) + (None, selector) } }; let provider = qualified_provider.as_ref().or(explicit_provider.as_ref()); let selected = catalog - .resolve_selection(Some(&model), provider, eligible) + .resolve_selection(Some(&selector), provider, eligible) .map_err(|error| session_selection_error(&error))?; Ok((selected.provider, selected.model)) } @@ -1599,7 +1601,12 @@ reasoning = false (openrouter.clone(), "gpt-5.6-sol".to_string()) ); assert_eq!( - canonical_session_model(&catalog, &both, Some("openrouter/gpt-56-sol"), None,).unwrap(), + canonical_session_model(&catalog, &both, Some("openrouter:gpt-56-sol"), None,).unwrap(), + (openrouter.clone(), "gpt-5.6-sol".to_string()) + ); + assert_eq!( + canonical_session_model(&catalog, &both, Some("openrouter:openai/gpt-5.6-sol"), None,) + .unwrap(), (openrouter, "gpt-5.6-sol".to_string()) ); } @@ -1670,7 +1677,7 @@ reasoning = false } #[test] - fn canonical_session_model_still_treats_non_legacy_qualified_model_as_a_pin() { + fn canonical_session_model_treats_colon_qualified_model_as_a_pin() { let catalog = portable_session_catalog(); let openrouter = ProviderId::new("openrouter"); @@ -1678,7 +1685,7 @@ reasoning = false canonical_session_model( &catalog, &catalog.all_provider_ids(), - Some("openrouter/gpt-56-sol"), + Some("openrouter:gpt-56-sol"), None, ) .unwrap(), @@ -1692,7 +1699,7 @@ reasoning = false let error = canonical_session_model( &catalog, &catalog.all_provider_ids(), - Some("openrouter/gpt-56-sol"), + Some("openrouter:gpt-56-sol"), Some(&ProviderId::openai()), ) .unwrap_err(); diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 3b560c7cb..576850624 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -670,14 +670,14 @@ fn resolve_fallback_chain( } ResolvedModelRef::Model { provider: fallback_provider, - model, + selector, } => { if let Some(provider) = fallback_provider { let provider = canonical_provider_id(catalog, &provider); if !eligible.contains(&provider) { return Err(ModelSelectionError::ProviderUnavailable { provider }.into()); } - match catalog.resolve_on_provider(&provider, &model) { + match catalog.resolve_on_provider(&provider, &selector) { Ok(info) => chain.push(FallbackTarget { provider: info.provider.to_string(), model: info.id.to_string(), @@ -685,13 +685,13 @@ fn resolve_fallback_chain( Err(ModelSelectionError::UnknownSelectorOnProvider { .. }) => { chain.push(FallbackTarget { provider: provider.to_string(), - model, + model: selector, }); } Err(error) => return Err(error.into()), } } else { - match catalog.select(&model, None, eligible) { + match catalog.select(&selector, None, eligible) { Ok(info) => chain.push(FallbackTarget { provider: info.provider.to_string(), model: info.id.to_string(), @@ -699,7 +699,7 @@ fn resolve_fallback_chain( Err(ModelSelectionError::UnknownSelector { .. }) => { chain.push(FallbackTarget { provider: provider.to_string(), - model, + model: selector, }); } Err(error) => return Err(error.into()), @@ -1337,7 +1337,7 @@ reasoning = false fn resolve_fallback_chain_resolves_explicit_model_fallbacks() { let catalog = test_catalog(); let settings = ResolvedRunModelSettings { - fallbacks: vec!["openai/gpt-5.4-mini".parse::().unwrap()], + fallbacks: vec!["openai:gpt-5.4-mini".parse::().unwrap()], ..ResolvedRunModelSettings::default() }; @@ -1383,7 +1383,7 @@ reasoning = false fn resolve_fallback_chain_resolves_provider_qualified_shared_alias() { let catalog = portable_model_catalog(); let settings = ResolvedRunModelSettings { - fallbacks: vec!["openrouter/gpt-56-sol".parse::().unwrap()], + fallbacks: vec!["openrouter:gpt-56-sol".parse::().unwrap()], ..ResolvedRunModelSettings::default() }; @@ -1402,6 +1402,88 @@ reasoning = false }]); } + #[test] + fn resolve_fallback_chain_supports_openrouter_kimi_then_portable_terra() { + let overrides: fabro_model::catalog::LlmCatalogSettings = toml::from_str( + r" +[providers.openrouter] +enabled = true +", + ) + .unwrap(); + let catalog = Catalog::from_builtin_with_overrides(&overrides).unwrap(); + let settings = ResolvedRunModelSettings { + fallbacks: vec![ + "openrouter:kimi-k3".parse::().unwrap(), + "gpt-terra".parse::().unwrap(), + ], + ..ResolvedRunModelSettings::default() + }; + + let chain = resolve_fallback_chain( + &catalog, + &ProviderId::new("kimi"), + "kimi-k3", + &settings, + &HashSet::from([ + ProviderId::new("kimi"), + ProviderId::new("openrouter"), + ProviderId::openai(), + ]), + ) + .unwrap(); + + assert_eq!(chain, vec![ + FallbackTarget { + provider: "openrouter".to_string(), + model: "kimi-k3".to_string(), + }, + FallbackTarget { + provider: "openai".to_string(), + model: "gpt-5.6-terra".to_string(), + }, + ]); + } + + #[test] + fn resolve_fallback_chain_canonicalizes_provider_api_id() { + let overrides: fabro_model::catalog::LlmCatalogSettings = toml::from_str( + r" +[providers.openrouter] +enabled = true +", + ) + .unwrap(); + let catalog = Catalog::from_builtin_with_overrides(&overrides).unwrap(); + let settings = ResolvedRunModelSettings { + fallbacks: vec![ + "openrouter:moonshotai/kimi-k3".parse::().unwrap(), + "gpt-terra".parse::().unwrap(), + ], + ..ResolvedRunModelSettings::default() + }; + + let chain = resolve_fallback_chain( + &catalog, + &ProviderId::new("kimi"), + "kimi-k3", + &settings, + &HashSet::from([ProviderId::new("kimi"), ProviderId::new("openrouter")]), + ) + .unwrap(); + + assert_eq!(chain, vec![ + FallbackTarget { + provider: "openrouter".to_string(), + model: "kimi-k3".to_string(), + }, + FallbackTarget { + provider: "openrouter".to_string(), + model: "gpt-5.6-terra".to_string(), + }, + ]); + } + #[test] fn resolve_fallback_chain_keeps_qualified_legacy_references_as_provider_pins() { let catalog = test_catalog(); diff --git a/lib/foundation/fabro-config/src/layers/run.rs b/lib/foundation/fabro-config/src/layers/run.rs index 655b79640..3f07e0bc2 100644 --- a/lib/foundation/fabro-config/src/layers/run.rs +++ b/lib/foundation/fabro-config/src/layers/run.rs @@ -148,8 +148,11 @@ pub struct RunModelLayer { #[serde(default, skip_serializing_if = "Option::is_none")] #[option(value_type = "string")] pub name: Option, - /// Ordered list of fallback model references. Supports `...` splice marker - /// at layering time — see [`super::splice_array`]. + /// Ordered fallback references: bare providers, bare model IDs or aliases, + /// or provider-qualified `provider:selector` values. A qualified selector + /// may be a model ID, alias, or provider API ID. Legacy `provider/model` + /// values remain accepted. Supports the `...` splice marker at layering + /// time — see [`super::splice_array`]. #[serde(default, skip_serializing_if = "Vec::is_empty")] #[option(default = "[]", value_type = "array")] pub fallbacks: Vec, diff --git a/lib/foundation/fabro-config/src/tests/combine.rs b/lib/foundation/fabro-config/src/tests/combine.rs index ff95a8aec..a66d2634e 100644 --- a/lib/foundation/fabro-config/src/tests/combine.rs +++ b/lib/foundation/fabro-config/src/tests/combine.rs @@ -7,7 +7,7 @@ use fabro_types::settings::InterpString; use fabro_types::settings::cli::{OutputFormat, OutputVerbosity}; use fabro_types::settings::server::LogDestination; -use crate::{Combine, SettingsLayer, StringOrSplice}; +use crate::{Combine, ModelRefOrSplice, SettingsLayer, StringOrSplice}; fn parse(input: &str) -> SettingsLayer { input @@ -101,7 +101,7 @@ fn run_model_fallbacks_splice_inserts_inherited() { let lower = parse( r#" [run.model] -fallbacks = ["openai", "gpt-5.4"] +fallbacks = ["openrouter:moonshotai/kimi-k3", "gpt-terra"] "#, ); let higher = parse( @@ -112,7 +112,18 @@ fallbacks = ["anthropic", "..."] ); let merged = higher.combine(lower); let fallbacks = merged.run.unwrap().model.unwrap().fallbacks; - assert_eq!(fallbacks.len(), 3); + let rendered = fallbacks + .iter() + .map(|entry| match entry { + ModelRefOrSplice::ModelRef(model_ref) => model_ref.to_string(), + ModelRefOrSplice::Splice => "...".to_string(), + }) + .collect::>(); + assert_eq!(rendered, vec![ + "anthropic", + "openrouter:moonshotai/kimi-k3", + "gpt-terra", + ]); } #[test] diff --git a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs index faf2ec963..086a76d08 100644 --- a/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs +++ b/lib/foundation/fabro-dev/src/commands/docs_options_reference.rs @@ -109,7 +109,7 @@ permissions = "read-write""#, r#"[run.model] provider = "anthropic" name = "claude-sonnet-4-5" -fallbacks = ["openai", "gpt-5.4"]"#, +fallbacks = ["openrouter:kimi-k3", "gpt-terra"]"#, ), Section::of::( "[cli.logging]", diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index f012a62ff..c250dd800 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -747,6 +747,12 @@ impl Catalog { &model.provider, )?; } + register_model_identifier( + identifiers, + resolved_settings.api_id.clone(), + model.id.clone(), + &model.provider, + )?; if model.default { defaults_by_provider @@ -800,7 +806,7 @@ impl Catalog { models.push(model); } let (offering_index, provider_selector_index, canonical_candidates, alias_candidates) = - build_model_indexes(&models); + build_model_indexes(&models, &model_settings_by_offering); Ok(Self { models, @@ -879,16 +885,26 @@ impl Catalog { .and_then(|idx| self.models.get(*idx)) } - /// Look up a selector on exactly one provider, without considering - /// provider availability. Historical built-in API identifiers normalize - /// to their canonical model slug before lookup. + /// Look up a canonical ID, alias, or API ID on exactly one provider, + /// without considering provider availability. Exact provider-scoped + /// identifiers win before historical built-in API identifiers normalize + /// to their canonical model slug. #[must_use] pub fn get_on_provider(&self, provider: &ProviderId, selector: &str) -> Option<&Model> { let provider = self.provider(provider)?; - let selector = normalize_legacy_builtin_selector(selector); - self.provider_selector_index - .get(&(provider.id.clone(), selector.into_owned())) - .and_then(|idx| self.models.get(*idx)) + let exact = self + .provider_selector_index + .get(&(provider.id.clone(), selector.to_string())); + let index = exact.or_else(|| { + let normalized = normalize_legacy_builtin_selector(selector); + (normalized.as_ref() != selector) + .then(|| { + self.provider_selector_index + .get(&(provider.id.clone(), normalized.into_owned())) + }) + .flatten() + }); + index.and_then(|idx| self.models.get(*idx)) } /// Look up a canonical offering by its composite identity. @@ -900,7 +916,7 @@ impl Catalog { .and_then(|idx| self.models.get(*idx)) } - /// Resolve a selector on exactly one provider. + /// Resolve a canonical ID, alias, or API ID on exactly one provider. pub fn resolve_on_provider( &self, provider: &ProviderId, @@ -926,8 +942,9 @@ impl Catalog { /// Historical built-in API identifiers normalize to their canonical model /// slug before selection. /// - /// An explicit provider is a pin. Unqualified selection checks canonical - /// IDs before aliases and uses the catalog's provider priority ordering. + /// An explicit provider is a pin and also permits that provider's API IDs. + /// Unqualified selection checks canonical IDs before aliases and uses the + /// catalog's provider priority ordering. pub fn select<'a>( &'a self, selector: &str, @@ -1472,7 +1489,10 @@ type ModelIndexes = ( HashMap>, ); -fn build_model_indexes(models: &[Model]) -> ModelIndexes { +fn build_model_indexes( + models: &[Model], + model_settings: &HashMap<(ProviderId, ModelId), CatalogModelSettings>, +) -> ModelIndexes { let mut offering_index = HashMap::new(); let mut provider_selector_index = HashMap::new(); let mut canonical_candidates = HashMap::>::new(); @@ -1489,6 +1509,10 @@ fn build_model_indexes(models: &[Model]) -> ModelIndexes { provider_selector_index.insert((model.provider.clone(), alias.clone()), idx); alias_candidates.entry(alias.clone()).or_default().push(idx); } + let settings = model_settings + .get(&(model.provider.clone(), model.id.clone())) + .expect("every catalog model should have resolved settings"); + provider_selector_index.insert((model.provider.clone(), settings.api_id.clone()), idx); } ( offering_index, @@ -4294,6 +4318,109 @@ reasoning = false )); } + #[test] + fn provider_scoped_lookup_accepts_canonical_alias_and_api_id_selectors() { + let catalog = Catalog::from_settings(&minimal_settings( + r#" +[providers.test] +display_name = "Test" +adapter = "openai" +agent_profile = "openai" +aliases = ["test-alias"] + +[providers.test.models.one] +api_id = "vendor/models/one:latest" +display_name = "One" +family = "test" +aliases = ["one-alias"] +default = true + +[providers.test.models.one.limits] +context_window = 1000 + +[providers.test.models.one.features] +tools = false +vision = false +reasoning = false +"#, + )) + .expect("provider-scoped selector fixture should build"); + + for selector in ["one", "one-alias", "vendor/models/one:latest"] { + let model = catalog + .resolve_on_provider(&ProviderId::new("test-alias"), selector) + .unwrap_or_else(|error| { + panic!("selector '{selector}' should resolve on provider alias: {error}") + }); + assert_eq!(model.provider, ProviderId::new("test"), "{selector}"); + assert_eq!(model.id, "one", "{selector}"); + } + + assert!(matches!( + catalog.select( + "vendor/models/one:latest", + None, + &HashSet::from([ProviderId::new("test")]), + ), + Err(ModelSelectionError::UnknownSelector { selector }) + if selector == "vendor/models/one:latest" + )); + } + + #[test] + fn catalog_from_settings_rejects_duplicate_provider_api_ids() { + let layer = minimal_settings( + r#" +[providers.test] +display_name = "Test" +adapter = "openai" +agent_profile = "openai" + +[providers.test.models.one] +api_id = "vendor/shared" +display_name = "One" +family = "test" +default = true + +[providers.test.models.one.limits] +context_window = 1000 + +[providers.test.models.one.features] +tools = false +vision = false +reasoning = false + +[providers.test.models.two] +api_id = "vendor/shared" +display_name = "Two" +family = "test" + +[providers.test.models.two.limits] +context_window = 1000 + +[providers.test.models.two.features] +tools = false +vision = false +reasoning = false +"#, + ); + + let err = Catalog::from_settings(&layer).unwrap_err(); + + assert!(matches!( + err, + CatalogBuildError::DuplicateProviderModelSelector { + provider, + selector, + first, + second, + } if provider == ProviderId::new("test") + && selector == "vendor/shared" + && first == "one" + && second == "two" + )); + } + #[test] fn provider_scoped_model_rejects_redundant_provider_field() { let error = Catalog::from_settings(&minimal_settings( diff --git a/lib/foundation/fabro-types/src/settings/model_ref.rs b/lib/foundation/fabro-types/src/settings/model_ref.rs index eb12263da..6d88565ad 100644 --- a/lib/foundation/fabro-types/src/settings/model_ref.rs +++ b/lib/foundation/fabro-types/src/settings/model_ref.rs @@ -4,8 +4,11 @@ //! //! - a bare token such as `openai` or `gpt-5.4` — the parser cannot tell alone //! whether the token is a provider name or a model alias -//! - a qualified reference such as `gemini/gemini-flash`, which names both a -//! provider and a model +//! - a qualified reference such as `gemini:gemini-flash`, which names both a +//! provider and a model selector +//! - a legacy qualified reference such as `gemini/gemini-flash`, which is +//! accepted on input and serialized using the canonical `provider:selector` +//! form //! //! The parser produces [`ModelRef`]; ambiguity resolution against a known //! registry of providers and models happens at consumption time via @@ -23,8 +26,8 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; pub enum ModelRef { /// A bare token. May be a provider name, a model alias, or a model id. Bare(String), - /// A provider-qualified model reference. - Qualified { provider: String, model: String }, + /// A provider-qualified model selector. + Qualified { provider: String, selector: String }, } /// An error returned when parsing a model reference fails. @@ -32,10 +35,9 @@ pub enum ModelRef { pub enum ParseModelRefError { /// The input was empty or whitespace only. Empty, - /// The input contained more than one `/`, which is not a valid qualified - /// ref. + /// A legacy slash-qualified input contained more than one `/`. TooManySlashes { input: String }, - /// The provider or model side of a qualified reference was empty. + /// The provider or selector side of a qualified reference was empty. EmptySide { input: String }, } @@ -46,13 +48,13 @@ impl fmt::Display for ParseModelRefError { Self::TooManySlashes { input } => { write!( f, - "model reference {input:?}: expected at most one \"/\" separator between provider and model" + "model reference {input:?}: legacy provider/model references allow one \"/\"; use \"provider:selector\" when the selector contains \"/\"" ) } Self::EmptySide { input } => { write!( f, - "model reference {input:?}: provider and model sides must both be non-empty" + "model reference {input:?}: provider and selector sides must both be non-empty" ) } } @@ -70,18 +72,30 @@ impl FromStr for ModelRef { return Err(ParseModelRefError::Empty); } + if let Some((provider, selector)) = trimmed.split_once(':') { + if provider.is_empty() || selector.is_empty() { + return Err(ParseModelRefError::EmptySide { + input: input.to_owned(), + }); + } + return Ok(Self::Qualified { + provider: provider.to_owned(), + selector: selector.to_owned(), + }); + } + let parts: Vec<&str> = trimmed.split('/').collect(); match parts.as_slice() { [bare] => Ok(Self::Bare((*bare).to_owned())), - [provider, model] => { - if provider.is_empty() || model.is_empty() { + [provider, selector] => { + if provider.is_empty() || selector.is_empty() { Err(ParseModelRefError::EmptySide { input: input.to_owned(), }) } else { Ok(Self::Qualified { provider: (*provider).to_owned(), - model: (*model).to_owned(), + selector: (*selector).to_owned(), }) } } @@ -96,7 +110,7 @@ impl fmt::Display for ModelRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Bare(token) => f.write_str(token), - Self::Qualified { provider, model } => write!(f, "{provider}/{model}"), + Self::Qualified { provider, selector } => write!(f, "{provider}:{selector}"), } } } @@ -113,7 +127,7 @@ impl fmt::Display for AmbiguousModelRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "model reference {:?} is ambiguous: matches provider names {:?} and model names {:?}; qualify it as \"provider/model\"", + "model reference {:?} is ambiguous: matches provider names {:?} and model names {:?}; qualify it as \"provider:model\"", self.input, self.providers, self.models ) } @@ -130,7 +144,7 @@ pub enum ResolvedModelRef { /// The reference named a model (qualified or unambiguously bare). Model { provider: Option, - model: String, + selector: String, }, } @@ -157,9 +171,9 @@ impl ModelRef { registry: &dyn ModelRegistry, ) -> Result { match self { - Self::Qualified { provider, model } => Ok(ResolvedModelRef::Model { + Self::Qualified { provider, selector } => Ok(ResolvedModelRef::Model { provider: Some(provider.clone()), - model: model.clone(), + selector: selector.clone(), }), Self::Bare(token) => { let is_provider = registry.is_provider(token); @@ -174,7 +188,7 @@ impl ModelRef { // Known and unknown bare models leave provider selection to the runtime. (false, _) => Ok(ResolvedModelRef::Model { provider: None, - model: token.clone(), + selector: token.clone(), }), } } @@ -197,7 +211,7 @@ impl<'de> Deserialize<'de> for ModelRef { fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str( - r#"a model reference such as "openai", "gpt-5.4", or "gemini/gemini-flash""#, + r#"a model reference such as "openai", "gpt-5.4", or "gemini:gemini-flash""#, ) } @@ -241,12 +255,43 @@ mod tests { } #[test] - fn parses_qualified() { + fn parses_colon_qualified() { + assert_eq!( + "gemini:gemini-flash".parse::().unwrap(), + ModelRef::Qualified { + provider: "gemini".into(), + selector: "gemini-flash".into(), + } + ); + } + + #[test] + fn parses_legacy_slash_qualified() { assert_eq!( "gemini/gemini-flash".parse::().unwrap(), ModelRef::Qualified { provider: "gemini".into(), - model: "gemini-flash".into(), + selector: "gemini-flash".into(), + } + ); + } + + #[test] + fn colon_qualified_selector_may_contain_slashes_and_colons() { + assert_eq!( + "openrouter:moonshotai/kimi-k3".parse::().unwrap(), + ModelRef::Qualified { + provider: "openrouter".into(), + selector: "moonshotai/kimi-k3".into(), + } + ); + assert_eq!( + "bedrock:us.anthropic.claude-haiku-4-5-20251001-v1:0" + .parse::() + .unwrap(), + ModelRef::Qualified { + provider: "bedrock".into(), + selector: "us.anthropic.claude-haiku-4-5-20251001-v1:0".into(), } ); } @@ -255,6 +300,10 @@ mod tests { fn rejects_too_many_slashes() { let err = "a/b/c".parse::().unwrap_err(); assert!(matches!(err, ParseModelRefError::TooManySlashes { .. })); + assert_eq!( + err.to_string(), + r#"model reference "a/b/c": legacy provider/model references allow one "/"; use "provider:selector" when the selector contains "/""# + ); } #[test] @@ -267,6 +316,14 @@ mod tests { "foo/".parse::().unwrap_err(), ParseModelRefError::EmptySide { .. } )); + assert!(matches!( + ":foo".parse::().unwrap_err(), + ParseModelRefError::EmptySide { .. } + )); + assert!(matches!( + "foo:".parse::().unwrap_err(), + ParseModelRefError::EmptySide { .. } + )); } #[test] @@ -296,7 +353,7 @@ mod tests { let resolved = ModelRef::Bare("gpt-5.4".into()).resolve(®).unwrap(); assert_eq!(resolved, ResolvedModelRef::Model { provider: None, - model: "gpt-5.4".into(), + selector: "gpt-5.4".into(), }); } @@ -320,24 +377,35 @@ mod tests { }; let resolved = ModelRef::Qualified { provider: "a".into(), - model: "b".into(), + selector: "b".into(), } .resolve(®) .unwrap(); assert_eq!(resolved, ResolvedModelRef::Model { provider: Some("a".into()), - model: "b".into(), + selector: "b".into(), }); } #[test] fn display_round_trip() { - for input in ["openai", "gpt-5.4", "gemini/gemini-flash"] { + for input in [ + "openai", + "gpt-5.4", + "gemini:gemini-flash", + "openrouter:moonshotai/kimi-k3", + ] { let parsed: ModelRef = input.parse().unwrap(); assert_eq!(parsed.to_string(), input); } } + #[test] + fn display_canonicalizes_legacy_slash_separator() { + let parsed: ModelRef = "gemini/gemini-flash".parse().unwrap(); + assert_eq!(parsed.to_string(), "gemini:gemini-flash"); + } + #[test] fn serde_round_trip_via_json() { #[derive(Debug, serde::Deserialize, serde::Serialize, PartialEq)] @@ -345,12 +413,12 @@ mod tests { m: ModelRef, } - let input = r#"{"m":"gemini/gemini-flash"}"#; + let input = r#"{"m":"openrouter:moonshotai/kimi-k3"}"#; let parsed: Wrap = serde_json::from_str(input).unwrap(); assert!(matches!( parsed.m, - ModelRef::Qualified { ref provider, ref model } - if provider == "gemini" && model == "gemini-flash" + ModelRef::Qualified { ref provider, ref selector } + if provider == "openrouter" && selector == "moonshotai/kimi-k3" )); let rendered = serde_json::to_string(&parsed).unwrap(); assert_eq!(rendered, input); diff --git a/lib/packages/fabro-api-client/src/models/create-run-session-request.ts b/lib/packages/fabro-api-client/src/models/create-run-session-request.ts index bea185e6d..20f9c44b2 100644 --- a/lib/packages/fabro-api-client/src/models/create-run-session-request.ts +++ b/lib/packages/fabro-api-client/src/models/create-run-session-request.ts @@ -17,7 +17,7 @@ export interface CreateRunSessionRequest { 'title'?: string; /** - * Catalog model ID or alias. The server selects among ready providers and stores the canonical model ID. + * Catalog model ID or alias, optionally qualified as `provider:selector`. A provider-qualified selector may be a canonical model ID, alias, or provider API ID. Legacy `provider/model` references remain accepted. The server stores the canonical model ID. */ 'model'?: string; /**