Support provider-qualified model fallback selectors

[brynary]

Summary

• accept canonical provider:selector model references while retaining legacy provider/model input
• resolve provider-scoped canonical IDs, aliases, and API IDs, including API IDs containing slashes or additional colons
• reject ambiguous provider-local model identifiers during catalog construction
• canonicalize fallback targets and update session selection, OpenAPI, generated client, and public docs

Example

fallbacks = [
  "openrouter:kimi-k3",
  "gpt-terra",
]

openrouter:moonshotai/kimi-k3 is also accepted and resolves to the same canonical OpenRouter offering.

Testing

• cargo nextest run -p fabro-model -p fabro-config -p fabro-workflow — 1,666 passed
• cargo nextest run -p fabro-server — 770 passed
• cargo nextest run -p fabro-api — 202 passed
• affected-crate Clippy with warnings denied
• pinned rustfmt, generated-doc checks, API build, and TypeScript client typecheck

[Copilot]

Pull request overview

This PR updates Fabro’s model reference handling so fallback selectors can be provider-qualified using the canonical provider:selector syntax (while continuing to accept legacy provider/model inputs), and extends provider-scoped selection to support provider API IDs that may contain / and additional : characters. These changes propagate through catalog construction/lookup, workflow fallback resolution, server session selection, OpenAPI + generated client types, and public documentation.

Changes:

• Introduce canonical provider:selector parsing/serialization for ModelRef while retaining legacy provider/model input.
• Extend provider-scoped catalog lookup/indexing to include provider API IDs (and reject ambiguous provider-local identifiers during catalog build).
• Update server/session selection, workflow fallback resolution, OpenAPI schema, generated TypeScript client docs, and public docs to reflect the new selector format.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
lib/packages/fabro-api-client/src/models/create-run-session-request.ts	Updates generated client docs for model to describe provider:selector and legacy compatibility.
lib/foundation/fabro-types/src/settings/model_ref.rs	Implements provider:selector parsing + canonical display and updates tests (but needs a fix for legacy parsing with colons).
lib/foundation/fabro-model/src/catalog.rs	Indexes provider API IDs per offering, updates provider-scoped lookup semantics, and adds catalog build/lookup tests.
lib/foundation/fabro-dev/src/commands/docs_options_reference.rs	Updates docs generator example to use openrouter:kimi-k3 + gpt-terra.
lib/foundation/fabro-config/src/tests/combine.rs	Adjusts config layering tests to validate rendered fallback ordering with new model ref rendering.
lib/foundation/fabro-config/src/layers/run.rs	Updates fallbacks field documentation to describe provider:selector + legacy acceptance.
lib/components/fabro-workflow/src/operations/start.rs	Updates fallback-chain resolution to use selector and adds workflow-level tests for OpenRouter/Kimi + API ID canonicalization.
lib/apps/fabro-server/src/server/handler/sessions.rs	Updates session model canonicalization to accept colon-qualified selectors and adds tests for provider-scoped API IDs containing /.
docs/public/reference/user-configuration.mdx	Updates reference docs for fallbacks semantics and examples.
docs/public/execution/run-configuration.mdx	Expands run configuration docs with provider:selector details and examples (including selectors with / and :).
docs/public/execution/failures.mdx	Updates failure/fallback docs to describe provider:selector and provider API ID support.
docs/public/core-concepts/models.mdx	Updates models concept docs to describe the new fallback selector format and compatibility notes.
docs/public/api-reference/fabro-api.yaml	Updates schema descriptions and adds ModelRef schema documentation for new selector format + legacy acceptance.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[brynary]

Ran a simplification pass over the branch and pushed 8a41d46.

• build_model_indexes now takes the paired (Model, CatalogModelSettings) slice it is built from, instead of a second settings map. 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, instead of a nested then/flatten chain.
• ModelRef::from_str picks the separator first, then checks both sides once. The empty-side check is no longer duplicated across two branches, and the slash split no longer allocates a Vec.
• Shortened the TooManySlashes message to the action the user should take.
• Merged the two near-identical fallback chain tests into one that runs both qualified selector forms through the same assertion.
• The fallbacks splice test asserts through the existing Serialize impl instead of hand-rolling the ModelRefOrSplice rendering.

No behavior change. cargo build --workspace, cargo nextest run on fabro-model, fabro-types, fabro-config, fabro-workflow, fabro-server, and fabro-dev (2821 tests), fmt --check --all, and clippy on the touched crates all pass.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

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

• The serde JSON test no longer exercises the documented legacy provider/model input behavior. Since ModelRef accepts legacy slash-qualified references but serializes them to the canonical provider:selector form, it would be good to cover that canonicalization in this test to prevent regressions.

        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 selector }
                if provider == "openrouter" && selector == "moonshotai/kimi-k3"
        ));
        let rendered = serde_json::to_string(&parsed).unwrap();
        assert_eq!(rendered, input);

[brynary]

Follow-up: found a behavior regression on this branch while reviewing. Flagging rather than fixing, since the fix is a design call.

Bare model IDs containing : no longer pass through

ModelRef::from_str splits on the first : before any registry check, so a bare selector containing a colon becomes Qualified with a bogus provider.

Verified by running the same probe on both branches:

canonical_session_model(&catalog, &both, Some("future-model:latest"), Some(&openrouter))

branch	result
origin/main	Ok((openrouter, "future-model:latest"))
this branch	Err(400 "unknown model provider 'future-model'")

The passthrough it breaks is deliberate and tested — catalog.rs maps UnknownSelectorOnProvider to SelectedModel { provider, model: <verbatim selector> }, and canonical_session_model_preserves_unknown_passthrough_on_selected_provider pins it.

This matters because colon-bearing model IDs are normal:

• Ollama IDs are universally name:tag (ollama.toml documents api_id = "qwen3.5:latest" in a commented example).
• Five built-in bedrock api_ids end in :0, e.g. us.anthropic.claude-haiku-4-5-20251001-v1:0.
• Bedrock ARNs (arn:aws:bedrock:...) name custom inference profiles.

Same shape hits run.model.fallbacks: a colon-bearing bare entry resolves canonical_provider_id to an unknown ID and fails run start with ProviderUnavailable, where it previously pushed a passthrough target.

Note the qualified form is fine and is tested — bedrock:us.anthropic.claude-haiku-4-5-20251001-v1:0 works, because the split takes only the first colon. Only the bare form regresses.

Two ways out:

1. Keep FromStr producing Bare for colon strings and do the split inside resolve(), guarded by registry.is_provider(prefix). This matches the module header, which already says ambiguity resolution against a known registry happens at consumption time.
2. Keep the parse, but at the two consumption sites treat an unknown provider on a Qualified ref as a bare selector of provider:selector rather than erroring.

Either way worth a test pinning bare "llama3:8b" with an ollama pin.

Also pushed b294251: the duplicate-api_id catalog test was a copy of the duplicate-alias one, so they are now a single table-driven test.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

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

• ModelRef parsing currently always prefers : when present. Inputs like openai/gpt-5.6-sol:0 (legacy slash-qualified with a : in the selector) will be mis-parsed as provider=openai/gpt-5.6-sol and selector=0, which is a backwards-compatibility regression and yields confusing downstream errors. Prefer whichever separator (: or /) appears first, and only treat : as the qualifier when it precedes any /.

        // 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,

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

• The new parsing rules around : vs legacy / separators would benefit from a regression test that covers a legacy slash-qualified selector containing : (e.g. openai/gpt-5.6-sol:0). This ensures the parser chooses the first separator and that serialization still canonicalizes to provider:selector.

    fn colon_qualified_selector_may_contain_slashes_and_colons() {
        assert_eq!(
            "openrouter:moonshotai/kimi-k3".parse::<ModelRef>().unwrap(),
            ModelRef::Qualified {
                provider: "openrouter".into(),

[brynary]

Fixed in 2103e3f — went with the registry-guarded split.

FromStr no longer splits on :. Colon-bearing tokens stay Bare, and a new 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.

Reasoning for that approach over patching the call sites: no separator is safe to split on by shape. Bedrock inference-profile ARNs contain both : and /, and docs/public/integrations/bedrock.mdx tells users to put arbitrary inference-profile ids in api_id. Slash was already unsafe — that is why this PR added the colon form — and colon is unsafe for the same reason one layer down. Only the registry can tell a provider prefix from a model ID that contains the separator. It also keeps the type honest: Qualified now only exists when a provider was confirmed.

Same probe as before, now on the fix:

case	result
bare future-model:latest, pinned openrouter	Ok((openrouter, "future-model:latest"))
bare future-model:latest, unpinned	Ok((openai, "future-model:latest"))
openrouter:gpt-56-sol	Ok((openrouter, "gpt-5.6-sol"))
openrouter:openai/gpt-5.6-sol	Ok((openrouter, "gpt-5.6-sol"))

The first row matches origin/main again. Both qualified forms still resolve.

Also in this commit:

• ModelRegistry is implemented for Catalog in fabro-types, replacing the CatalogModelRegistry wrapper that existed only in start.rs, so both call sites share one registry view.
• Regression tests at both surfaces, plus qualify unit tests covering ollama name:tag and a Bedrock ARN. canonical_session_model_preserves_unknown_passthrough_on_selected_provider — which this PR had been breaking — passes again.
• Docs and the OpenAPI descriptions now state the rule, with "llama3:8b" vs "ollama:llama3:8b" as the worked example.

Full workspace suite green: 7408 tests, plus fmt --check --all and workspace clippy.

Still open from the review, not addressed here: run.model.name, node model=, and POST /completions parse raw &str, so provider:selector silently passes through on those three surfaces; and ModelRef is absent from the fabro-api with_replacement table, so the generated type is a validation-free String newtype.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

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

• get_on_provider does a second lookup using normalize_legacy_builtin_selector(selector) even when normalization is a no-op (the common case for non-legacy selectors). That repeats the same HashMap lookup/allocation work when the selector is simply unknown. Consider only attempting the normalized lookup when the normalized value differs from the original selector.

        let index =
            lookup(selector).or_else(|| lookup(&normalize_legacy_builtin_selector(selector)))?;

[brynary]

Two follow-ups from this round.

Declined: the get_on_provider double lookup

Copilot flagged (low confidence) that get_on_provider attempts the normalized lookup even when normalization is a no-op, and suggested restoring the normalized != selector guard. Not taking it.

The guard would save one hash lookup on an identical key, but normalize_legacy_builtin_selector is not free — it linear-scans the 45-entry LEGACY_BUILTIN_MODEL_IDENTIFIERS table and allocates a ProviderId it discards. The old code ran that scan unconditionally on every call. Trying the exact hash lookup first skips the scan entirely on every hit, which is the dominant case. Net, the current form does strictly less work on hits and about the same on misses, for simpler code. Every caller is once-per-request or once-per-response next to a network round trip.

Unrelated: this branch makes all_spec_routes_are_routable ~40x slower

Found while verifying, not caused by my commits — it reproduces at 33b94d8, the original PR commit.

Same directory, same Docker state, back to back:

	result
origin/main	passes in 0.5–0.8s
this branch	14–20s, intermittently hitting nextest's 20s timeout and failing the run

The cause is GET /api/v1/system/df and GET /api/v1/system/resources. Probing get_system_df:

	storage_dir	build_disk_usage_response
origin/main	/var/folders/.../T/fabro-test-<ULID>/storage	161µs
this branch	/Users/<me>/.fabro/storage	39.8s

So on this branch the test escapes its temp dir and walks the developer's real ~/.fabro/storage — 194MB here. That is worth fixing beyond the speed: server tests reading real user storage is a test-isolation problem, and it makes the timing machine-dependent, which is probably why CI has not flagged it.

server_storage_dir() is server_settings().server.storage.root, whose default is Home::from_env().root().join("storage"). On main something is pointing that at a temp root for this test and on the branch it is not. I did not root-cause which change flips it — none of the 13 changed files touches storage or config resolution, so it is likely indirect. Reproduce with:

cargo test -p fabro-server --features test-support --test it -- \
  openapi_conformance::all_spec_routes_are_routable --nocapture

Leaving this one to you rather than guessing at a fix.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

docs/public/reference/user-configuration.mdx:389

• The public docs reference super::splice_array, which is a Rust module path and doesn’t resolve in the Mintlify docs. Consider removing the link and describing the splice marker in plain language.

| `fallbacks` | array<string> | [] | Ordered fallback references: bare providers, bare model IDs or aliases,<br />or provider-qualified `provider:selector` values. A qualified selector<br />may be a model ID, alias, or provider API ID. Legacy `provider/model`<br />values remain accepted. Supports the `...` splice marker at layering<br />time — see [`super::splice_array`]. |