Qualify colon model references against the provider registry

Splitting on the first colon in FromStr broke bare model IDs that
legitimately contain one. A reference like "llama3:8b" parsed as
provider "llama3" selector "8b", and since "llama3" is not a provider
the lookup failed instead of passing the ID through to the pinned
provider. Verified against origin/main: canonical_session_model with
"future-model:latest" pinned to openrouter returned the passthrough
before and a 400 after.

This is not fixable by choosing a different separator. Bedrock
inference-profile ARNs contain both colons and slashes, and
docs/public/integrations/bedrock.mdx tells users to put arbitrary
inference-profile IDs in api_id. Only the registry can tell a provider
prefix from a model ID that happens to contain the separator.

FromStr now leaves colon-bearing tokens bare, and ModelRef::qualify
promotes only those whose prefix names a known provider. resolve()
applies it, so the fallback path is covered; sessions.rs applies it
before its own match so it keeps its tailored ambiguity messages.

ModelRegistry is now implemented for Catalog in fabro-types, replacing
the CatalogModelRegistry wrapper that existed only in start.rs, so both
call sites share one registry view.

Covered by regression tests at both surfaces, plus qualify unit tests
for ollama tags and Bedrock ARNs. The pre-existing passthrough test
canonical_session_model_preserves_unknown_passthrough_on_selected_provider
passes again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-07-28 17:02:16 -04:00
parent b2942519d5
commit 2103e3fbde
No known key found for this signature in database
6 changed files with 211 additions and 82 deletions

View file

@ -7943,7 +7943,9 @@ components:
description: |
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
canonical model ID, alias, or provider API ID. A value counts as
qualified only when the text before the first `:` names a known
provider, so model IDs containing a colon stay whole. Legacy
`provider/model` references remain accepted. The server stores the
canonical model ID.
provider:
@ -14126,7 +14128,10 @@ components:
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
or provider API ID and may contain `/` or additional colons. A value
is treated as qualified only when the text before the first `:` names
a known provider, so model IDs that contain a colon — ollama
`name:tag` values, Bedrock inference-profile IDs — stay whole. Legacy
`provider/model` references remain accepted.
example: openrouter:moonshotai/kimi-k3

View file

@ -144,7 +144,7 @@ name = "claude-sonnet-4-5"
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.<id>]`.
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:
For a qualified fallback, the selector may be that provider's canonical model ID, alias, or API ID. Fabro splits on the first `:` when the part before it names a known provider, so provider API IDs may contain `/` or additional colons:
```toml title="run.toml"
[run.model]
@ -156,6 +156,8 @@ fallbacks = [
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`.
A colon alone does not make a reference qualified. Many model IDs contain one — ollama `name:tag` values, Bedrock inference-profile IDs and ARNs — so Fabro treats the reference as qualified only when the text before the first `:` names a known provider. `"llama3:8b"` stays a single model ID, while `"ollama:llama3:8b"` pins the `ollama` provider and passes `llama3:8b` as the selector.
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.

View file

@ -857,7 +857,8 @@ fn canonical_session_model(
}
let model_ref = requested
.parse::<SettingsModelRef>()
.map_err(|err| ApiError::bad_request(err.to_string()))?;
.map_err(|err| ApiError::bad_request(err.to_string()))?
.qualify(catalog);
let (qualified_provider, selector) = match model_ref {
SettingsModelRef::Qualified { provider, selector } => {
let requested_provider = ProviderId::new(provider);
@ -1629,6 +1630,31 @@ reasoning = false
);
}
/// A colon in a model ID does not make it provider-qualified. Ollama
/// `name:tag` values and Bedrock ARNs must still reach the provider.
#[test]
fn canonical_session_model_passes_through_colon_bearing_model_ids() {
let catalog = portable_session_catalog();
let openai = ProviderId::openai();
let openrouter = ProviderId::new("openrouter");
let both = std::collections::HashSet::from([openai.clone(), openrouter.clone()]);
assert_eq!(
canonical_session_model(
&catalog,
&both,
Some("future-model:latest"),
Some(&openrouter),
)
.unwrap(),
(openrouter, "future-model:latest".to_string())
);
assert_eq!(
canonical_session_model(&catalog, &both, Some("future-model:latest"), None).unwrap(),
(openai, "future-model:latest".to_string())
);
}
#[test]
fn canonical_session_model_rejects_an_unavailable_explicit_provider() {
let catalog = portable_session_catalog();

View file

@ -15,12 +15,12 @@ use fabro_sandbox::from_environment::{
};
use fabro_sandbox::{DockerSandboxOptions, SandboxSpec};
use fabro_static::EnvVars;
use fabro_types::settings::ResolvedModelRef;
use fabro_types::settings::run::{
ApprovalMode, McpServerSettings as ResolvedMcpServerSettings, PullRequestSettings,
ResolvedMcpEntry, RunMode, RunModelSettings as ResolvedRunModelSettings,
RunNamespace as ResolvedRunSettings, RunPrepareSettings as ResolvedRunPrepareSettings,
};
use fabro_types::settings::{ModelRegistry, ResolvedModelRef};
use fabro_types::{ManifestPath, RunId, RunRunnableSource, SandboxProviderKind};
use fabro_vault::Vault;
use tokio::runtime::Handle;
@ -645,12 +645,11 @@ fn resolve_fallback_chain(
if settings.fallbacks.is_empty() {
return Ok(Vec::new());
}
let registry = CatalogModelRegistry { catalog };
let primary = catalog.get_on_provider(provider, model);
let mut chain = Vec::new();
for model_ref in &settings.fallbacks {
match model_ref.resolve(&registry)? {
match model_ref.resolve(catalog)? {
ResolvedModelRef::Provider(provider_name) => {
let provider_id = canonical_provider_id(catalog, &provider_name);
if !eligible.contains(&provider_id) {
@ -718,20 +717,6 @@ fn canonical_provider_id(catalog: &Catalog, provider_name: &str) -> ProviderId {
.map_or(provider_id, |provider| provider.id.clone())
}
struct CatalogModelRegistry<'a> {
catalog: &'a Catalog,
}
impl ModelRegistry for CatalogModelRegistry<'_> {
fn is_provider(&self, token: &str) -> bool {
self.catalog.provider(&ProviderId::from(token)).is_some()
}
fn is_model(&self, token: &str) -> bool {
self.catalog.is_model_selector(token)
}
}
/// Build the launch-time MCP config from resolved settings, resolving any
/// `{{ env.* }}` and `{{ secrets.* }}` tokens in the transport
/// (`command`/`url`/`env`/`headers`) against the worker process environment and
@ -1455,6 +1440,32 @@ enabled = true
}
}
/// A colon in a model ID does not make it provider-qualified, so an
/// unknown colon-bearing selector still passes through to a provider
/// instead of failing the run with an unknown-provider error.
#[test]
fn resolve_fallback_chain_passes_through_colon_bearing_model_ids() {
let catalog = portable_model_catalog();
let settings = ResolvedRunModelSettings {
fallbacks: vec!["future-model:latest".parse::<ModelRef>().unwrap()],
..ResolvedRunModelSettings::default()
};
let chain = resolve_fallback_chain(
&catalog,
&ProviderId::openai(),
"gpt-5.6-sol",
&settings,
&HashSet::from([ProviderId::openai(), ProviderId::new("openrouter")]),
)
.unwrap();
assert_eq!(chain, vec![FallbackTarget {
provider: ProviderId::openai().to_string(),
model: "future-model:latest".to_string(),
}]);
}
#[test]
fn resolve_fallback_chain_keeps_qualified_legacy_references_as_provider_pins() {
let catalog = test_catalog();

View file

@ -13,6 +13,12 @@
//! The parser produces [`ModelRef`]; ambiguity resolution against a known
//! registry of providers and models happens at consumption time via
//! [`ModelRef::resolve`].
//!
//! That split matters for the `:` form. Model IDs legitimately contain colons —
//! ollama `name:tag` values, Bedrock inference-profile ARNs — so no separator
//! is safe to split on by shape alone. Parsing leaves colon-bearing tokens bare
//! and [`ModelRef::qualify`] promotes only the ones whose prefix names a
//! provider.
use std::fmt;
use std::str::FromStr;
@ -72,22 +78,25 @@ impl FromStr for ModelRef {
return Err(ParseModelRefError::Empty);
}
// A `:` splits provider from selector. The selector keeps any further
// `:` or `/`, so provider API IDs survive intact. Without a `:`, a
// single `/` is the legacy qualified form.
let (provider, selector) = match trimmed.split_once(':') {
Some(qualified) => qualified,
None => match trimmed.split_once('/') {
Some((_, selector)) if selector.contains('/') => {
return Err(ParseModelRefError::TooManySlashes {
input: input.to_owned(),
});
}
Some(legacy) => legacy,
None => return Ok(Self::Bare(trimmed.to_owned())),
},
};
// A `:` may separate provider from selector, but model IDs legitimately
// contain colons — ollama `name:tag` values, Bedrock ARNs. Only a
// registry can tell the two apart, so colon-bearing tokens stay bare
// here and [`ModelRef::qualify`] promotes the ones that name a
// provider.
if trimmed.contains(':') {
return Ok(Self::Bare(trimmed.to_owned()));
}
// Legacy `provider/model`. A selector with a further `/` needs the
// `provider:selector` form.
let Some((provider, selector)) = trimmed.split_once('/') else {
return Ok(Self::Bare(trimmed.to_owned()));
};
if selector.contains('/') {
return Err(ParseModelRefError::TooManySlashes {
input: input.to_owned(),
});
}
if provider.is_empty() || selector.is_empty() {
return Err(ParseModelRefError::EmptySide {
input: input.to_owned(),
@ -153,9 +162,37 @@ pub trait ModelRegistry {
}
impl ModelRef {
/// Promote a bare `provider:selector` token to [`ModelRef::Qualified`] when
/// the prefix names a known provider.
///
/// Parsing alone cannot do this. A model ID may itself contain a colon —
/// ollama `name:tag` values, Bedrock inference-profile ARNs — and those
/// must stay whole. Anything else is returned unchanged.
#[must_use]
pub fn qualify(self, registry: &dyn ModelRegistry) -> Self {
let Self::Bare(token) = self else {
return self;
};
match token.split_once(':') {
Some((provider, selector))
if !provider.is_empty()
&& !selector.is_empty()
&& registry.is_provider(provider) =>
{
Self::Qualified {
provider: provider.to_owned(),
selector: selector.to_owned(),
}
}
_ => Self::Bare(token),
}
}
/// Resolve this reference against a registry.
///
/// - [`ModelRef::Qualified`] always resolves to a model.
/// - A bare `provider:selector` token is qualified first — see
/// [`ModelRef::qualify`].
/// - [`ModelRef::Bare`] resolves to a provider if the token is only a
/// provider, to a model if the token is only a model, and returns
/// [`AmbiguousModelRef`] if the token matches both a provider and a model
@ -164,25 +201,25 @@ impl ModelRef {
&self,
registry: &dyn ModelRegistry,
) -> Result<ResolvedModelRef, AmbiguousModelRef> {
match self {
match self.clone().qualify(registry) {
Self::Qualified { provider, selector } => Ok(ResolvedModelRef::Model {
provider: Some(provider.clone()),
selector: selector.clone(),
provider: Some(provider),
selector,
}),
Self::Bare(token) => {
let is_provider = registry.is_provider(token);
let is_model = registry.is_model(token);
let is_provider = registry.is_provider(&token);
let is_model = registry.is_model(&token);
match (is_provider, is_model) {
(true, false) => Ok(ResolvedModelRef::Provider(token.clone())),
(true, false) => Ok(ResolvedModelRef::Provider(token)),
(true, true) => Err(AmbiguousModelRef {
input: token.clone(),
providers: vec![token.clone()],
models: vec![token.clone()],
models: vec![token],
}),
// Known and unknown bare models leave provider selection to the runtime.
(false, _) => Ok(ResolvedModelRef::Model {
provider: None,
selector: token.clone(),
selector: token,
}),
}
}
@ -190,6 +227,17 @@ impl ModelRef {
}
}
impl ModelRegistry for fabro_model::Catalog {
fn is_provider(&self, token: &str) -> bool {
self.provider(&fabro_model::ProviderId::from(token))
.is_some()
}
fn is_model(&self, token: &str) -> bool {
self.is_model_selector(token)
}
}
impl Serialize for ModelRef {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_str(&self.to_string())
@ -248,15 +296,21 @@ mod tests {
);
}
/// Parsing cannot tell a provider prefix from a model ID that contains a
/// colon, so it defers to [`ModelRef::qualify`].
#[test]
fn parses_colon_qualified() {
assert_eq!(
"gemini:gemini-flash".parse::<ModelRef>().unwrap(),
ModelRef::Qualified {
provider: "gemini".into(),
selector: "gemini-flash".into(),
}
);
fn parses_colon_tokens_as_bare() {
for input in [
"gemini:gemini-flash",
"openrouter:moonshotai/kimi-k3",
"bedrock:us.anthropic.claude-haiku-4-5-20251001-v1:0",
] {
assert_eq!(
input.parse::<ModelRef>().unwrap(),
ModelRef::Bare(input.into()),
"{input}"
);
}
}
#[test]
@ -271,23 +325,55 @@ mod tests {
}
#[test]
fn colon_qualified_selector_may_contain_slashes_and_colons() {
assert_eq!(
"openrouter:moonshotai/kimi-k3".parse::<ModelRef>().unwrap(),
ModelRef::Qualified {
provider: "openrouter".into(),
selector: "moonshotai/kimi-k3".into(),
}
);
assert_eq!(
"bedrock:us.anthropic.claude-haiku-4-5-20251001-v1:0"
.parse::<ModelRef>()
.unwrap(),
ModelRef::Qualified {
provider: "bedrock".into(),
selector: "us.anthropic.claude-haiku-4-5-20251001-v1:0".into(),
}
);
fn qualify_promotes_a_known_provider_prefix() {
let reg = TestRegistry {
providers: &["openrouter", "bedrock"],
models: &[],
};
for (input, selector) in [
("openrouter:kimi-k3", "kimi-k3"),
("openrouter:moonshotai/kimi-k3", "moonshotai/kimi-k3"),
(
"bedrock:us.anthropic.claude-haiku-4-5-20251001-v1:0",
"us.anthropic.claude-haiku-4-5-20251001-v1:0",
),
] {
let qualified = input.parse::<ModelRef>().unwrap().qualify(&reg);
let provider = input.split_once(':').unwrap().0;
assert_eq!(
qualified,
ModelRef::Qualified {
provider: provider.into(),
selector: selector.into(),
},
"{input}"
);
}
}
/// The regression this guards: a model ID that merely contains a colon —
/// an ollama `name:tag`, a Bedrock ARN — must not be read as qualified.
#[test]
fn qualify_leaves_colon_bearing_model_ids_bare() {
let reg = TestRegistry {
providers: &["ollama", "bedrock"],
models: &[],
};
for input in [
"llama3:8b",
"qwen3.5:latest",
"us.anthropic.claude-haiku-4-5-20251001-v1:0",
"arn:aws:bedrock:us-east-1:1234:inference-profile/us.anthropic.claude-fable-5",
":foo",
"foo:",
] {
let parsed = input.parse::<ModelRef>().unwrap();
assert_eq!(
parsed.clone().qualify(&reg),
parsed,
"{input} should stay bare"
);
}
}
#[test]
@ -310,14 +396,6 @@ mod tests {
"foo/".parse::<ModelRef>().unwrap_err(),
ParseModelRefError::EmptySide { .. }
));
assert!(matches!(
":foo".parse::<ModelRef>().unwrap_err(),
ParseModelRefError::EmptySide { .. }
));
assert!(matches!(
"foo:".parse::<ModelRef>().unwrap_err(),
ParseModelRefError::EmptySide { .. }
));
}
#[test]
@ -407,14 +485,21 @@ mod tests {
m: ModelRef,
}
// Colon tokens stay bare until a registry qualifies them, and survive
// the round trip verbatim either way.
let input = r#"{"m":"openrouter:moonshotai/kimi-k3"}"#;
let parsed: Wrap = serde_json::from_str(input).unwrap();
assert!(matches!(
assert_eq!(
parsed.m,
ModelRef::Qualified { ref provider, ref selector }
if provider == "openrouter" && selector == "moonshotai/kimi-k3"
));
ModelRef::Bare("openrouter:moonshotai/kimi-k3".into())
);
let rendered = serde_json::to_string(&parsed).unwrap();
assert_eq!(rendered, input);
let legacy: Wrap = serde_json::from_str(r#"{"m":"gemini/gemini-flash"}"#).unwrap();
assert_eq!(
serde_json::to_string(&legacy).unwrap(),
r#"{"m":"gemini:gemini-flash"}"#
);
}
}

View file

@ -17,7 +17,7 @@
export interface CreateRunSessionRequest {
'title'?: string;
/**
* 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.
* 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. A value counts as qualified only when the text before the first `:` names a known provider, so model IDs containing a colon stay whole. Legacy `provider/model` references remain accepted. The server stores the canonical model ID.
*/
'model'?: string;
/**