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");