Simplify model reference parsing and catalog indexing

Follow-up cleanup on the provider-qualified selector work.

- build_model_indexes now takes the paired (Model, CatalogModelSettings)
  slice it is built from, instead of a separate settings map. This drops
  a per-model map lookup with two cloned key components and removes the
  expect() panic path for an invariant the caller already guarantees.
- get_on_provider expresses the exact-then-legacy lookup as one closure
  applied twice, rather than a nested then/flatten chain.
- ModelRef::from_str selects the separator first and then checks both
  sides once, so the empty-side check is no longer duplicated across two
  branches and the slash split no longer allocates a Vec.
- Shorten the TooManySlashes message to the action the user should take.
- Merge the two near-identical fallback chain tests into one that runs
  both qualified selector forms through the same assertion.
- The fallbacks splice test now asserts through the existing Serialize
  impl instead of hand-rolling the ModelRefOrSplice rendering.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

lib/components/fabro-workflow/src/operations/start.rs

@@ -1402,8 +1402,11 @@ reasoning = false
         }]);
     }
 
+    /// A qualified fallback resolves to the same offering whether the selector
+    /// is the canonical model ID or the provider's API ID. The trailing bare
+    /// alias still goes through ready-provider priority selection.
     #[test]
-    fn resolve_fallback_chain_supports_openrouter_kimi_then_portable_terra() {
+    fn resolve_fallback_chain_resolves_qualified_model_id_and_api_id_alike() {
         let overrides: fabro_model::catalog::LlmCatalogSettings = toml::from_str(
             r"
 [providers.openrouter]
@@ -1412,76 +1415,44 @@ enabled = true
         )
         .unwrap();
         let catalog = Catalog::from_builtin_with_overrides(&overrides).unwrap();
-        let settings = ResolvedRunModelSettings {
-            fallbacks: vec![
-                "openrouter:kimi-k3".parse::<ModelRef>().unwrap(),
-                "gpt-terra".parse::<ModelRef>().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();
+        for selector in ["openrouter:kimi-k3", "openrouter:moonshotai/kimi-k3"] {
+            let settings = ResolvedRunModelSettings {
+                fallbacks: vec![
+                    selector.parse::<ModelRef>().unwrap(),
+                    "gpt-terra".parse::<ModelRef>().unwrap(),
+                ],
+                ..ResolvedRunModelSettings::default()
+            };
 
-        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(),
-            },
-        ]);
-    }
+            let chain = resolve_fallback_chain(
+                &catalog,
+                &ProviderId::new("kimi"),
+                "kimi-k3",
+                &settings,
+                &HashSet::from([
+                    ProviderId::new("kimi"),
+                    ProviderId::new("openrouter"),
+                    ProviderId::openai(),
+                ]),
+            )
+            .unwrap();
 
-    #[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::<ModelRef>().unwrap(),
-                "gpt-terra".parse::<ModelRef>().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(),
-            },
-        ]);
+            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(),
+                    },
+                ],
+                "{selector}"
+            );
+        }
     }
 
     #[test]

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, ModelRefOrSplice, SettingsLayer, StringOrSplice};
+use crate::{Combine, SettingsLayer, StringOrSplice};
 
 fn parse(input: &str) -> SettingsLayer {
     input
@@ -112,18 +112,10 @@ fallbacks = ["anthropic", "..."]
     );
     let merged = higher.combine(lower);
     let fallbacks = merged.run.unwrap().model.unwrap().fallbacks;
-    let rendered = fallbacks
-        .iter()
-        .map(|entry| match entry {
-            ModelRefOrSplice::ModelRef(model_ref) => model_ref.to_string(),
-            ModelRefOrSplice::Splice => "...".to_string(),
-        })
-        .collect::<Vec<_>>();
-    assert_eq!(rendered, vec![
-        "anthropic",
-        "openrouter:moonshotai/kimi-k3",
-        "gpt-terra",
-    ]);
+    assert_eq!(
+        serde_json::to_value(&fallbacks).unwrap(),
+        serde_json::json!(["anthropic", "openrouter:moonshotai/kimi-k3", "gpt-terra",])
+    );
 }
 
 #[test]

lib/foundation/fabro-model/src/catalog.rs

@@ -799,14 +799,14 @@ impl Catalog {
                 .then_with(|| left.id.cmp(&right.id))
         });
         warn_multiple_probe_models(&models_with_settings);
+        let (offering_index, provider_selector_index, canonical_candidates, alias_candidates) =
+            build_model_indexes(&models_with_settings);
         let mut model_settings_by_offering = HashMap::new();
         let mut models = Vec::new();
         for (model, settings) in models_with_settings {
             model_settings_by_offering.insert((model.provider.clone(), model.id.clone()), settings);
             models.push(model);
         }
-        let (offering_index, provider_selector_index, canonical_candidates, alias_candidates) =
-            build_model_indexes(&models, &model_settings_by_offering);
 
         Ok(Self {
             models,
@@ -892,19 +892,13 @@ impl Catalog {
     #[must_use]
     pub fn get_on_provider(&self, provider: &ProviderId, selector: &str) -> Option<&Model> {
         let provider = self.provider(provider)?;
-        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))
+        let lookup = |selector: &str| {
+            self.provider_selector_index
+                .get(&(provider.id.clone(), selector.to_string()))
+        };
+        let index =
+            lookup(selector).or_else(|| lookup(&normalize_legacy_builtin_selector(selector)))?;
+        self.models.get(*index)
     }
 
     /// Look up a canonical offering by its composite identity.
@@ -1489,15 +1483,12 @@ type ModelIndexes = (
     HashMap<String, Vec<usize>>,
 );
 
-fn build_model_indexes(
-    models: &[Model],
-    model_settings: &HashMap<(ProviderId, ModelId), CatalogModelSettings>,
-) -> ModelIndexes {
+fn build_model_indexes(models: &[(Model, CatalogModelSettings)]) -> ModelIndexes {
     let mut offering_index = HashMap::new();
     let mut provider_selector_index = HashMap::new();
     let mut canonical_candidates = HashMap::<ModelId, Vec<usize>>::new();
     let mut alias_candidates = HashMap::<String, Vec<usize>>::new();
-    for (idx, model) in models.iter().enumerate() {
+    for (idx, (model, settings)) in models.iter().enumerate() {
         offering_index.insert((model.provider.clone(), model.id.clone()), idx);
         provider_selector_index
             .insert((model.provider.clone(), model.id.as_str().to_string()), idx);
@@ -1509,9 +1500,6 @@ fn build_model_indexes(
             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);
     }
     (

lib/foundation/fabro-types/src/settings/model_ref.rs

@@ -48,7 +48,7 @@ impl fmt::Display for ParseModelRefError {
             Self::TooManySlashes { input } => {
                 write!(
                     f,
-                    "model reference {input:?}: legacy provider/model references allow one \"/\"; use \"provider:selector\" when the selector contains \"/\""
+                    "model reference {input:?}: qualify it as \"provider:selector\" when the selector contains \"/\""
                 )
             }
             Self::EmptySide { input } => {
@@ -72,37 +72,31 @@ 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(),
+        // 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())),
+            },
+        };
+
+        if provider.is_empty() || selector.is_empty() {
+            return Err(ParseModelRefError::EmptySide {
+                input: input.to_owned(),
             });
         }
-
-        let parts: Vec<&str> = trimmed.split('/').collect();
-        match parts.as_slice() {
-            [bare] => Ok(Self::Bare((*bare).to_owned())),
-            [provider, selector] => {
-                if provider.is_empty() || selector.is_empty() {
-                    Err(ParseModelRefError::EmptySide {
-                        input: input.to_owned(),
-                    })
-                } else {
-                    Ok(Self::Qualified {
-                        provider: (*provider).to_owned(),
-                        selector: (*selector).to_owned(),
-                    })
-                }
-            }
-            _ => Err(ParseModelRefError::TooManySlashes {
-                input: input.to_owned(),
-            }),
-        }
+        Ok(Self::Qualified {
+            provider: provider.to_owned(),
+            selector: selector.to_owned(),
+        })
     }
 }
 
@@ -302,7 +296,7 @@ mod tests {
         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 "/""#
+            r#"model reference "a/b/c": qualify it as "provider:selector" when the selector contains "/""#
         );
     }