scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-11 22:53:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix: preserve legacy model selector compatibility

Browse source
This commit is contained in:
Bryan Helmkamp 2026-07-23 12:12:09 -04:00
parent 05e6507095
commit 14317ccaf7
No known key found for this signature in database
8 changed files with 271 additions and 109 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
docs/public/core-concepts/models.mdx
View file

@ -145,9 +145,9 @@ reasoning = false
`api_id` is the opaque model name sent to that provider's API. It defaults to the exact model slug, so omit it when the two strings match. Fabro does not infer vendor prefixes or rewrite the value.
<Warning>
Built-in catalog keys that formerly exposed an aggregator's wire API ID are retired selectors. Fabro rejects them with their replacement address instead of silently routing them. Some contain a slash and look provider-qualified: for example, the old OpenRouter key `openai/gpt-5.6-sol` is retired in favor of `openrouter/gpt-5.6-sol`. To pin the direct OpenAI offering, set `provider = "openai"` and `name = "gpt-5.6-sol"` as separate fields.
</Warning>
<Note>
Historical built-in catalog keys that exposed provider API IDs remain accepted as compatibility selectors. Fabro normalizes a primary or node selector such as `openai/gpt-5.6-sol` to the canonical `gpt-5.6-sol` slug before normal provider-aware selection. With no provider pin, the highest-priority ready offering wins; a separate `provider = "openrouter"` pin selects the OpenRouter offering. Fabro also normalizes these keys in legacy top-level `[llm.models]` rows without rewriting the settings file.
</Note>
Model roles are separate: `default = true` controls normal model selection for workflow execution, while `small_default = true` marks the provider's small/cheap utility model for metadata tasks such as generated run titles. If a provider has no small default, Fabro falls back to that provider's normal default.
@ -271,7 +271,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.
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.
<Note>
The precedence order is: node-level stylesheet > run config TOML > CLI flags > server defaults. More specific settings always win.

2
docs/public/execution/failures.mdx
View file

@ -127,6 +127,8 @@ When Anthropic fails, Fabro tries Gemini first, then OpenAI. Fallback resolution
- A qualified selector such as `"openrouter/gpt-56-sol"` resolves only within that provider.
- 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.
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.
### What triggers failover

2
docs/public/execution/run-configuration.mdx
View file

@ -146,6 +146,8 @@ Provider values are catalog provider ID strings. Built-in IDs like `anthropic` a
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.
#### `[run.model.controls]`
Set default model controls for all nodes that do not override them in the workflow stylesheet:

48
lib/crates/fabro-config/src/parse.rs
View file

@ -429,24 +429,46 @@ display_name = "Legacy"
}
#[test]
fn retired_legacy_model_id_names_replacement() {
let error = r#"
fn legacy_builtin_model_id_normalizes_with_explicit_provider() {
let parsed = r#"
[llm.models."openai/gpt-5.6-sol"]
provider = "openrouter"
display_name = "Configured Sol"
"#
.parse::<SettingsLayer>()
.unwrap_err();
.unwrap();
let llm = parsed.llm.unwrap();
assert!(matches!(
error,
ParseError::LlmCatalog(LegacyModelError::Retired {
identifier,
provider,
model,
}) if identifier == "openai/gpt-5.6-sol"
&& provider.as_str() == "openrouter"
&& model.as_str() == "gpt-5.6-sol"
));
assert!(llm.models.is_empty());
let model = llm
.providers
.get("openrouter")
.unwrap()
.models
.get("gpt-5.6-sol")
.unwrap();
assert_eq!(model.display_name.as_deref(), Some("Configured Sol"));
assert!(model.provider.is_none());
}
#[test]
fn legacy_builtin_model_id_without_provider_uses_historical_catalog_provider() {
let parsed = r#"
[llm.models."anthropic/claude-fable-5"]
display_name = "Configured Fable"
"#
.parse::<SettingsLayer>()
.unwrap();
let llm = parsed.llm.unwrap();
assert!(llm.models.is_empty());
assert!(
llm.providers
.get("openrouter")
.unwrap()
.models
.contains_key("claude-fable-5")
);
}
#[test]

202
lib/crates/fabro-model/src/catalog.rs
View file

@ -616,12 +616,6 @@ pub enum ModelSelectionError {
"no default model is available on an eligible provider; providers with defaults: {providers:?}"
)]
NoDefaultModel { providers: Vec<ProviderId> },
#[error("model identifier '{identifier}' has been retired; use '{provider}/{model}' instead")]
RetiredModelIdentifier {
identifier: String,
provider: ProviderId,
model: ModelId,
},
}
/// One provider/model pair chosen by [`Catalog::resolve_selection`]. The
@ -711,13 +705,11 @@ impl Catalog {
continue;
}
if let Some((replacement_provider, replacement_model)) =
retired_model_replacement(&model_id)
{
return Err(LegacyModelError::Retired {
if let Some((_, canonical_model)) = legacy_builtin_model(&model_id) {
return Err(LegacyModelError::LegacyIdentifierAsModelId {
identifier: model_id,
provider: replacement_provider,
model: replacement_model,
provider: provider.id.clone(),
model: canonical_model,
}
.into());
}
@ -870,12 +862,14 @@ impl Catalog {
}
/// Look up a selector on exactly one provider, without considering
/// provider availability.
/// provider availability. Historical built-in API identifiers normalize
/// to their canonical model slug before lookup.
#[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.to_string()))
.get(&(provider.id.clone(), selector.into_owned()))
.and_then(|idx| self.models.get(*idx))
}
@ -894,14 +888,6 @@ impl Catalog {
provider: &ProviderId,
selector: &str,
) -> Result<&Model, ModelSelectionError> {
if let Some((replacement_provider, replacement_model)) = retired_model_replacement(selector)
{
return Err(ModelSelectionError::RetiredModelIdentifier {
identifier: selector.to_string(),
provider: replacement_provider,
model: replacement_model,
});
}
let provider =
self.provider(provider)
.ok_or_else(|| ModelSelectionError::UnknownProvider {
@ -919,6 +905,9 @@ impl Catalog {
/// Select one concrete offering for a selector and ready-provider
/// snapshot.
///
/// 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.
pub fn select<'a>(
@ -927,14 +916,6 @@ impl Catalog {
explicit_provider: Option<&ProviderId>,
eligible_providers: &HashSet<ProviderId>,
) -> Result<&'a Model, ModelSelectionError> {
if let Some((replacement_provider, replacement_model)) = retired_model_replacement(selector)
{
return Err(ModelSelectionError::RetiredModelIdentifier {
identifier: selector.to_string(),
provider: replacement_provider,
model: replacement_model,
});
}
let eligible = eligible_providers
.iter()
.filter_map(|provider| self.provider(provider).map(|provider| provider.id.clone()))
@ -954,7 +935,10 @@ impl Catalog {
return self.resolve_on_provider(&provider.id, selector);
}
let canonical = self.canonical_candidates.get(&ModelId::new(selector));
let normalized_selector = normalize_legacy_builtin_selector(selector);
let canonical = self
.canonical_candidates
.get(&ModelId::new(normalized_selector.as_ref()));
if let Some(indices) = canonical {
if let Some(model) = indices
.iter()
@ -965,7 +949,7 @@ impl Catalog {
}
}
let aliases = self.alias_candidates.get(selector);
let aliases = self.alias_candidates.get(normalized_selector.as_ref());
if let Some(indices) = aliases {
if let Some(model) = indices
.iter()
@ -1117,9 +1101,10 @@ impl Catalog {
}
fn candidate_indices(&self, selector: &str) -> Option<&Vec<usize>> {
let selector = normalize_legacy_builtin_selector(selector);
self.canonical_candidates
.get(&ModelId::new(selector))
.or_else(|| self.alias_candidates.get(selector))
.get(&ModelId::new(selector.as_ref()))
.or_else(|| self.alias_candidates.get(selector.as_ref()))
}
#[must_use]
@ -1537,8 +1522,10 @@ fn reject_scoped_provider_fields(settings: &LlmCatalogSettings) -> Result<(), Le
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 {
#[error(
"legacy built-in model identifier '{identifier}' cannot be used as a canonical model ID under provider '{provider}'; use '{model}'"
)]
LegacyIdentifierAsModelId {
identifier: String,
provider: ProviderId,
model: ModelId,
@ -1647,20 +1634,23 @@ impl LegacyModelIndex {
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.
/// Resolve one legacy row to its provider-scoped address. Historical
/// built-in identifiers normalize to their canonical slug and use their
/// historical provider when no explicit provider is present. Other
/// unknown explicit providers or model selectors pass 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((historical_provider, model)) = legacy_builtin_model(legacy_id) {
let provider = explicit_provider.map_or(historical_provider, |explicit| {
self.canonical_provider(explicit)
.unwrap_or_else(|| ProviderId::new(explicit))
});
return Ok((provider, model));
}
if let Some(explicit) = explicit_provider {
let provider = self
@ -1755,11 +1745,11 @@ impl LegacyModelIndex {
}
}
/// Built-in catalog keys retired when provider API identifiers stopped being
/// Fabro model IDs. Keep this list explicit so old workflow and persisted-run
/// references fail with an actionable replacement instead of silently
/// selecting another route.
const RETIRED_MODEL_IDENTIFIERS: &[(&str, &str, &str)] = &[
/// Historical built-in catalog keys from before Fabro separated canonical
/// model slugs from provider API identifiers. The provider records the key's
/// original offering for legacy catalog-row normalization; runtime selectors
/// normalize to the model slug and use normal provider-aware selection.
const LEGACY_BUILTIN_MODEL_IDENTIFIERS: &[(&str, &str, &str)] = &[
("openai.gpt-5.5", "bedrock-openai", "gpt-5.5"),
("openai.gpt-5.4", "bedrock-openai", "gpt-5.4"),
(
@ -1833,15 +1823,23 @@ const RETIRED_MODEL_IDENTIFIERS: &[(&str, &str, &str)] = &[
("mistralai/devstral-2512", "openrouter", "devstral-2512"),
];
/// Return the replacement address for a retired built-in catalog key.
/// Return the historical provider and canonical model slug for a legacy
/// built-in catalog key.
#[must_use]
pub fn retired_model_replacement(identifier: &str) -> Option<(ProviderId, ModelId)> {
RETIRED_MODEL_IDENTIFIERS
pub fn legacy_builtin_model(identifier: &str) -> Option<(ProviderId, ModelId)> {
LEGACY_BUILTIN_MODEL_IDENTIFIERS
.iter()
.find(|(retired, _, _)| *retired == identifier)
.find(|(legacy, _, _)| *legacy == identifier)
.map(|(_, provider, model)| (ProviderId::new(*provider), ModelId::new(*model)))
}
fn normalize_legacy_builtin_selector(selector: &str) -> Cow<'_, str> {
legacy_builtin_model(selector).map_or_else(
|| Cow::Borrowed(selector),
|(_, model)| Cow::Owned(model.into_inner()),
)
}
fn merge_catalog_settings(
higher: LlmCatalogSettings,
mut fallback: LlmCatalogSettings,
@ -3105,6 +3103,47 @@ enabled = true
}
}
#[test]
fn builtin_legacy_vendor_ids_normalize_for_pinned_and_unpinned_selection() {
let catalog = Catalog::from_builtin_with_overrides(&minimal_settings(
r"
[providers.openrouter]
enabled = true
",
))
.expect("enabled OpenRouter override should build from the built-in provider settings");
let openrouter = ProviderId::new("openrouter");
for (selector, canonical_id) in [
("anthropic/claude-fable-5", "claude-fable-5"),
("openai/gpt-5.6-sol", "gpt-5.6-sol"),
] {
let model = catalog
.resolve_on_provider(&openrouter, selector)
.unwrap_or_else(|error| panic!("{selector} should resolve on OpenRouter: {error}"));
assert_eq!(model.provider, openrouter, "{selector}");
assert_eq!(model.id, canonical_id, "{selector}");
}
let anthropic = ProviderId::anthropic();
let selector = "anthropic/claude-fable-5";
let selected = catalog
.resolve_selection(
Some(selector),
None,
&HashSet::from([anthropic.clone(), openrouter.clone()]),
)
.unwrap();
assert_eq!(selected.provider, anthropic);
assert_eq!(selected.model, "claude-fable-5");
let selected = catalog
.resolve_selection(Some(selector), None, &HashSet::from([openrouter.clone()]))
.unwrap();
assert_eq!(selected.provider, openrouter);
assert_eq!(selected.model, "claude-fable-5");
}
#[test]
fn builtin_openrouter_includes_glm_5_2_when_enabled() {
let catalog = Catalog::from_builtin_with_overrides(&minimal_settings(
@ -3803,6 +3842,33 @@ provider = "test"
));
}
#[test]
fn provider_scoped_model_rejects_legacy_builtin_id_as_canonical_id() {
let error = Catalog::from_settings(&minimal_settings(
r#"
[providers.openrouter]
display_name = "OpenRouter"
adapter = "openai_compatible"
[providers.openrouter.models."openai/gpt-5.6-sol"]
"#,
))
.unwrap_err();
assert!(matches!(
error,
CatalogBuildError::LegacyModel(
LegacyModelError::LegacyIdentifierAsModelId {
identifier,
provider,
model,
}
) if identifier == "openai/gpt-5.6-sol"
&& provider == ProviderId::new("openrouter")
&& model == "gpt-5.6-sol"
));
}
#[test]
fn provider_aware_selection_uses_readiness_priority_and_api_ids() {
let catalog = portable_model_catalog();
@ -3856,6 +3922,36 @@ provider = "test"
));
}
#[test]
fn legacy_builtin_selector_uses_readiness_priority_and_explicit_pins() {
let catalog = portable_model_catalog();
let openai = ProviderId::openai();
let openrouter = ProviderId::new("openrouter");
let selector = "openai/gpt-5.6-sol";
for (eligible, expected_provider) in [
(HashSet::from([openai.clone()]), openai.clone()),
(HashSet::from([openrouter.clone()]), openrouter.clone()),
(
HashSet::from([openai.clone(), openrouter.clone()]),
openai.clone(),
),
] {
let selected = catalog
.resolve_selection(Some(selector), None, &eligible)
.unwrap();
assert_eq!(selected.provider, expected_provider);
assert_eq!(selected.model, "gpt-5.6-sol");
}
let both = HashSet::from([openai, openrouter.clone()]);
let selected = catalog
.resolve_selection(Some(selector), Some(&openrouter), &both)
.unwrap();
assert_eq!(selected.provider, openrouter);
assert_eq!(selected.model, "gpt-5.6-sol");
}
#[test]
fn equal_provider_priorities_use_canonical_provider_id_as_tie_breaker() {
let catalog = Catalog::from_settings(&minimal_settings(

3
lib/crates/fabro-server/src/server/handler/models.rs
View file

@ -270,8 +270,7 @@ fn model_selection_response(error: &ModelSelectionError) -> Response {
| ModelSelectionError::UnknownSelectorOnProvider { .. } => {
ApiError::not_found(error.to_string()).into_response()
}
ModelSelectionError::RetiredModelIdentifier { .. }
| ModelSelectionError::ProviderUnavailable { .. }
ModelSelectionError::ProviderUnavailable { .. }
| ModelSelectionError::NoEligibleOffering { .. }
| ModelSelectionError::NoDefaultModel { .. } => {
ApiError::bad_request(error.to_string()).into_response()

73
lib/crates/fabro-server/src/server/handler/sessions.rs
View file

@ -859,14 +859,11 @@ fn canonical_session_model(
if requested.is_empty() {
return Err(ApiError::bad_request("Session model must not be empty."));
}
if let Some((provider, model)) = catalog::retired_model_replacement(requested) {
return Err(session_selection_error(
&ModelSelectionError::RetiredModelIdentifier {
identifier: requested.to_string(),
provider,
model,
},
));
if catalog::legacy_builtin_model(requested).is_some() {
let selected = catalog
.resolve_selection(Some(requested), explicit_provider.as_ref(), eligible)
.map_err(|error| session_selection_error(&error))?;
return Ok((selected.provider, selected.model));
}
let model_ref = requested
.parse::<SettingsModelRef>()
@ -1715,13 +1712,63 @@ reasoning = false
}
#[test]
fn canonical_session_model_rejects_retired_wire_identifier_before_qualification() {
let catalog = Catalog::from_builtin().unwrap();
fn canonical_session_model_normalizes_legacy_builtin_selector_before_qualification() {
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("openai/gpt-5.6-sol"), None,).unwrap(),
(openai, "gpt-5.6-sol".to_string())
);
assert_eq!(
canonical_session_model(
&catalog,
&both,
Some("openai/gpt-5.6-sol"),
Some(&openrouter),
)
.unwrap(),
(openrouter.clone(), "gpt-5.6-sol".to_string())
);
assert_eq!(
canonical_session_model(
&catalog,
&std::collections::HashSet::from([openrouter.clone()]),
Some("openai/gpt-5.6-sol"),
None,
)
.unwrap(),
(openrouter, "gpt-5.6-sol".to_string())
);
}
#[test]
fn canonical_session_model_still_treats_non_legacy_qualified_model_as_a_pin() {
let catalog = portable_session_catalog();
let openrouter = ProviderId::new("openrouter");
assert_eq!(
canonical_session_model(
&catalog,
&catalog.all_provider_ids(),
Some("openrouter/gpt-56-sol"),
None,
)
.unwrap(),
(openrouter, "gpt-5.6-sol".to_string())
);
}
#[test]
fn canonical_session_model_rejects_conflicting_non_legacy_provider_pins() {
let catalog = portable_session_catalog();
let error = canonical_session_model(
&catalog,
&catalog.all_provider_ids(),
Some("openai/gpt-5.6-sol"),
None,
Some("openrouter/gpt-56-sol"),
Some(&ProviderId::openai()),
)
.unwrap_err();
@ -1730,7 +1777,7 @@ reasoning = false
error
.into_response_entry()
.detail
.contains("openrouter/gpt-5.6-sol")
.contains("conflicts with model reference provider")
);
}

42
lib/crates/fabro-workflow/src/operations/start.rs
View file

@ -7,7 +7,7 @@ use fabro_auth::{CredentialSource, EnvCredentialSource, VaultCredentialSource};
use fabro_interview::{AutoApproveInterviewer, Interviewer};
use fabro_llm::client::Client as LlmClient;
use fabro_mcp::config::McpServerSettings;
use fabro_model::{Catalog, FallbackTarget, ModelSelectionError, ProviderId, catalog};
use fabro_model::{Catalog, FallbackTarget, ModelSelectionError, ProviderId};
use fabro_sandbox::daytona::DaytonaConfig;
use fabro_sandbox::from_environment::{
daytona_config_from_environment, docker_config_from_environment_with_secrets,
@ -648,15 +648,6 @@ fn resolve_fallback_chain(
let mut chain = Vec::new();
for model_ref in &settings.fallbacks {
let identifier = model_ref.to_string();
if let Some((provider, model)) = catalog::retired_model_replacement(&identifier) {
return Err(ModelSelectionError::RetiredModelIdentifier {
identifier,
provider,
model,
}
.into());
}
match model_ref.resolve(&registry)? {
ResolvedModelRef::Provider(provider_name) => {
let provider_id = canonical_provider_id(catalog, &provider_name);
@ -1411,32 +1402,35 @@ reasoning = false
}
#[test]
fn resolve_fallback_chain_rejects_retired_wire_identifier_before_qualification() {
fn resolve_fallback_chain_keeps_qualified_legacy_references_as_provider_pins() {
let catalog = test_catalog();
let settings = ResolvedRunModelSettings {
fallbacks: vec!["openai/gpt-5.6-sol".parse::<ModelRef>().unwrap()],
fallbacks: vec![
"openai/gpt-5.6-sol".parse::<ModelRef>().unwrap(),
"anthropic/claude-fable-5".parse::<ModelRef>().unwrap(),
],
..ResolvedRunModelSettings::default()
};
let error = resolve_fallback_chain(
let chain = resolve_fallback_chain(
catalog.as_ref(),
&ProviderId::anthropic(),
"claude-opus-4-6",
&settings,
&catalog.all_provider_ids(),
)
.unwrap_err();
.unwrap();
assert!(matches!(
error,
Error::ModelSelection(ModelSelectionError::RetiredModelIdentifier {
identifier,
provider,
model,
}) if identifier == "openai/gpt-5.6-sol"
&& provider == ProviderId::new("openrouter")
&& model == fabro_model::ModelId::new("gpt-5.6-sol")
));
assert_eq!(chain, vec![
FallbackTarget {
provider: "openai".to_string(),
model: "gpt-5.6-sol".to_string(),
},
FallbackTarget {
provider: "anthropic".to_string(),
model: "claude-fable-5".to_string(),
},
]);
}
#[test]