From 27cf30ff8b4ccf68032a2be86e78b268c6ad8ce3 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 06:33:22 -0400 Subject: [PATCH 01/11] feat: add GPT-5.6 name aliases --- lib/foundation/fabro-model/src/catalog.rs | 3 +++ .../fabro-model/src/catalog/providers/openai.toml | 6 +++--- .../fabro-model/src/catalog/providers/openrouter.toml | 6 +++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index cac9f878f..f7c42f5ba 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -3118,8 +3118,11 @@ enabled = true for provider in [ProviderId::openai(), ProviderId::new("openrouter")] { for (alias, canonical_id) in [ ("sol", "gpt-5.6-sol"), + ("gpt-sol", "gpt-5.6-sol"), ("terra", "gpt-5.6-terra"), + ("gpt-terra", "gpt-5.6-terra"), ("luna", "gpt-5.6-luna"), + ("gpt-luna", "gpt-5.6-luna"), ] { let model = catalog .resolve_on_provider(&provider, alias) diff --git a/lib/foundation/fabro-model/src/catalog/providers/openai.toml b/lib/foundation/fabro-model/src/catalog/providers/openai.toml index 10b7b2193..6e44cd97e 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/openai.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/openai.toml @@ -14,7 +14,7 @@ family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" default = true -aliases = ["sol", "gpt56-sol", "gpt-56-sol", "gpt-5.6", "gpt56", "gpt-56"] +aliases = ["sol", "gpt-sol", "gpt56-sol", "gpt-56-sol", "gpt-5.6", "gpt56", "gpt-56"] [providers.openai.models."gpt-5.6-sol".limits] context_window = 272000 @@ -37,7 +37,7 @@ display_name = "GPT-5.6 Terra" family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" -aliases = ["terra", "gpt56-terra", "gpt-56-terra"] +aliases = ["terra", "gpt-terra", "gpt56-terra", "gpt-56-terra"] [providers.openai.models."gpt-5.6-terra".limits] context_window = 272000 @@ -60,7 +60,7 @@ display_name = "GPT-5.6 Luna" family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" -aliases = ["luna", "gpt56-luna", "gpt-56-luna"] +aliases = ["luna", "gpt-luna", "gpt56-luna", "gpt-56-luna"] [providers.openai.models."gpt-5.6-luna".limits] context_window = 272000 diff --git a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml index b83aa7662..201608654 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml @@ -167,7 +167,7 @@ display_name = "GPT-5.6 Sol (via OpenRouter)" family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" -aliases = ["sol", "gpt56-sol", "gpt-56-sol", "gpt-5.6", "gpt56", "gpt-56"] +aliases = ["sol", "gpt-sol", "gpt56-sol", "gpt-56-sol", "gpt-5.6", "gpt56", "gpt-56"] [providers.openrouter.models."gpt-5.6-sol".limits] context_window = 1050000 @@ -192,7 +192,7 @@ display_name = "GPT-5.6 Terra (via OpenRouter)" family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" -aliases = ["terra", "gpt56-terra", "gpt-56-terra"] +aliases = ["terra", "gpt-terra", "gpt56-terra", "gpt-56-terra"] [providers.openrouter.models."gpt-5.6-terra".limits] context_window = 1050000 @@ -217,7 +217,7 @@ display_name = "GPT-5.6 Luna (via OpenRouter)" family = "gpt-5" training = "2026-02-16" knowledge_cutoff = "February 16, 2026" -aliases = ["luna", "gpt56-luna", "gpt-56-luna"] +aliases = ["luna", "gpt-luna", "gpt56-luna", "gpt-56-luna"] [providers.openrouter.models."gpt-5.6-luna".limits] context_window = 1050000 From 673a7064fe2ad3e1d72c448691cc57f4bbf70a77 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:04:31 -0400 Subject: [PATCH 02/11] Validate completion reasoning effort --- docs/public/api-reference/fabro-api.yaml | 2 +- .../src/server/handler/completions.rs | 2 +- lib/apps/fabro-server/src/server/tests.rs | 21 ++++++++++++++++ .../create_completion_request_round_trip.rs | 25 +++++++++++++++++++ .../src/models/create-completion-request.ts | 5 +++- 5 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 519ee4188..e13097f1b 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -8498,7 +8498,7 @@ components: type: string description: Stop sequences. reasoning_effort: - type: string + $ref: "#/components/schemas/ReasoningEffort" description: Reasoning effort level. provider: type: string diff --git a/lib/apps/fabro-server/src/server/handler/completions.rs b/lib/apps/fabro-server/src/server/handler/completions.rs index b53e56d31..7afe6e943 100644 --- a/lib/apps/fabro-server/src/server/handler/completions.rs +++ b/lib/apps/fabro-server/src/server/handler/completions.rs @@ -109,7 +109,7 @@ async fn create_completion( } else { Some(req.stop_sequences) }, - reasoning_effort: req.reasoning_effort.as_deref().and_then(|s| s.parse().ok()), + reasoning_effort: req.reasoning_effort, speed: None, metadata: None, provider_options: req.provider_options, diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 454455be0..641ea0b7b 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -15240,6 +15240,27 @@ async fn create_completion_missing_messages_returns_422() { assert_status!(response, StatusCode::UNPROCESSABLE_ENTITY).await; } +#[tokio::test] +async fn create_completion_invalid_reasoning_effort_returns_422() { + let app = test_app_with(); + + let req = Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ + "messages": [], + "reasoning_effort": "bogus" + }) + .to_string(), + )) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + assert_status!(response, StatusCode::UNPROCESSABLE_ENTITY).await; +} + #[tokio::test] async fn create_completion_unknown_provider_returns_clear_error() { let app = test_app_with(); diff --git a/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs b/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs new file mode 100644 index 000000000..913947453 --- /dev/null +++ b/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs @@ -0,0 +1,25 @@ +use fabro_api::types::CreateCompletionRequest; +use fabro_model::ReasoningEffort; +use serde_json::json; + +#[test] +fn create_completion_request_reuses_canonical_reasoning_effort() { + let request: CreateCompletionRequest = serde_json::from_value(json!({ + "messages": [], + "reasoning_effort": "high" + })) + .unwrap(); + + let reasoning_effort: Option = request.reasoning_effort; + assert_eq!(reasoning_effort, Some(ReasoningEffort::High)); +} + +#[test] +fn create_completion_request_rejects_unknown_reasoning_effort() { + let result = serde_json::from_value::(json!({ + "messages": [], + "reasoning_effort": "bogus" + })); + + assert!(result.is_err()); +} diff --git a/lib/packages/fabro-api-client/src/models/create-completion-request.ts b/lib/packages/fabro-api-client/src/models/create-completion-request.ts index 923431e9b..836e20c0d 100644 --- a/lib/packages/fabro-api-client/src/models/create-completion-request.ts +++ b/lib/packages/fabro-api-client/src/models/create-completion-request.ts @@ -22,6 +22,9 @@ import type { CompletionToolChoice } from './completion-tool-choice'; // May contain unused imports in some cases // @ts-ignore import type { CompletionToolDefinition } from './completion-tool-definition'; +// May contain unused imports in some cases +// @ts-ignore +import type { ReasoningEffort } from './reasoning-effort'; export interface CreateCompletionRequest { /** @@ -56,7 +59,7 @@ export interface CreateCompletionRequest { /** * Reasoning effort level. */ - 'reasoning_effort'?: string; + 'reasoning_effort'?: ReasoningEffort; /** * Optional provider pin. */ From 05fa48563792d1fb53af4f226c5cae8330e11502 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:07:56 -0400 Subject: [PATCH 03/11] feat: add portable GLM and DeepSeek aliases --- docs/public/core-concepts/models.mdx | 4 +- docs/public/integrations/openrouter.mdx | 5 +- lib/foundation/fabro-model/src/catalog.rs | 62 ++++++++++++++++++- .../src/catalog/providers/openrouter.toml | 3 + .../src/catalog/providers/zai.toml | 2 +- 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/docs/public/core-concepts/models.mdx b/docs/public/core-concepts/models.mdx index ab88318dd..e5eb837b3 100644 --- a/docs/public/core-concepts/models.mdx +++ b/docs/public/core-concepts/models.mdx @@ -59,7 +59,7 @@ Fabro performs this selection once when creating a run and persists the chosen p | `kimi-k2.5` | kimi | `kimi` | 262K | $0.60 / $3.00 | 50 tok/s | | `laguna-s-2.1` | poolside | `laguna`, `laguna-s` | 1M | $0.10 / $0.20 | n/a | | `laguna-xs-2.1` | poolside | `laguna-xs` | 262K | $0.10 / $0.20 | n/a | -| `glm-4.7` | zai | `glm`, `glm4` | 203K | $0.60 / $2.20 | 100 tok/s | +| `glm-5.2` | zai | `glm`, `glm5`, `glm52`, `glm5.2` | 1M | $1.40 / $4.40 | n/a | | `minimax-m2.5` | minimax | `minimax` | 197K | $0.30 / $1.20 | 45 tok/s | | `mercury-2` | inception | `mercury` | 131K | $0.25 / $0.75 | 1000 tok/s | @@ -206,7 +206,7 @@ When no model or provider is specified, Fabro chooses the default offering on th | `gemini` | `gemini-3.5-flash` | | `kimi` | `kimi-k2.5` | | `poolside` | `laguna-s-2.1` | -| `zai` | `glm-4.7` | +| `zai` | `glm-5.2` | | `minimax` | `minimax-m2.5` | | `inception` | `mercury-2` | diff --git a/docs/public/integrations/openrouter.mdx b/docs/public/integrations/openrouter.mdx index 6f1f046b5..c725ddca6 100644 --- a/docs/public/integrations/openrouter.mdx +++ b/docs/public/integrations/openrouter.mdx @@ -53,10 +53,11 @@ The built-in catalog gives OpenRouter offerings the same human-facing model slug | `claude-haiku-4-5` | `anthropic/claude-haiku-4.5`; provider small default | | `gpt-5.4`, `gpt-5.5` | `openai/gpt-5.4`, `openai/gpt-5.5` | | `gemini-3.1-pro-preview`, `gemini-3.5-flash` | `google/...` API IDs | -| `deepseek-v4-pro`, `deepseek-v4-flash` | `deepseek/...` API IDs | +| `deepseek-v4-pro` (`deepseek`, `deepseek-v4`), `deepseek-v4-flash` (`deepseek-flash`) | `deepseek/...` API IDs | | `kimi-k2.6`, `qwen3-coder`, `qwen3.6-flash` | Vendor-prefixed API IDs | | `laguna-s-2.1`, `laguna-xs-2.1` | `poolside/...`; native reasoning and tool use | -| `glm-4.6`, `minimax-m2.7`, `mimo-v2.5-pro` | Vendor-prefixed API IDs | +| `glm-5.2` (`glm`, `glm5`, `glm52`, `glm5.2`), `glm-4.6` | `z-ai/...` API IDs | +| `minimax-m2.7`, `mimo-v2.5-pro` | Vendor-prefixed API IDs | | `nemotron-3-super-120b-a12b`, `devstral-2512` | Vendor-prefixed API IDs | Any other OpenRouter model can be added under the provider. Choose a stable Fabro model slug as the table key and put OpenRouter's exact vendor/model string in `api_id`: diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index f7c42f5ba..c58035f8d 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -3135,6 +3135,57 @@ enabled = true } } + #[test] + fn builtin_glm_5_2_aliases_are_portable() { + 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"); + + for provider in [ProviderId::new("zai"), ProviderId::new("openrouter")] { + for alias in ["glm", "glm5", "glm52", "glm5.2"] { + let model = catalog + .resolve_on_provider(&provider, alias) + .unwrap_or_else(|error| { + panic!("{alias} should resolve on {provider}: {error}") + }); + assert_eq!(model.provider, provider, "{alias}"); + assert_eq!(model.id, "glm-5.2", "{alias}"); + } + } + } + + #[test] + fn builtin_deepseek_v4_selectors_resolve_on_openrouter() { + 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 [ + ("deepseek-v4-pro", "deepseek-v4-pro"), + ("deepseek-v4", "deepseek-v4-pro"), + ("deepseek", "deepseek-v4-pro"), + ("deepseek-v4-flash", "deepseek-v4-flash"), + ("deepseek-flash", "deepseek-v4-flash"), + ] { + 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}"); + } + } + #[test] fn builtin_legacy_vendor_ids_normalize_for_pinned_and_unpinned_selection() { let catalog = Catalog::from_builtin_with_overrides(&minimal_settings( @@ -3260,7 +3311,12 @@ enabled = true ), }, estimated_output_tps: None, - aliases: [], + aliases: [ + "glm", + "glm5", + "glm52", + "glm5.2", + ], default: false, small_default: false, configured: false, @@ -6107,6 +6163,8 @@ sampling_params = false aliases: [ "glm", "glm5", + "glm52", + "glm5.2", ], default: true, small_default: false, @@ -6124,6 +6182,8 @@ sampling_params = false ]); assert_eq!(catalog.get("glm").unwrap().id, "glm-5.2"); assert_eq!(catalog.get("glm5").unwrap().id, "glm-5.2"); + assert_eq!(catalog.get("glm52").unwrap().id, "glm-5.2"); + assert_eq!(catalog.get("glm5.2").unwrap().id, "glm-5.2"); } #[test] diff --git a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml index 201608654..175d76cad 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml @@ -354,6 +354,7 @@ output_cost_per_mtok = 1.20 api_id = "deepseek/deepseek-v4-pro" display_name = "DeepSeek V4 Pro" family = "deepseek-v4" +aliases = ["deepseek-v4", "deepseek"] [providers.openrouter.models."deepseek-v4-pro".limits] context_window = 1050000 @@ -372,6 +373,7 @@ output_cost_per_mtok = 0.87 api_id = "deepseek/deepseek-v4-flash" display_name = "DeepSeek V4 Flash" family = "deepseek-v4" +aliases = ["deepseek-flash"] [providers.openrouter.models."deepseek-v4-flash".limits] context_window = 1050000 @@ -513,6 +515,7 @@ output_cost_per_mtok = 1.125 api_id = "z-ai/glm-5.2" display_name = "GLM 5.2 (via OpenRouter)" family = "glm-5" +aliases = ["glm", "glm5", "glm52", "glm5.2"] [providers.openrouter.models."glm-5.2".limits] context_window = 1048576 diff --git a/lib/foundation/fabro-model/src/catalog/providers/zai.toml b/lib/foundation/fabro-model/src/catalog/providers/zai.toml index c6d70cd64..58fb8c38a 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/zai.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/zai.toml @@ -12,7 +12,7 @@ credentials = ["env:ZAI_API_KEY", "vault:ZAI_API_KEY"] display_name = "GLM 5.2" family = "glm-5" default = true -aliases = ["glm", "glm5"] +aliases = ["glm", "glm5", "glm52", "glm5.2"] [providers.zai.models."glm-5.2".limits] context_window = 1048576 From 3cfac20343b1b73a79d31e64b830d532dad604a5 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:12:01 -0400 Subject: [PATCH 04/11] test: capture preflight provider routing gap --- lib/apps/fabro-server/src/run_manifest.rs | 149 ++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 259e3d2b5..375ef57e5 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1470,6 +1470,81 @@ mod tests { Arc::new(Catalog::from_builtin().unwrap()) } + fn openai_compatible_completion(model: &str) -> serde_json::Value { + serde_json::json!({ + "id": "chatcmpl_preflight", + "object": "chat.completion", + "created": 1_700_000_000, + "model": model, + "choices": [{ + "index": 0, + "message": {"role": "assistant", "content": "OK"}, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2} + }) + } + + fn ready_kimi_and_openrouter_state( + server: &httpmock::MockServer, + ) -> Arc { + let kimi_url = server.url("/kimi/v1"); + let openrouter_url = server.url("/openrouter/v1"); + let llm_catalog_settings: LlmCatalogSettings = toml::from_str(&format!( + r#" +[providers.kimi] +base_url = "{kimi_url}" + +[providers.openrouter] +base_url = "{openrouter_url}" +enabled = true +"# + )) + .expect("catalog overrides should parse"); + + crate::test_support::TestAppStateBuilder::new() + .llm_catalog_settings(llm_catalog_settings) + .vault_entries([ + (EnvVars::KIMI_API_KEY, "test-kimi-key"), + (EnvVars::OPENROUTER_API_KEY, "test-openrouter-key"), + ]) + .build() + } + + async fn preflight_for_model( + state: &Arc, + model: &str, + ) -> (types::PreflightResponse, bool) { + let mut ready_providers = state.ready_llm_provider_ids().await; + ready_providers.sort(); + assert_eq!(ready_providers, vec![ + ProviderId::new("kimi"), + ProviderId::new("openrouter") + ]); + + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().source = format!( + r#" +digraph Demo {{ + start [shape=Mdiamond] + exit [shape=Msquare] + work [prompt="Do work", model="{model}"] + start -> work -> exit +}} +"# + ); + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + let validated = validate_prepared_manifest(&prepared, state.catalog()).unwrap(); + + run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap() + } + fn manifest_workflow() -> types::ManifestWorkflow { types::ManifestWorkflow { config: None, @@ -2390,6 +2465,80 @@ digraph Demo { assert!(response_mock.calls_async().await >= 1); } + #[tokio::test] + async fn preflight_uses_ready_providers_for_known_shared_alias() { + let server = httpmock::MockServer::start_async().await; + let openrouter_probe = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/openrouter/v1/chat/completions") + .header("authorization", "Bearer test-openrouter-key") + .json_body_includes(r#"{"model":"anthropic/claude-fable-5"}"#); + then.status(200) + .header("content-type", "application/json") + .json_body(openai_compatible_completion("anthropic/claude-fable-5")); + }) + .await; + let state = ready_kimi_and_openrouter_state(&server); + + let (response, _ok) = preflight_for_model(&state, "claude-fable").await; + + let llm_check = response.checks.sections[0] + .checks + .iter() + .find(|check| check.name == "LLM" && check.summary == "claude-fable-5") + .expect("preflight should include Claude Fable"); + assert_eq!( + llm_check + .details + .iter() + .map(|detail| detail.text.as_str()) + .find(|detail| detail.starts_with("Provider: ")), + Some("Provider: openrouter") + ); + assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Pass); + openrouter_probe.assert_async().await; + } + + #[tokio::test] + async fn preflight_uses_ready_providers_for_unknown_unqualified_model() { + let server = httpmock::MockServer::start_async().await; + let kimi_probe = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/kimi/v1/chat/completions") + .header("authorization", "Bearer test-kimi-key") + .json_body_includes(r#"{"model":"provider-private-preview"}"#); + then.status(200) + .header("content-type", "application/json") + .json_body(openai_compatible_completion("provider-private-preview")); + }) + .await; + let state = ready_kimi_and_openrouter_state(&server); + + let (response, _ok) = preflight_for_model(&state, "provider-private-preview").await; + + assert!(response.workflow.diagnostics.iter().any(|diagnostic| { + diagnostic.rule == "node_model_known" + && diagnostic.message.contains("provider-private-preview") + })); + let llm_check = response.checks.sections[0] + .checks + .iter() + .find(|check| check.name == "LLM" && check.summary == "provider-private-preview") + .expect("preflight should include the unknown passthrough model"); + assert_eq!( + llm_check + .details + .iter() + .map(|detail| detail.text.as_str()) + .find(|detail| detail.starts_with("Provider: ")), + Some("Provider: kimi") + ); + assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Pass); + kimi_probe.assert_async().await; + } + #[test] fn static_validation_rejects_unknown_llm_provider() { let mut manifest = minimal_manifest(); From 377eb961ec805dfba15317a06fe06d0d2f4534ea Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:26:22 -0400 Subject: [PATCH 05/11] feat: add Poolside provider logo Adapted from poolside's official favicon mark: monochrome fill="currentColor" at 24x24 to match the other provider logos, with the brand's gradient-fade tail preserved via the original alpha mask. Co-Authored-By: Claude Fable 5 --- .../public/images/providers/poolside.svg | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 apps/fabro-web/public/images/providers/poolside.svg diff --git a/apps/fabro-web/public/images/providers/poolside.svg b/apps/fabro-web/public/images/providers/poolside.svg new file mode 100644 index 000000000..8c160c5ad --- /dev/null +++ b/apps/fabro-web/public/images/providers/poolside.svg @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + From 0cc4a018824f1c4bae06333ed5e5a659eaf7e307 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:28:06 -0400 Subject: [PATCH 06/11] Forward reasoning effort for structured completions --- .../src/server/handler/completions.rs | 3 + lib/apps/fabro-server/src/server/tests.rs | 66 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/lib/apps/fabro-server/src/server/handler/completions.rs b/lib/apps/fabro-server/src/server/handler/completions.rs index b53e56d31..78a973087 100644 --- a/lib/apps/fabro-server/src/server/handler/completions.rs +++ b/lib/apps/fabro-server/src/server/handler/completions.rs @@ -155,6 +155,9 @@ async fn create_completion( if let Some(top_p) = request.top_p { params = params.top_p(top_p); } + if let Some(reasoning_effort) = request.reasoning_effort { + params = params.reasoning_effort(reasoning_effort); + } match generate_object(params, schema).await { Ok(result) => { // `result.finish_reason` / `result.usage` resolve through diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 454455be0..c4d5cac28 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -15347,6 +15347,72 @@ reasoning = false completion.assert(); } +#[tokio::test] +async fn create_completion_structured_output_forwards_reasoning_effort() { + let upstream = MockServer::start(); + let completion = upstream.mock(|when, then| { + when.method(POST) + .path("/chat/completions") + .json_body_includes(r#"{"model":"kimi-k3","reasoning_effort":"high"}"#); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ + "id": "chatcmpl-kimi-structured", + "model": "kimi-k3", + "choices": [{ + "message": { + "role": "assistant", + "content": "{\"answer\":42}" + }, + "finish_reason": "stop" + }], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 4, + "total_tokens": 14 + } + })); + }); + let state = TestAppStateBuilder::new() + .provider_base_url("kimi", upstream.base_url()) + .vault_entries([(EnvVars::KIMI_API_KEY, "test-kimi-api-key")]) + .build(); + let app = crate::test_support::build_test_router(state); + + let req = Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ + "provider": "kimi", + "model": "kimi-k3", + "reasoning_effort": "high", + "stream": false, + "schema": { + "type": "object", + "properties": { + "answer": {"type": "integer"} + }, + "required": ["answer"] + }, + "messages": [ + { + "role": "user", + "content": [{"kind": "text", "data": "Return the answer."}] + } + ] + }) + .to_string(), + )) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::OK).await; + assert_eq!(body["output"], json!({"answer": 42})); + completion.assert_calls(1); +} + #[tokio::test] async fn demo_list_runs_returns_run_list_items() { let state = test_app_state(); From 1c1ea53093cced949bf9c49c1d547b9bd60b132d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:28:51 -0400 Subject: [PATCH 07/11] fix: prefer ready providers during preflight --- lib/apps/fabro-server/src/run_manifest.rs | 192 ++++++++++++------ .../fabro-server/src/server/handler/runs.rs | 24 ++- .../fabro-workflow/src/operations/create.rs | 6 + .../fabro-workflow/src/operations/mod.rs | 2 +- .../fabro-workflow/src/operations/validate.rs | 33 ++- .../fabro-workflow/src/pipeline/transform.rs | 5 + .../fabro-workflow/src/pipeline/types.rs | 1 + .../fabro-workflow/src/pipeline/validate.rs | 1 + .../fabro-workflow/src/run_materialization.rs | 47 ++++- .../src/transforms/model_resolution.rs | 74 ++++++- .../fabro-workflow/tests/it/integration.rs | 1 + lib/foundation/fabro-model/src/catalog.rs | 61 ++++++ 12 files changed, 365 insertions(+), 82 deletions(-) diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 375ef57e5..aff59b0c5 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -34,14 +34,17 @@ use fabro_types::{ use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; use fabro_validate::Severity; use fabro_workflow::Error as WorkflowError; -use fabro_workflow::operations::{CreateRunInput, ValidateInput, WorkflowInput, validate}; +use fabro_workflow::operations::{ + CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_provider_fallback, +}; use fabro_workflow::pipeline::Validated; +#[cfg(test)] use fabro_workflow::run_materialization::materialize_run; +use fabro_workflow::run_materialization::materialize_run_with_provider_fallback; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; use futures_util::stream::{self, StreamExt}; use tokio::process::Command; use tokio::time; -use tracing::warn; use crate::interp::process_env_var; use crate::server::AppState; @@ -206,6 +209,27 @@ pub(crate) fn validate_prepared_manifest_with_vars( }) } +pub(crate) fn validate_prepared_manifest_for_preflight( + prepared: &PreparedManifest, + catalog: Arc, + vars: HashMap, + ready_providers: &[ProviderId], +) -> Result { + let fallback_providers = catalog.all_provider_ids().into_iter().collect::>(); + validate_with_provider_fallback( + ValidateInput { + workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), + settings: prepared.settings.clone(), + vars, + cwd: prepared.cwd.clone(), + custom_transforms: Vec::new(), + catalog, + }, + ready_providers, + &fallback_providers, + ) +} + pub(crate) fn create_run_input( prepared: PreparedManifest, configured_providers: Vec, @@ -238,8 +262,11 @@ pub(crate) async fn run_preflight( state: &AppState, prepared: &PreparedManifest, validated: &Validated, + preferred_providers: &[ProviderId], + llm_result: Result, ) -> Result<(types::PreflightResponse, bool)> { - let (report, checks_ok) = build_preflight_report(state, prepared, validated).await?; + let (report, checks_ok) = + build_preflight_report(state, prepared, validated, preferred_providers, llm_result).await?; let preflight_ok = !validated.has_errors() && checks_ok; Ok(( preflight_response( @@ -458,6 +485,8 @@ async fn build_preflight_report( state: &AppState, prepared: &PreparedManifest, validated: &Validated, + preferred_providers: &[ProviderId], + llm_result: Result, ) -> Result<(CheckReport, bool)> { let graph = validated.graph(); let mut checks = base_preflight_checks(prepared, graph); @@ -475,20 +504,13 @@ async fn build_preflight_report( } let catalog = state.catalog(); - let llm_result = state.resolve_llm_client().await; - if let Err(err) = &llm_result { - warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); - } - // Preflight is credential-independent static validation. Materialize - // against every enabled catalog provider so aliases and defaults can be - // inspected even when the corresponding adapter is not currently ready; - // `run_llm_check` below reports actual credential/registration readiness. - let enabled_providers = catalog.all_provider_ids().into_iter().collect::>(); - let materialized = materialize_run( + let fallback_providers = catalog.all_provider_ids().into_iter().collect::>(); + let materialized = materialize_run_with_provider_fallback( prepared.settings.clone(), graph, catalog.as_ref(), - &enabled_providers, + preferred_providers, + &fallback_providers, )?; let resolved_run = materialized.run; let server_settings = state.server_settings(); @@ -1033,13 +1055,21 @@ async fn run_llm_check( catalog: &Catalog, llm_result: Result, ) -> bool { - let model = settings - .model - .name - .as_deref() - .unwrap_or_else(|| catalog.default_for_configured_ids(&[]).id.as_str()); - let provider = settings.model.provider.as_deref(); - let default_provider = provider.unwrap_or("anthropic"); + let (Some(model), Some(default_provider)) = ( + settings.model.name.as_deref(), + settings.model.provider.as_deref(), + ) else { + checks.push(CheckResult { + name: "LLM".into(), + status: CheckStatus::Error, + summary: "model resolution failed".into(), + details: Vec::new(), + remediation: Some( + "Preflight did not produce a resolved run model and provider".to_string(), + ), + }); + return false; + }; let mut model_providers = std::collections::BTreeSet::new(); let mut has_llm_nodes = false; @@ -1050,24 +1080,7 @@ async fn run_llm_check( has_llm_nodes = true; let node_model = node.model().unwrap_or(model); let node_provider = node.provider().unwrap_or(default_provider); - let resolved = if node.provider().is_some() { - catalog.get_on_provider(&ProviderId::new(node_provider), node_model) - } else { - catalog - .select(node_model, None, &catalog.all_provider_ids()) - .ok() - }; - let (resolved_model, resolved_provider) = if let Some(info) = resolved { - (info.id.to_string(), info.provider.to_string()) - } else { - (node_model.to_string(), node_provider.to_string()) - }; - let final_provider = if node.provider().is_some() { - node_provider.to_string() - } else { - resolved_provider - }; - model_providers.insert((resolved_model, final_provider)); + model_providers.insert((node_model.to_string(), node_provider.to_string())); } if !has_llm_nodes { @@ -1515,7 +1528,11 @@ enabled = true state: &Arc, model: &str, ) -> (types::PreflightResponse, bool) { - let mut ready_providers = state.ready_llm_provider_ids().await; + let llm_result = state.resolve_llm_client().await; + let mut ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); ready_providers.sort(); assert_eq!(ready_providers, vec![ ProviderId::new("kimi"), @@ -1538,11 +1555,37 @@ digraph Demo {{ &manifest, ) .unwrap(); - let validated = validate_prepared_manifest(&prepared, state.catalog()).unwrap(); + let validated = validate_prepared_manifest_for_preflight( + &prepared, + state.catalog(), + HashMap::new(), + &ready_providers, + ) + .unwrap(); - run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap() + run_preflight( + state.as_ref(), + &prepared, + &validated, + &ready_providers, + llm_result, + ) + .await + .unwrap() + } + + async fn run_preflight_with_catalog_routes( + state: &AppState, + prepared: &PreparedManifest, + validated: &Validated, + ) -> Result<(types::PreflightResponse, bool)> { + let llm_result = state.resolve_llm_client().await; + let preferred_providers = state + .catalog() + .all_provider_ids() + .into_iter() + .collect::>(); + run_preflight(state, prepared, validated, &preferred_providers, llm_result).await } fn manifest_workflow() -> types::ManifestWorkflow { @@ -2174,9 +2217,10 @@ name = "Control Plane" assert!(validated.has_errors()); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = + run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(!ok); assert_eq!(response.workflow.name, "Invalid"); @@ -2218,9 +2262,10 @@ issues = "read" let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); assert!(!validated.has_errors()); - let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, _ok) = + run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!( response.checks.sections[0] @@ -2269,9 +2314,10 @@ id = "local" assert!(!validated.has_errors()); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = + run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(ok); assert!(response.workflow.diagnostics.is_empty()); @@ -2376,9 +2422,10 @@ id = "daytona" .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, _ok) = + run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(response.workflow.diagnostics.is_empty()); assert!( @@ -2444,9 +2491,10 @@ digraph Demo { .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = + run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(!ok); let llm_check = response.checks.sections[0] @@ -2615,11 +2663,29 @@ digraph Demo { &manifest, ) .unwrap(); - let validated = validate_prepared_manifest(&prepared, state.catalog()).unwrap(); + let llm_result = state.resolve_llm_client().await; + let ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + assert!(ready_providers.is_empty()); + let validated = validate_prepared_manifest_for_preflight( + &prepared, + state.catalog(), + HashMap::new(), + &ready_providers, + ) + .unwrap(); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = run_preflight( + state.as_ref(), + &prepared, + &validated, + &ready_providers, + llm_result, + ) + .await + .unwrap(); assert!(!ok); let llm_check = response.checks.sections[0] diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 614f2d719..20a4b3f9d 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -835,10 +835,22 @@ async fn run_preflight( return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let mut validated = match run_manifest::validate_prepared_manifest_with_vars( + let llm_result = state.resolve_llm_client().await; + if let Err(error) = &llm_result { + tracing::warn!( + error = ?error, + "Failed to resolve LLM client while checking ready providers" + ); + } + let ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + let mut validated = match run_manifest::validate_prepared_manifest_for_preflight( &prepared, state.catalog(), vars, + &ready_providers, ) { Ok(validated) => validated, Err(WorkflowError::Parse(_)) => { @@ -847,7 +859,15 @@ async fn run_preflight( Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; validated.promote_template_undefined_variables_to_errors(); - let response = match run_manifest::run_preflight(&state, &prepared, &validated).await { + let response = match run_manifest::run_preflight( + &state, + &prepared, + &validated, + &ready_providers, + llm_result, + ) + .await + { Ok((response, _ok)) => response, Err(err) => { return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 144cf5976..39bd097bf 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -312,6 +312,7 @@ fn create_from_source( .filter(|provider| !provider.is_empty()) .map(ProviderId::new), &options.configured_providers, + None, &options.catalog, )?; @@ -336,6 +337,7 @@ pub(super) fn preprocess_and_validate( render_mode: RenderMode, default_provider: Option, eligible_providers: &[ProviderId], + fallback_providers: Option<&[ProviderId]>, catalog: &Arc, ) -> Result { let mut parsed = pipeline::parse(dot_source)?; @@ -351,6 +353,7 @@ pub(super) fn preprocess_and_validate( catalog: Arc::clone(catalog), default_provider, eligible_providers: eligible_providers.iter().cloned().collect(), + fallback_providers: fallback_providers.map(|providers| providers.iter().cloned().collect()), })?; Ok(pipeline::validate(transformed, catalog.as_ref(), &[])) } @@ -579,6 +582,7 @@ reasoning = false RenderMode::Structural, None, &test_provider_ids(), + None, &test_catalog(), ) .unwrap() @@ -749,6 +753,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), + None, &test_catalog(), ); let Err(err) = result else { @@ -787,6 +792,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), + None, &test_catalog(), ); let Err(err) = result else { diff --git a/lib/components/fabro-workflow/src/operations/mod.rs b/lib/components/fabro-workflow/src/operations/mod.rs index 4710d97a9..9e5726515 100644 --- a/lib/components/fabro-workflow/src/operations/mod.rs +++ b/lib/components/fabro-workflow/src/operations/mod.rs @@ -22,7 +22,7 @@ pub use rewind::{RewindInput, RewindOutcome, rewind}; pub use source::WorkflowInput; pub use start::{StartServices, Started, start}; pub use timeline::{ForkTarget, RunTimeline, TimelineEntry, build_timeline, timeline}; -pub use validate::{ValidateInput, validate}; +pub use validate::{ValidateInput, validate, validate_with_provider_fallback}; pub use crate::pipeline::{LlmSpec, SandboxEnvSpec}; pub use crate::transforms::RenderMode; diff --git a/lib/components/fabro-workflow/src/operations/validate.rs b/lib/components/fabro-workflow/src/operations/validate.rs index a95b13920..2fb7235b0 100644 --- a/lib/components/fabro-workflow/src/operations/validate.rs +++ b/lib/components/fabro-workflow/src/operations/validate.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; -use fabro_model::Catalog; +use fabro_model::{Catalog, ProviderId}; use fabro_types::WorkflowSettings; use super::create::{preprocess_and_validate, template_context}; @@ -28,17 +28,35 @@ pub struct ValidateInput { /// Returns `Validated` even when validation produced errors. Call /// `validated.raise_on_errors()` if the caller wants to fail fast. pub fn validate(input: ValidateInput) -> Result { + let eligible_providers = input + .catalog + .all_provider_ids() + .into_iter() + .collect::>(); + validate_with_provider_sets(input, &eligible_providers, None) +} + +/// Parse, transform, and validate while preferring one provider snapshot and +/// falling back to another only for provider-readiness selection failures. +pub fn validate_with_provider_fallback( + input: ValidateInput, + preferred_providers: &[ProviderId], + fallback_providers: &[ProviderId], +) -> Result { + validate_with_provider_sets(input, preferred_providers, Some(fallback_providers)) +} + +fn validate_with_provider_sets( + input: ValidateInput, + eligible_providers: &[ProviderId], + fallback_providers: Option<&[ProviderId]>, +) -> Result { let resolved = resolve_workflow(ResolveWorkflowInput { workflow: input.workflow, settings: input.settings, cwd: input.cwd, }) .map_err(|err| Error::Parse(err.to_string()))?; - let eligible_providers = input - .catalog - .all_provider_ids() - .into_iter() - .collect::>(); preprocess_and_validate( &resolved.raw_source, @@ -60,7 +78,8 @@ pub fn validate(input: ValidateInput) -> Result { .as_deref() .filter(|provider| !provider.is_empty()) .map(fabro_model::ProviderId::new), - &eligible_providers, + eligible_providers, + fallback_providers, &input.catalog, ) } diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index 6b76380d0..cc3fe089d 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -68,6 +68,7 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result, pub default_provider: Option, pub eligible_providers: HashSet, + pub fallback_providers: Option>, } /// Options for the FINALIZE phase. diff --git a/lib/components/fabro-workflow/src/pipeline/validate.rs b/lib/components/fabro-workflow/src/pipeline/validate.rs index 06120e8ce..8e19bb5a6 100644 --- a/lib/components/fabro-workflow/src/pipeline/validate.rs +++ b/lib/components/fabro-workflow/src/pipeline/validate.rs @@ -51,6 +51,7 @@ mod tests { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), + fallback_providers: None, }) .unwrap(); validate(transformed, catalog.as_ref(), &[]) diff --git a/lib/components/fabro-workflow/src/run_materialization.rs b/lib/components/fabro-workflow/src/run_materialization.rs index 7e82964ac..27183bcf5 100644 --- a/lib/components/fabro-workflow/src/run_materialization.rs +++ b/lib/components/fabro-workflow/src/run_materialization.rs @@ -9,10 +9,36 @@ use fabro_types::settings::run::RunGoal; use crate::error::Error; pub fn materialize_run( + settings: WorkflowSettings, + graph: &Graph, + catalog: &Catalog, + configured_providers: &[ProviderId], +) -> Result { + materialize_run_with_provider_sets(settings, graph, catalog, configured_providers, None) +} + +pub fn materialize_run_with_provider_fallback( + settings: WorkflowSettings, + graph: &Graph, + catalog: &Catalog, + preferred_providers: &[ProviderId], + fallback_providers: &[ProviderId], +) -> Result { + materialize_run_with_provider_sets( + settings, + graph, + catalog, + preferred_providers, + Some(fallback_providers), + ) +} + +fn materialize_run_with_provider_sets( mut settings: WorkflowSettings, graph: &Graph, catalog: &Catalog, configured_providers: &[ProviderId], + fallback_providers: Option<&[ProviderId]>, ) -> Result { let configured_model = settings.run.model.name.take(); let configured_provider = settings.run.model.provider.take(); @@ -30,11 +56,24 @@ pub fn materialize_run( let provider = configured_provider.or(graph_provider); let model = configured_model.or(graph_model); let eligible = configured_providers.iter().cloned().collect::>(); - let (resolved_model, resolved_provider) = - resolve_run_model(catalog, &eligible, model.as_deref(), provider.as_deref())?; + let fallback = + fallback_providers.map(|providers| providers.iter().cloned().collect::>()); + let provider = provider + .as_deref() + .filter(|provider| !provider.is_empty()) + .map(ProviderId::new); + let selected = match fallback { + Some(fallback) => catalog.resolve_selection_with_fallback( + model.as_deref(), + provider.as_ref(), + &eligible, + &fallback, + ), + None => catalog.resolve_selection(model.as_deref(), provider.as_ref(), &eligible), + }?; - settings.run.model.name = Some(resolved_model); - settings.run.model.provider = Some(resolved_provider.into_inner()); + settings.run.model.name = Some(selected.model); + settings.run.model.provider = Some(selected.provider.into_inner()); let goal = graph.goal().to_string(); settings.run.goal = if goal.is_empty() { diff --git a/lib/components/fabro-workflow/src/transforms/model_resolution.rs b/lib/components/fabro-workflow/src/transforms/model_resolution.rs index 7eecbcfd1..00e71ba74 100644 --- a/lib/components/fabro-workflow/src/transforms/model_resolution.rs +++ b/lib/components/fabro-workflow/src/transforms/model_resolution.rs @@ -13,6 +13,7 @@ pub struct ModelResolutionTransform { catalog: Arc, default_provider: Option, eligible_providers: HashSet, + fallback_providers: Option>, } impl ModelResolutionTransform { @@ -23,6 +24,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, + fallback_providers: None, } } @@ -32,6 +34,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, + fallback_providers: None, } } @@ -41,16 +44,33 @@ impl ModelResolutionTransform { self } + #[must_use] + pub fn with_fallback_providers( + mut self, + fallback_providers: Option>, + ) -> Self { + self.fallback_providers = fallback_providers; + self + } + fn resolve_model( &self, model: &str, explicit_provider: Option<&ProviderId>, ) -> Result<(String, ProviderId), Error> { - let selected = self.catalog.resolve_selection( - Some(model), - explicit_provider, - &self.eligible_providers, - )?; + let selected = match &self.fallback_providers { + Some(fallback_providers) => self.catalog.resolve_selection_with_fallback( + Some(model), + explicit_provider, + &self.eligible_providers, + fallback_providers, + ), + None => self.catalog.resolve_selection( + Some(model), + explicit_provider, + &self.eligible_providers, + ), + }?; Ok((selected.model, selected.provider)) } } @@ -326,6 +346,50 @@ reasoning = false ); } + #[test] + fn fallback_resolution_keeps_ready_preference_for_unpinned_nodes() { + let overrides: LlmCatalogSettings = toml::from_str( + r" +[providers.openrouter] +enabled = true +", + ) + .unwrap(); + let catalog = Arc::new(Catalog::from_builtin_with_overrides(&overrides).unwrap()); + let mut graph = Graph::new("test"); + let mut portable = Node::new("portable"); + portable.attrs.insert( + "model".to_string(), + AttrValue::String("claude-fable".to_string()), + ); + graph.nodes.insert("portable".to_string(), portable); + let mut pinned = Node::new("pinned"); + pinned.attrs.insert( + "model".to_string(), + AttrValue::String("claude-fable".to_string()), + ); + pinned.attrs.insert( + "provider".to_string(), + AttrValue::String("anthropic".to_string()), + ); + graph.nodes.insert("pinned".to_string(), pinned); + + let graph = ModelResolutionTransform::for_eligible( + Arc::clone(&catalog), + HashSet::from([ProviderId::new("openrouter")]), + ) + .with_fallback_providers(Some(catalog.all_provider_ids())) + .apply(graph) + .unwrap(); + + assert_eq!( + graph.nodes["portable"].provider(), + Some("openrouter"), + "the unrelated unavailable pin must not force catalog-wide routing" + ); + assert_eq!(graph.nodes["pinned"].provider(), Some("anthropic")); + } + #[test] fn graph_default_alias_materializes_to_canonical_offering() { let mut graph = Graph::new("test"); diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 5f3f75a55..44307ca81 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -4908,6 +4908,7 @@ async fn import_e2e_through_engine() { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), + fallback_providers: None, }) .unwrap(); let validated = validate(transformed, catalog.as_ref(), &[]); diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index cac9f878f..da9ab96fd 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -1101,6 +1101,32 @@ impl Catalog { } } + /// Resolve a selection against a preferred provider snapshot, falling back + /// to a broader eligible set only when the preferred set cannot supply the + /// requested provider or model. + /// + /// This is useful for readiness checks: ready providers remain preferred, + /// while a catalog-only offering can still be selected so the caller can + /// report why its provider is unavailable. Semantic failures such as an + /// unknown provider do not fall back. + pub fn resolve_selection_with_fallback( + &self, + selector: Option<&str>, + explicit_provider: Option<&ProviderId>, + preferred_providers: &HashSet, + fallback_providers: &HashSet, + ) -> Result { + match self.resolve_selection(selector, explicit_provider, preferred_providers) { + Ok(selected) => Ok(selected), + Err( + ModelSelectionError::ProviderUnavailable { .. } + | ModelSelectionError::NoEligibleOffering { .. } + | ModelSelectionError::NoDefaultModel { .. }, + ) => self.resolve_selection(selector, explicit_provider, fallback_providers), + Err(error) => Err(error), + } + } + #[must_use] pub fn is_model_selector(&self, selector: &str) -> bool { self.candidate_indices(selector).is_some() @@ -3989,6 +4015,41 @@ adapter = "openai_compatible" )); } + #[test] + fn selection_fallback_preserves_ready_preference_per_request() { + let catalog = portable_model_catalog(); + let openai = ProviderId::openai(); + let openrouter = ProviderId::new("openrouter"); + let ready = HashSet::from([openrouter.clone()]); + let catalog_providers = HashSet::from([openai.clone(), openrouter.clone()]); + + let shared = catalog + .resolve_selection_with_fallback(Some("portable"), None, &ready, &catalog_providers) + .unwrap(); + assert_eq!(shared.provider, openrouter); + + let pinned = catalog + .resolve_selection_with_fallback( + Some("portable"), + Some(&openai), + &ready, + &catalog_providers, + ) + .unwrap(); + assert_eq!(pinned.provider, openai); + + let unknown = catalog + .resolve_selection_with_fallback( + Some("provider-private-preview"), + None, + &ready, + &catalog_providers, + ) + .unwrap(); + assert_eq!(unknown.provider, ProviderId::new("openrouter")); + assert_eq!(unknown.model, "provider-private-preview"); + } + #[test] fn legacy_builtin_selector_uses_readiness_priority_and_explicit_pins() { let catalog = portable_model_catalog(); From 142862f3422d72ab4106796342fdb4bbd0245da8 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 07:36:56 -0400 Subject: [PATCH 08/11] Expose detailed completion token usage --- docs/public/api-reference/fabro-api.yaml | 25 ++++++- .../src/server/handler/completions.rs | 18 ++--- lib/apps/fabro-server/src/server/tests.rs | 67 +++++++++++++++++++ lib/foundation/fabro-api/build.rs | 1 + lib/foundation/fabro-api/src/lib.rs | 1 + .../tests/completion_usage_round_trip.rs | 59 ++++++++++++++++ .../src/models/completion-usage.ts | 21 ++++++ 7 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 lib/foundation/fabro-api/tests/completion_usage_round_trip.rs diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 519ee4188..342ef745a 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -8507,15 +8507,38 @@ components: description: Provider-specific options. CompletionUsage: + description: > + Five disjoint token buckets for one completion. `input_tokens` excludes + cache reads and writes, while `output_tokens` excludes reasoning tokens + when the provider reports them separately. type: object - required: [input_tokens, output_tokens] + required: + - input_tokens + - output_tokens + - reasoning_tokens + - cache_read_tokens + - cache_write_tokens properties: input_tokens: type: integer format: int64 + description: Number of uncached input tokens consumed. output_tokens: type: integer format: int64 + description: Number of non-reasoning output tokens generated. + reasoning_tokens: + type: integer + format: int64 + description: Number of separately reported reasoning tokens. + cache_read_tokens: + type: integer + format: int64 + description: Number of input tokens served from a provider cache. + cache_write_tokens: + type: integer + format: int64 + description: Number of input tokens written to a provider cache. CompletionResponse: type: object diff --git a/lib/apps/fabro-server/src/server/handler/completions.rs b/lib/apps/fabro-server/src/server/handler/completions.rs index b53e56d31..a8d7c3bb4 100644 --- a/lib/apps/fabro-server/src/server/handler/completions.rs +++ b/lib/apps/fabro-server/src/server/handler/completions.rs @@ -4,10 +4,10 @@ use std::sync::Arc; use fabro_model::{Catalog, ModelSelectionError}; use super::super::{ - ApiError, AppState, CompletionResponse, CompletionToolChoiceMode, CompletionUsage, - CreateCompletionRequest, FinishReason, GenerateParams, IntoResponse, Json, LlmMessage, - LlmRequest, ProviderId, RequiredUser, Response, Router, State, StatusCode, ToolChoice, - ToolDefinition, Ulid, error, generate_object, info, post, warn, + ApiError, AppState, CompletionResponse, CompletionToolChoiceMode, CreateCompletionRequest, + FinishReason, GenerateParams, IntoResponse, Json, LlmMessage, LlmRequest, ProviderId, + RequiredUser, Response, Router, State, StatusCode, ToolChoice, ToolDefinition, Ulid, error, + generate_object, info, post, warn, }; use super::llm_sse; @@ -169,10 +169,7 @@ async fn create_completion( provider: selected_provider, message: response.message, stop_reason, - usage: CompletionUsage { - input_tokens: response.usage.input_tokens, - output_tokens: response.usage.output_tokens, - }, + usage: response.usage, output, cost_usd: response.cost_usd, cost_source: response.cost_source, @@ -192,10 +189,7 @@ async fn create_completion( provider: ProviderId::new(response.provider), message: response.message, stop_reason, - usage: CompletionUsage { - input_tokens: response.usage.input_tokens, - output_tokens: response.usage.output_tokens, - }, + usage: response.usage, output: None, cost_usd: response.cost_usd, cost_source: response.cost_source, diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 454455be0..982b270f6 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -15272,6 +15272,73 @@ async fn create_completion_unknown_provider_returns_clear_error() { ); } +#[tokio::test] +async fn create_completion_returns_disjoint_usage_buckets() { + let upstream = MockServer::start(); + let completion = upstream.mock(|when, then| { + when.method(POST).path("/chat/completions"); + then.status(200) + .header("content-type", "application/json") + .json_body(json!({ + "id": "chatcmpl-usage", + "model": "kimi-k3", + "choices": [{ + "message": {"role": "assistant", "content": "OK"}, + "finish_reason": "stop" + }], + "usage": { + "prompt_tokens": 200, + "completion_tokens": 30, + "total_tokens": 230, + "prompt_tokens_details": { + "cached_tokens": 50, + "cache_write_tokens": 100 + }, + "completion_tokens_details": { + "reasoning_tokens": 20 + } + } + })); + }); + let state = TestAppStateBuilder::new() + .provider_base_url("kimi", upstream.base_url()) + .vault_entries([(EnvVars::KIMI_API_KEY, "test-kimi-api-key")]) + .build(); + let app = crate::test_support::build_test_router(state); + + let req = Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + json!({ + "provider": "kimi", + "model": "kimi-k3", + "stream": false, + "messages": [{ + "role": "user", + "content": [{"kind": "text", "data": "hi"}] + }] + }) + .to_string(), + )) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::OK).await; + assert_eq!( + body["usage"], + json!({ + "input_tokens": 50, + "output_tokens": 10, + "reasoning_tokens": 20, + "cache_read_tokens": 50, + "cache_write_tokens": 100 + }) + ); + completion.assert(); +} + #[tokio::test] async fn create_completion_default_model_uses_app_state_catalog() { let upstream = MockServer::start(); diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index cdec7f07a..f979a22d1 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -461,6 +461,7 @@ fn main() { "fabro_types::PendingInterviewRecord", &[], ), + ("CompletionUsage", "fabro_model::TokenCounts", &[]), ("BilledTokenCounts", "fabro_types::BilledTokenCounts", &[]), ("BillingModelRef", "fabro_model::ModelRef", &[]), ("BillingSpeed", "fabro_model::Speed", &[]), diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index 6d68d57b4..4a7a65bcd 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -22,6 +22,7 @@ pub mod types { pub use fabro_model::{ CostSource, Model, ModelCosts, ModelFeatures, ModelLimits, ModelRef as BillingModelRef, ModelTestMode, Provider, ReasoningEffort, ReasoningEffortFeature, Speed as BillingSpeed, + TokenCounts as CompletionUsage, }; pub use fabro_types::run_event::AgentSessionActivatedProps; pub use fabro_types::settings::run::McpHttpProtocol; diff --git a/lib/foundation/fabro-api/tests/completion_usage_round_trip.rs b/lib/foundation/fabro-api/tests/completion_usage_round_trip.rs new file mode 100644 index 000000000..58da0288b --- /dev/null +++ b/lib/foundation/fabro-api/tests/completion_usage_round_trip.rs @@ -0,0 +1,59 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::CompletionUsage as ApiCompletionUsage; +use fabro_model::TokenCounts; +use serde_json::json; + +#[test] +fn completion_usage_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn completion_usage_json_matches_openapi_shape() { + let usage = TokenCounts { + input_tokens: 10, + output_tokens: 20, + reasoning_tokens: 3, + cache_read_tokens: 4, + cache_write_tokens: 5, + }; + + let json = serde_json::to_value(&usage).unwrap(); + assert_eq!(json["input_tokens"], 10); + assert_eq!(json["output_tokens"], 20); + assert_eq!(json["reasoning_tokens"], 3); + assert_eq!(json["cache_read_tokens"], 4); + assert_eq!(json["cache_write_tokens"], 5); + + let round_trip: ApiCompletionUsage = serde_json::from_value(json).unwrap(); + assert_eq!(round_trip, usage); +} + +#[test] +fn completion_usage_keeps_zero_counts_present() { + let json = serde_json::to_value(TokenCounts::default()).unwrap(); + assert_eq!( + json, + json!({ + "input_tokens": 0, + "output_tokens": 0, + "reasoning_tokens": 0, + "cache_read_tokens": 0, + "cache_write_tokens": 0 + }) + ); + + let round_trip: ApiCompletionUsage = serde_json::from_value(json).unwrap(); + assert_eq!(round_trip, TokenCounts::default()); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/packages/fabro-api-client/src/models/completion-usage.ts b/lib/packages/fabro-api-client/src/models/completion-usage.ts index d10caeb7d..056007b1b 100644 --- a/lib/packages/fabro-api-client/src/models/completion-usage.ts +++ b/lib/packages/fabro-api-client/src/models/completion-usage.ts @@ -14,7 +14,28 @@ +/** + * Five disjoint token buckets for one completion. `input_tokens` excludes cache reads and writes, while `output_tokens` excludes reasoning tokens when the provider reports them separately. + */ export interface CompletionUsage { + /** + * Number of uncached input tokens consumed. + */ 'input_tokens': number; + /** + * Number of non-reasoning output tokens generated. + */ 'output_tokens': number; + /** + * Number of separately reported reasoning tokens. + */ + 'reasoning_tokens': number; + /** + * Number of input tokens served from a provider cache. + */ + 'cache_read_tokens': number; + /** + * Number of input tokens written to a provider cache. + */ + 'cache_write_tokens': number; } From 40d6992148f26a5c0b872d197a947cf03b1beb05 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 08:13:54 -0400 Subject: [PATCH 09/11] Trim redundant reasoning effort request tests The unknown-value rejection test duplicated strum coverage in fabro-model and the HTTP 422 test in fabro-server. Keep only the field-type assertion, using the same field-pinning idiom as stage_model_usage_round_trip. Co-Authored-By: Claude Fable 5 --- .../tests/create_completion_request_round_trip.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs b/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs index 913947453..084827149 100644 --- a/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs +++ b/lib/foundation/fabro-api/tests/create_completion_request_round_trip.rs @@ -10,16 +10,5 @@ fn create_completion_request_reuses_canonical_reasoning_effort() { })) .unwrap(); - let reasoning_effort: Option = request.reasoning_effort; - assert_eq!(reasoning_effort, Some(ReasoningEffort::High)); -} - -#[test] -fn create_completion_request_rejects_unknown_reasoning_effort() { - let result = serde_json::from_value::(json!({ - "messages": [], - "reasoning_effort": "bogus" - })); - - assert!(result.is_err()); + assert_eq!(request.reasoning_effort, Some(ReasoningEffort::High)); } From 0cd22ebd7586680b9b4e4f1b8ea762d83ba806ce Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 08:40:55 -0400 Subject: [PATCH 10/11] refactor: build structured-output GenerateParams via struct update Replaces the per-field if-let cascade in the structured completion path with a single struct-update expression. The cascade had to be extended by hand for every request field and silently dropped stop_sequences and provider_options, which the non-structured path already forwarded. Co-Authored-By: Claude Fable 5 --- .../src/server/handler/completions.rs | 36 +++++++++---------- lib/apps/fabro-server/src/server/tests.rs | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lib/apps/fabro-server/src/server/handler/completions.rs b/lib/apps/fabro-server/src/server/handler/completions.rs index eb6dafaf4..146d20808 100644 --- a/lib/apps/fabro-server/src/server/handler/completions.rs +++ b/lib/apps/fabro-server/src/server/handler/completions.rs @@ -139,25 +139,23 @@ async fn create_completion( let msg_id = Ulid::new().to_string(); if let Some(schema) = req.schema { - // Structured output uses generate_object for JSON parsing logic - let mut params = - GenerateParams::new(&request.model, std::sync::Arc::new(client.clone())) - .messages(request.messages); - if let Some(ref p) = request.provider { - params = params.provider(p); - } - if let Some(temp) = request.temperature { - params = params.temperature(temp); - } - if let Some(max_tokens) = request.max_tokens { - params = params.max_tokens(max_tokens); - } - if let Some(top_p) = request.top_p { - params = params.top_p(top_p); - } - if let Some(reasoning_effort) = request.reasoning_effort { - params = params.reasoning_effort(reasoning_effort); - } + // Structured output uses generate_object for JSON parsing logic. + // tools/tool_choice are not forwarded: GenerateParams carries + // executable Arcs, not wire ToolDefinitions, and + // generate_object sets response_format from the schema itself. + let params = GenerateParams { + messages: Some(request.messages), + provider: request.provider, + temperature: request.temperature, + top_p: request.top_p, + max_tokens: request.max_tokens, + stop_sequences: request.stop_sequences, + reasoning_effort: request.reasoning_effort, + speed: request.speed, + metadata: request.metadata, + provider_options: request.provider_options, + ..GenerateParams::new(request.model, std::sync::Arc::new(client.clone())) + }; match generate_object(params, schema).await { Ok(result) => { // `result.finish_reason` / `result.usage` resolve through diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 8e7b124e7..eb0083948 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -15431,7 +15431,7 @@ async fn create_completion_structured_output_forwards_reasoning_effort() { let response = app.oneshot(req).await.unwrap(); let body = response_json!(response, StatusCode::OK).await; assert_eq!(body["output"], json!({"answer": 42})); - completion.assert_calls(1); + completion.assert(); } #[tokio::test] From c06c60214aa478d89bf53d4472c10ce27ea5596d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 24 Jul 2026 08:49:01 -0400 Subject: [PATCH 11/11] refactor: simplify readiness-fallback plumbing The fallback provider set was always catalog.all_provider_ids(), computed at every call site and threaded through five layers alongside the catalog itself. Fold it into Catalog::resolve_selection_with_catalog_fallback and carry only a catalog_fallback flag through the transform/validate/ materialize entry points. - materialize_run delegates to resolve_run_model again instead of re-inlining its provider normalization and selection - run_preflight derives ready providers from llm_result instead of taking both, so callers cannot pass inconsistent pairs; the legacy tests now exercise the production ready-first routing path - AppState::resolve_llm_client_with_ready_ids replaces three copies of resolve-then-extract-provider-ids, and ready_llm_provider_ids delegates to it - the unreachable "model resolution failed" preflight check becomes an invariant error where the materialized run is produced - validate_prepared_manifest_with_vars/_for_preflight share the ValidateInput construction Co-Authored-By: Claude Fable 5 --- lib/apps/fabro-server/src/run_manifest.rs | 153 +++++++----------- lib/apps/fabro-server/src/server.rs | 26 ++- .../fabro-server/src/server/handler/runs.rs | 49 ++---- .../fabro-workflow/src/operations/create.rs | 12 +- .../fabro-workflow/src/operations/mod.rs | 2 +- .../fabro-workflow/src/operations/start.rs | 1 + .../fabro-workflow/src/operations/validate.rs | 20 +-- .../fabro-workflow/src/pipeline/transform.rs | 10 +- .../fabro-workflow/src/pipeline/types.rs | 4 +- .../fabro-workflow/src/pipeline/validate.rs | 2 +- .../fabro-workflow/src/run_materialization.rs | 59 +++---- .../src/transforms/model_resolution.rs | 32 ++-- .../fabro-workflow/tests/it/integration.rs | 2 +- lib/foundation/fabro-model/src/catalog.rs | 26 +-- 14 files changed, 166 insertions(+), 232 deletions(-) diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index aff59b0c5..65cc6d36e 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -35,12 +35,10 @@ use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSecti use fabro_validate::Severity; use fabro_workflow::Error as WorkflowError; use fabro_workflow::operations::{ - CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_provider_fallback, + CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_ready_providers, }; use fabro_workflow::pipeline::Validated; -#[cfg(test)] -use fabro_workflow::run_materialization::materialize_run; -use fabro_workflow::run_materialization::materialize_run_with_provider_fallback; +use fabro_workflow::run_materialization::materialize_run_with_ready_providers; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; use futures_util::stream::{self, StreamExt}; use tokio::process::Command; @@ -199,14 +197,7 @@ pub(crate) fn validate_prepared_manifest_with_vars( catalog: Arc, vars: HashMap, ) -> Result { - validate(ValidateInput { - workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), - settings: prepared.settings.clone(), - vars, - cwd: prepared.cwd.clone(), - custom_transforms: Vec::new(), - catalog, - }) + validate(manifest_validate_input(prepared, catalog, vars)) } pub(crate) fn validate_prepared_manifest_for_preflight( @@ -215,21 +206,27 @@ pub(crate) fn validate_prepared_manifest_for_preflight( vars: HashMap, ready_providers: &[ProviderId], ) -> Result { - let fallback_providers = catalog.all_provider_ids().into_iter().collect::>(); - validate_with_provider_fallback( - ValidateInput { - workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), - settings: prepared.settings.clone(), - vars, - cwd: prepared.cwd.clone(), - custom_transforms: Vec::new(), - catalog, - }, + validate_with_ready_providers( + manifest_validate_input(prepared, catalog, vars), ready_providers, - &fallback_providers, ) } +fn manifest_validate_input( + prepared: &PreparedManifest, + catalog: Arc, + vars: HashMap, +) -> ValidateInput { + ValidateInput { + workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), + settings: prepared.settings.clone(), + vars, + cwd: prepared.cwd.clone(), + custom_transforms: Vec::new(), + catalog, + } +} + pub(crate) fn create_run_input( prepared: PreparedManifest, configured_providers: Vec, @@ -262,11 +259,10 @@ pub(crate) async fn run_preflight( state: &AppState, prepared: &PreparedManifest, validated: &Validated, - preferred_providers: &[ProviderId], llm_result: Result, ) -> Result<(types::PreflightResponse, bool)> { let (report, checks_ok) = - build_preflight_report(state, prepared, validated, preferred_providers, llm_result).await?; + build_preflight_report(state, prepared, validated, llm_result).await?; let preflight_ok = !validated.has_errors() && checks_ok; Ok(( preflight_response( @@ -485,7 +481,6 @@ async fn build_preflight_report( state: &AppState, prepared: &PreparedManifest, validated: &Validated, - preferred_providers: &[ProviderId], llm_result: Result, ) -> Result<(CheckReport, bool)> { let graph = validated.graph(); @@ -504,15 +499,23 @@ async fn build_preflight_report( } let catalog = state.catalog(); - let fallback_providers = catalog.all_provider_ids().into_iter().collect::>(); - let materialized = materialize_run_with_provider_fallback( + let ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + let materialized = materialize_run_with_ready_providers( prepared.settings.clone(), graph, catalog.as_ref(), - preferred_providers, - &fallback_providers, + &ready_providers, )?; let resolved_run = materialized.run; + let (Some(run_model), Some(run_provider)) = ( + resolved_run.model.name.as_deref(), + resolved_run.model.provider.as_deref(), + ) else { + bail!("materialized run is missing a resolved model or provider"); + }; let server_settings = state.server_settings(); let github_integration = &server_settings.server.integrations.github; let sandbox_provider = effective_sandbox_provider(&resolved_run); @@ -575,7 +578,8 @@ async fn build_preflight_report( let llm_ok = run_llm_check( &mut checks, graph, - &resolved_run, + run_model, + run_provider, catalog.as_ref(), llm_result, ) @@ -1051,25 +1055,11 @@ struct PendingModelProbe { async fn run_llm_check( checks: &mut Vec, graph: &Graph, - settings: &RunNamespace, + model: &str, + default_provider: &str, catalog: &Catalog, llm_result: Result, ) -> bool { - let (Some(model), Some(default_provider)) = ( - settings.model.name.as_deref(), - settings.model.provider.as_deref(), - ) else { - checks.push(CheckResult { - name: "LLM".into(), - status: CheckStatus::Error, - summary: "model resolution failed".into(), - details: Vec::new(), - remediation: Some( - "Preflight did not produce a resolved run model and provider".to_string(), - ), - }); - return false; - }; let mut model_providers = std::collections::BTreeSet::new(); let mut has_llm_nodes = false; @@ -1388,6 +1378,7 @@ fn report_to_api(report: &CheckReport) -> types::PreflightCheckReport { mod tests { use fabro_model::ProviderId; use fabro_model::catalog::LlmCatalogSettings; + use fabro_workflow::run_materialization::materialize_run; use super::*; @@ -1563,29 +1554,18 @@ digraph Demo {{ ) .unwrap(); - run_preflight( - state.as_ref(), - &prepared, - &validated, - &ready_providers, - llm_result, - ) - .await - .unwrap() + run_preflight(state.as_ref(), &prepared, &validated, llm_result) + .await + .unwrap() } - async fn run_preflight_with_catalog_routes( + async fn resolve_and_run_preflight( state: &AppState, prepared: &PreparedManifest, validated: &Validated, ) -> Result<(types::PreflightResponse, bool)> { let llm_result = state.resolve_llm_client().await; - let preferred_providers = state - .catalog() - .all_provider_ids() - .into_iter() - .collect::>(); - run_preflight(state, prepared, validated, &preferred_providers, llm_result).await + run_preflight(state, prepared, validated, llm_result).await } fn manifest_workflow() -> types::ManifestWorkflow { @@ -2217,10 +2197,9 @@ name = "Control Plane" assert!(validated.has_errors()); - let (response, ok) = - run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(!ok); assert_eq!(response.workflow.name, "Invalid"); @@ -2262,10 +2241,9 @@ issues = "read" let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); assert!(!validated.has_errors()); - let (response, _ok) = - run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!( response.checks.sections[0] @@ -2314,10 +2292,9 @@ id = "local" assert!(!validated.has_errors()); - let (response, ok) = - run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(ok); assert!(response.workflow.diagnostics.is_empty()); @@ -2422,10 +2399,9 @@ id = "daytona" .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, _ok) = - run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(response.workflow.diagnostics.is_empty()); assert!( @@ -2491,10 +2467,9 @@ digraph Demo { .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, ok) = - run_preflight_with_catalog_routes(state.as_ref(), &prepared, &validated) - .await - .unwrap(); + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); assert!(!ok); let llm_check = response.checks.sections[0] @@ -2677,15 +2652,9 @@ digraph Demo { ) .unwrap(); - let (response, ok) = run_preflight( - state.as_ref(), - &prepared, - &validated, - &ready_providers, - llm_result, - ) - .await - .unwrap(); + let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated, llm_result) + .await + .unwrap(); assert!(!ok); let llm_check = response.checks.sections[0] diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index fac2dd51a..47b57a462 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1445,14 +1445,26 @@ impl AppState { self.llm_source.configured_providers(catalog.as_ref()).await } - pub(crate) async fn ready_llm_provider_ids(&self) -> Vec { - match self.resolve_llm_client().await { - Ok(result) => result.provider_ids(), - Err(err) => { - warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); - Vec::new() - } + /// Resolve the LLM client once and derive the ready provider IDs from it, + /// logging a warning when resolution fails. Callers that need both values + /// must use this instead of `ready_llm_provider_ids` so the client is not + /// resolved twice. + pub(crate) async fn resolve_llm_client_with_ready_ids( + &self, + ) -> (anyhow::Result, Vec) { + let llm_result = self.resolve_llm_client().await; + if let Err(err) = &llm_result { + warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); } + let ready_provider_ids = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + (llm_result, ready_provider_ids) + } + + pub(crate) async fn ready_llm_provider_ids(&self) -> Vec { + self.resolve_llm_client_with_ready_ids().await.1 } pub(crate) async fn decorate_run_summary(&self, run: fabro_types::Run) -> fabro_types::Run { diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 20a4b3f9d..fdb9c1c3e 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -51,7 +51,6 @@ use crate::run_files::{list_run_commits, list_run_files}; use crate::run_manifest; use crate::run_selector::{ResolveRunError, resolve_run_by_selector}; use crate::run_title_generation::{self, GenerateTitleInput, TitlePromptInput, WorkflowSummary}; -use crate::server_secrets::LlmClientResult; #[cfg(any(test, feature = "test-support"))] use crate::test_support as server_test_support; @@ -591,17 +590,8 @@ pub(crate) async fn create_run_from_manifest( // and ask-fabro-readiness) and the LLM client itself (for the spawned // title-generation task). `ready_llm_provider_ids` would otherwise call // `resolve_llm_client` a second time and discard the client. - let llm_client_for_title = match state.resolve_llm_client().await { - Ok(result) => Some(result), - Err(err) => { - tracing::warn!(error = ?err, "Failed to resolve LLM client while creating run"); - None - } - }; - let ready_provider_ids = llm_client_for_title - .as_ref() - .map(LlmClientResult::provider_ids) - .unwrap_or_default(); + let (llm_result, ready_provider_ids) = state.resolve_llm_client_with_ready_ids().await; + let llm_client_for_title = llm_result.ok(); let run_materialization_provider_ids = { #[cfg(any(test, feature = "test-support"))] { @@ -835,17 +825,7 @@ async fn run_preflight( return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let llm_result = state.resolve_llm_client().await; - if let Err(error) = &llm_result { - tracing::warn!( - error = ?error, - "Failed to resolve LLM client while checking ready providers" - ); - } - let ready_providers = llm_result - .as_ref() - .map(LlmClientResult::provider_ids) - .unwrap_or_default(); + let (llm_result, ready_providers) = state.resolve_llm_client_with_ready_ids().await; let mut validated = match run_manifest::validate_prepared_manifest_for_preflight( &prepared, state.catalog(), @@ -859,21 +839,14 @@ async fn run_preflight( Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; validated.promote_template_undefined_variables_to_errors(); - let response = match run_manifest::run_preflight( - &state, - &prepared, - &validated, - &ready_providers, - llm_result, - ) - .await - { - Ok((response, _ok)) => response, - Err(err) => { - return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) - .into_response(); - } - }; + let response = + match run_manifest::run_preflight(&state, &prepared, &validated, llm_result).await { + Ok((response, _ok)) => response, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; (StatusCode::OK, Json(response)).into_response() } diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 39bd097bf..2ec40d443 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -312,7 +312,7 @@ fn create_from_source( .filter(|provider| !provider.is_empty()) .map(ProviderId::new), &options.configured_providers, - None, + false, &options.catalog, )?; @@ -337,7 +337,7 @@ pub(super) fn preprocess_and_validate( render_mode: RenderMode, default_provider: Option, eligible_providers: &[ProviderId], - fallback_providers: Option<&[ProviderId]>, + catalog_fallback: bool, catalog: &Arc, ) -> Result { let mut parsed = pipeline::parse(dot_source)?; @@ -353,7 +353,7 @@ pub(super) fn preprocess_and_validate( catalog: Arc::clone(catalog), default_provider, eligible_providers: eligible_providers.iter().cloned().collect(), - fallback_providers: fallback_providers.map(|providers| providers.iter().cloned().collect()), + catalog_fallback, })?; Ok(pipeline::validate(transformed, catalog.as_ref(), &[])) } @@ -582,7 +582,7 @@ reasoning = false RenderMode::Structural, None, &test_provider_ids(), - None, + false, &test_catalog(), ) .unwrap() @@ -753,7 +753,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), - None, + false, &test_catalog(), ); let Err(err) = result else { @@ -792,7 +792,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), - None, + false, &test_catalog(), ); let Err(err) = result else { diff --git a/lib/components/fabro-workflow/src/operations/mod.rs b/lib/components/fabro-workflow/src/operations/mod.rs index 9e5726515..38b7d9aaa 100644 --- a/lib/components/fabro-workflow/src/operations/mod.rs +++ b/lib/components/fabro-workflow/src/operations/mod.rs @@ -22,7 +22,7 @@ pub use rewind::{RewindInput, RewindOutcome, rewind}; pub use source::WorkflowInput; pub use start::{StartServices, Started, start}; pub use timeline::{ForkTarget, RunTimeline, TimelineEntry, build_timeline, timeline}; -pub use validate::{ValidateInput, validate, validate_with_provider_fallback}; +pub use validate::{ValidateInput, validate, validate_with_ready_providers}; pub use crate::pipeline::{LlmSpec, SandboxEnvSpec}; pub use crate::transforms::RenderMode; diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 9c2e3ed8d..ba720b11f 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -622,6 +622,7 @@ fn resolve_start_llm( &eligible, settings.model.name.as_deref(), settings.model.provider.as_deref(), + false, )?; let fallback_chain = resolve_fallback_chain(catalog, &provider_id, &model, &settings.model, &eligible)?; diff --git a/lib/components/fabro-workflow/src/operations/validate.rs b/lib/components/fabro-workflow/src/operations/validate.rs index 2fb7235b0..c2b990f5c 100644 --- a/lib/components/fabro-workflow/src/operations/validate.rs +++ b/lib/components/fabro-workflow/src/operations/validate.rs @@ -33,23 +33,23 @@ pub fn validate(input: ValidateInput) -> Result { .all_provider_ids() .into_iter() .collect::>(); - validate_with_provider_sets(input, &eligible_providers, None) + validate_with_eligible_providers(input, &eligible_providers, false) } -/// Parse, transform, and validate while preferring one provider snapshot and -/// falling back to another only for provider-readiness selection failures. -pub fn validate_with_provider_fallback( +/// Parse, transform, and validate, resolving models against the ready +/// providers first and falling back to the full catalog only for +/// provider-readiness selection failures. +pub fn validate_with_ready_providers( input: ValidateInput, - preferred_providers: &[ProviderId], - fallback_providers: &[ProviderId], + ready_providers: &[ProviderId], ) -> Result { - validate_with_provider_sets(input, preferred_providers, Some(fallback_providers)) + validate_with_eligible_providers(input, ready_providers, true) } -fn validate_with_provider_sets( +fn validate_with_eligible_providers( input: ValidateInput, eligible_providers: &[ProviderId], - fallback_providers: Option<&[ProviderId]>, + catalog_fallback: bool, ) -> Result { let resolved = resolve_workflow(ResolveWorkflowInput { workflow: input.workflow, @@ -79,7 +79,7 @@ fn validate_with_provider_sets( .filter(|provider| !provider.is_empty()) .map(fabro_model::ProviderId::new), eligible_providers, - fallback_providers, + catalog_fallback, &input.catalog, ) } diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index cc3fe089d..b399d6637 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -68,7 +68,7 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result, pub default_provider: Option, pub eligible_providers: HashSet, - pub fallback_providers: Option>, + /// Fall back to the full catalog when the eligible providers cannot + /// supply a requested model, instead of erroring. + pub catalog_fallback: bool, } /// Options for the FINALIZE phase. diff --git a/lib/components/fabro-workflow/src/pipeline/validate.rs b/lib/components/fabro-workflow/src/pipeline/validate.rs index 8e19bb5a6..f0cd51dbd 100644 --- a/lib/components/fabro-workflow/src/pipeline/validate.rs +++ b/lib/components/fabro-workflow/src/pipeline/validate.rs @@ -51,7 +51,7 @@ mod tests { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), - fallback_providers: None, + catalog_fallback: false, }) .unwrap(); validate(transformed, catalog.as_ref(), &[]) diff --git a/lib/components/fabro-workflow/src/run_materialization.rs b/lib/components/fabro-workflow/src/run_materialization.rs index 27183bcf5..d1118b076 100644 --- a/lib/components/fabro-workflow/src/run_materialization.rs +++ b/lib/components/fabro-workflow/src/run_materialization.rs @@ -14,31 +14,27 @@ pub fn materialize_run( catalog: &Catalog, configured_providers: &[ProviderId], ) -> Result { - materialize_run_with_provider_sets(settings, graph, catalog, configured_providers, None) + materialize_run_with_eligible_providers(settings, graph, catalog, configured_providers, false) } -pub fn materialize_run_with_provider_fallback( +/// Materialize while resolving the run model against the ready providers +/// first, falling back to the full catalog only for provider-readiness +/// selection failures. +pub fn materialize_run_with_ready_providers( settings: WorkflowSettings, graph: &Graph, catalog: &Catalog, - preferred_providers: &[ProviderId], - fallback_providers: &[ProviderId], + ready_providers: &[ProviderId], ) -> Result { - materialize_run_with_provider_sets( - settings, - graph, - catalog, - preferred_providers, - Some(fallback_providers), - ) + materialize_run_with_eligible_providers(settings, graph, catalog, ready_providers, true) } -fn materialize_run_with_provider_sets( +fn materialize_run_with_eligible_providers( mut settings: WorkflowSettings, graph: &Graph, catalog: &Catalog, - configured_providers: &[ProviderId], - fallback_providers: Option<&[ProviderId]>, + eligible_providers: &[ProviderId], + catalog_fallback: bool, ) -> Result { let configured_model = settings.run.model.name.take(); let configured_provider = settings.run.model.provider.take(); @@ -55,25 +51,17 @@ fn materialize_run_with_provider_sets( let provider = configured_provider.or(graph_provider); let model = configured_model.or(graph_model); - let eligible = configured_providers.iter().cloned().collect::>(); - let fallback = - fallback_providers.map(|providers| providers.iter().cloned().collect::>()); - let provider = provider - .as_deref() - .filter(|provider| !provider.is_empty()) - .map(ProviderId::new); - let selected = match fallback { - Some(fallback) => catalog.resolve_selection_with_fallback( - model.as_deref(), - provider.as_ref(), - &eligible, - &fallback, - ), - None => catalog.resolve_selection(model.as_deref(), provider.as_ref(), &eligible), - }?; + let eligible = eligible_providers.iter().cloned().collect::>(); + let (resolved_model, resolved_provider) = resolve_run_model( + catalog, + &eligible, + model.as_deref(), + provider.as_deref(), + catalog_fallback, + )?; - settings.run.model.name = Some(selected.model); - settings.run.model.provider = Some(selected.provider.into_inner()); + settings.run.model.name = Some(resolved_model); + settings.run.model.provider = Some(resolved_provider.into_inner()); let goal = graph.goal().to_string(); settings.run.goal = if goal.is_empty() { @@ -99,10 +87,15 @@ pub(crate) fn resolve_run_model( eligible: &HashSet, model: Option<&str>, provider: Option<&str>, + catalog_fallback: bool, ) -> Result<(String, ProviderId), ModelSelectionError> { let provider = provider .filter(|provider| !provider.is_empty()) .map(ProviderId::new); - let selected = catalog.resolve_selection(model, provider.as_ref(), eligible)?; + let selected = if catalog_fallback { + catalog.resolve_selection_with_catalog_fallback(model, provider.as_ref(), eligible)? + } else { + catalog.resolve_selection(model, provider.as_ref(), eligible)? + }; Ok((selected.model, selected.provider)) } diff --git a/lib/components/fabro-workflow/src/transforms/model_resolution.rs b/lib/components/fabro-workflow/src/transforms/model_resolution.rs index 00e71ba74..12f29ac5a 100644 --- a/lib/components/fabro-workflow/src/transforms/model_resolution.rs +++ b/lib/components/fabro-workflow/src/transforms/model_resolution.rs @@ -13,7 +13,7 @@ pub struct ModelResolutionTransform { catalog: Arc, default_provider: Option, eligible_providers: HashSet, - fallback_providers: Option>, + catalog_fallback: bool, } impl ModelResolutionTransform { @@ -24,7 +24,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, - fallback_providers: None, + catalog_fallback: false, } } @@ -34,7 +34,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, - fallback_providers: None, + catalog_fallback: false, } } @@ -44,12 +44,11 @@ impl ModelResolutionTransform { self } + /// When enabled, provider-readiness selection failures fall back to the + /// full catalog instead of erroring. #[must_use] - pub fn with_fallback_providers( - mut self, - fallback_providers: Option>, - ) -> Self { - self.fallback_providers = fallback_providers; + pub fn with_catalog_fallback(mut self, catalog_fallback: bool) -> Self { + self.catalog_fallback = catalog_fallback; self } @@ -58,18 +57,15 @@ impl ModelResolutionTransform { model: &str, explicit_provider: Option<&ProviderId>, ) -> Result<(String, ProviderId), Error> { - let selected = match &self.fallback_providers { - Some(fallback_providers) => self.catalog.resolve_selection_with_fallback( + let selected = if self.catalog_fallback { + self.catalog.resolve_selection_with_catalog_fallback( Some(model), explicit_provider, &self.eligible_providers, - fallback_providers, - ), - None => self.catalog.resolve_selection( - Some(model), - explicit_provider, - &self.eligible_providers, - ), + ) + } else { + self.catalog + .resolve_selection(Some(model), explicit_provider, &self.eligible_providers) }?; Ok((selected.model, selected.provider)) } @@ -378,7 +374,7 @@ enabled = true Arc::clone(&catalog), HashSet::from([ProviderId::new("openrouter")]), ) - .with_fallback_providers(Some(catalog.all_provider_ids())) + .with_catalog_fallback(true) .apply(graph) .unwrap(); diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 44307ca81..8e415c81a 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -4908,7 +4908,7 @@ async fn import_e2e_through_engine() { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), - fallback_providers: None, + catalog_fallback: false, }) .unwrap(); let validated = validate(transformed, catalog.as_ref(), &[]); diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index da9ab96fd..db59dcb98 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -1102,19 +1102,18 @@ impl Catalog { } /// Resolve a selection against a preferred provider snapshot, falling back - /// to a broader eligible set only when the preferred set cannot supply the - /// requested provider or model. + /// to every provider in the catalog only when the preferred set cannot + /// supply the requested provider or model. /// /// This is useful for readiness checks: ready providers remain preferred, /// while a catalog-only offering can still be selected so the caller can /// report why its provider is unavailable. Semantic failures such as an /// unknown provider do not fall back. - pub fn resolve_selection_with_fallback( + pub fn resolve_selection_with_catalog_fallback( &self, selector: Option<&str>, explicit_provider: Option<&ProviderId>, preferred_providers: &HashSet, - fallback_providers: &HashSet, ) -> Result { match self.resolve_selection(selector, explicit_provider, preferred_providers) { Ok(selected) => Ok(selected), @@ -1122,7 +1121,7 @@ impl Catalog { ModelSelectionError::ProviderUnavailable { .. } | ModelSelectionError::NoEligibleOffering { .. } | ModelSelectionError::NoDefaultModel { .. }, - ) => self.resolve_selection(selector, explicit_provider, fallback_providers), + ) => self.resolve_selection(selector, explicit_provider, &self.all_provider_ids()), Err(error) => Err(error), } } @@ -4021,30 +4020,19 @@ adapter = "openai_compatible" let openai = ProviderId::openai(); let openrouter = ProviderId::new("openrouter"); let ready = HashSet::from([openrouter.clone()]); - let catalog_providers = HashSet::from([openai.clone(), openrouter.clone()]); let shared = catalog - .resolve_selection_with_fallback(Some("portable"), None, &ready, &catalog_providers) + .resolve_selection_with_catalog_fallback(Some("portable"), None, &ready) .unwrap(); assert_eq!(shared.provider, openrouter); let pinned = catalog - .resolve_selection_with_fallback( - Some("portable"), - Some(&openai), - &ready, - &catalog_providers, - ) + .resolve_selection_with_catalog_fallback(Some("portable"), Some(&openai), &ready) .unwrap(); assert_eq!(pinned.provider, openai); let unknown = catalog - .resolve_selection_with_fallback( - Some("provider-private-preview"), - None, - &ready, - &catalog_providers, - ) + .resolve_selection_with_catalog_fallback(Some("provider-private-preview"), None, &ready) .unwrap(); assert_eq!(unknown.provider, ProviderId::new("openrouter")); assert_eq!(unknown.model, "provider-private-preview");