mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-09 22:33:37 +00:00
Merge pull request #615 from fabro-sh/fix/preflight-ready-provider-routing
Fix preflight provider routing
This commit is contained in:
commit
856101bbd7
14 changed files with 462 additions and 96 deletions
|
|
@ -34,14 +34,15 @@ 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_ready_providers,
|
||||
};
|
||||
use fabro_workflow::pipeline::Validated;
|
||||
use fabro_workflow::run_materialization::materialize_run;
|
||||
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;
|
||||
use tokio::time;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::interp::process_env_var;
|
||||
use crate::server::AppState;
|
||||
|
|
@ -196,14 +197,34 @@ pub(crate) fn validate_prepared_manifest_with_vars(
|
|||
catalog: Arc<Catalog>,
|
||||
vars: HashMap<String, String>,
|
||||
) -> Result<Validated, WorkflowError> {
|
||||
validate(ValidateInput {
|
||||
validate(manifest_validate_input(prepared, catalog, vars))
|
||||
}
|
||||
|
||||
pub(crate) fn validate_prepared_manifest_for_preflight(
|
||||
prepared: &PreparedManifest,
|
||||
catalog: Arc<Catalog>,
|
||||
vars: HashMap<String, String>,
|
||||
ready_providers: &[ProviderId],
|
||||
) -> Result<Validated, WorkflowError> {
|
||||
validate_with_ready_providers(
|
||||
manifest_validate_input(prepared, catalog, vars),
|
||||
ready_providers,
|
||||
)
|
||||
}
|
||||
|
||||
fn manifest_validate_input(
|
||||
prepared: &PreparedManifest,
|
||||
catalog: Arc<Catalog>,
|
||||
vars: HashMap<String, String>,
|
||||
) -> 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(
|
||||
|
|
@ -238,8 +259,10 @@ pub(crate) async fn run_preflight(
|
|||
state: &AppState,
|
||||
prepared: &PreparedManifest,
|
||||
validated: &Validated,
|
||||
llm_result: Result<LlmClientResult>,
|
||||
) -> 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, llm_result).await?;
|
||||
let preflight_ok = !validated.has_errors() && checks_ok;
|
||||
Ok((
|
||||
preflight_response(
|
||||
|
|
@ -458,6 +481,7 @@ async fn build_preflight_report(
|
|||
state: &AppState,
|
||||
prepared: &PreparedManifest,
|
||||
validated: &Validated,
|
||||
llm_result: Result<LlmClientResult>,
|
||||
) -> Result<(CheckReport, bool)> {
|
||||
let graph = validated.graph();
|
||||
let mut checks = base_preflight_checks(prepared, graph);
|
||||
|
|
@ -475,22 +499,23 @@ 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::<Vec<_>>();
|
||||
let materialized = materialize_run(
|
||||
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(),
|
||||
&enabled_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);
|
||||
|
|
@ -553,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,
|
||||
)
|
||||
|
|
@ -1029,17 +1055,11 @@ struct PendingModelProbe {
|
|||
async fn run_llm_check(
|
||||
checks: &mut Vec<CheckResult>,
|
||||
graph: &Graph,
|
||||
settings: &RunNamespace,
|
||||
model: &str,
|
||||
default_provider: &str,
|
||||
catalog: &Catalog,
|
||||
llm_result: Result<LlmClientResult>,
|
||||
) -> 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 mut model_providers = std::collections::BTreeSet::new();
|
||||
let mut has_llm_nodes = false;
|
||||
|
||||
|
|
@ -1050,24 +1070,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 {
|
||||
|
|
@ -1375,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::*;
|
||||
|
||||
|
|
@ -1470,6 +1474,100 @@ 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<crate::server::AppState> {
|
||||
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<crate::server::AppState>,
|
||||
model: &str,
|
||||
) -> (types::PreflightResponse, bool) {
|
||||
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"),
|
||||
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_for_preflight(
|
||||
&prepared,
|
||||
state.catalog(),
|
||||
HashMap::new(),
|
||||
&ready_providers,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
run_preflight(state.as_ref(), &prepared, &validated, llm_result)
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
async fn resolve_and_run_preflight(
|
||||
state: &AppState,
|
||||
prepared: &PreparedManifest,
|
||||
validated: &Validated,
|
||||
) -> Result<(types::PreflightResponse, bool)> {
|
||||
let llm_result = state.resolve_llm_client().await;
|
||||
run_preflight(state, prepared, validated, llm_result).await
|
||||
}
|
||||
|
||||
fn manifest_workflow() -> types::ManifestWorkflow {
|
||||
types::ManifestWorkflow {
|
||||
config: None,
|
||||
|
|
@ -2099,7 +2197,7 @@ name = "Control Plane"
|
|||
|
||||
assert!(validated.has_errors());
|
||||
|
||||
let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated)
|
||||
let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -2143,7 +2241,7 @@ 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)
|
||||
let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -2194,7 +2292,7 @@ id = "local"
|
|||
|
||||
assert!(!validated.has_errors());
|
||||
|
||||
let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated)
|
||||
let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -2301,7 +2399,7 @@ id = "daytona"
|
|||
.unwrap();
|
||||
let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
|
||||
|
||||
let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated)
|
||||
let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -2369,7 +2467,7 @@ digraph Demo {
|
|||
.unwrap();
|
||||
let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
|
||||
|
||||
let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated)
|
||||
let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -2390,6 +2488,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();
|
||||
|
|
@ -2466,9 +2638,21 @@ 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)
|
||||
let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated, llm_result)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
|
|
|||
|
|
@ -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<ProviderId> {
|
||||
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<LlmClientResult>, Vec<ProviderId>) {
|
||||
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<ProviderId> {
|
||||
self.resolve_llm_client_with_ready_ids().await.1
|
||||
}
|
||||
|
||||
pub(crate) async fn decorate_run_summary(&self, run: fabro_types::Run) -> fabro_types::Run {
|
||||
|
|
|
|||
|
|
@ -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,10 +825,12 @@ 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, ready_providers) = state.resolve_llm_client_with_ready_ids().await;
|
||||
let mut validated = match run_manifest::validate_prepared_manifest_for_preflight(
|
||||
&prepared,
|
||||
state.catalog(),
|
||||
vars,
|
||||
&ready_providers,
|
||||
) {
|
||||
Ok(validated) => validated,
|
||||
Err(WorkflowError::Parse(_)) => {
|
||||
|
|
@ -847,13 +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).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()
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -312,6 +312,7 @@ fn create_from_source(
|
|||
.filter(|provider| !provider.is_empty())
|
||||
.map(ProviderId::new),
|
||||
&options.configured_providers,
|
||||
false,
|
||||
&options.catalog,
|
||||
)?;
|
||||
|
||||
|
|
@ -336,6 +337,7 @@ pub(super) fn preprocess_and_validate(
|
|||
render_mode: RenderMode,
|
||||
default_provider: Option<ProviderId>,
|
||||
eligible_providers: &[ProviderId],
|
||||
catalog_fallback: bool,
|
||||
catalog: &Arc<Catalog>,
|
||||
) -> Result<Validated, Error> {
|
||||
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(),
|
||||
catalog_fallback,
|
||||
})?;
|
||||
Ok(pipeline::validate(transformed, catalog.as_ref(), &[]))
|
||||
}
|
||||
|
|
@ -579,6 +582,7 @@ reasoning = false
|
|||
RenderMode::Structural,
|
||||
None,
|
||||
&test_provider_ids(),
|
||||
false,
|
||||
&test_catalog(),
|
||||
)
|
||||
.unwrap()
|
||||
|
|
@ -749,6 +753,7 @@ reasoning = false
|
|||
RenderMode::Strict,
|
||||
None,
|
||||
&test_provider_ids(),
|
||||
false,
|
||||
&test_catalog(),
|
||||
);
|
||||
let Err(err) = result else {
|
||||
|
|
@ -787,6 +792,7 @@ reasoning = false
|
|||
RenderMode::Strict,
|
||||
None,
|
||||
&test_provider_ids(),
|
||||
false,
|
||||
&test_catalog(),
|
||||
);
|
||||
let Err(err) = result else {
|
||||
|
|
|
|||
|
|
@ -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_ready_providers};
|
||||
|
||||
pub use crate::pipeline::{LlmSpec, SandboxEnvSpec};
|
||||
pub use crate::transforms::RenderMode;
|
||||
|
|
|
|||
|
|
@ -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)?;
|
||||
|
|
|
|||
|
|
@ -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<Validated, Error> {
|
||||
let eligible_providers = input
|
||||
.catalog
|
||||
.all_provider_ids()
|
||||
.into_iter()
|
||||
.collect::<Vec<_>>();
|
||||
validate_with_eligible_providers(input, &eligible_providers, false)
|
||||
}
|
||||
|
||||
/// 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,
|
||||
ready_providers: &[ProviderId],
|
||||
) -> Result<Validated, Error> {
|
||||
validate_with_eligible_providers(input, ready_providers, true)
|
||||
}
|
||||
|
||||
fn validate_with_eligible_providers(
|
||||
input: ValidateInput,
|
||||
eligible_providers: &[ProviderId],
|
||||
catalog_fallback: bool,
|
||||
) -> Result<Validated, Error> {
|
||||
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::<Vec<_>>();
|
||||
|
||||
preprocess_and_validate(
|
||||
&resolved.raw_source,
|
||||
|
|
@ -60,7 +78,8 @@ pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
|
|||
.as_deref()
|
||||
.filter(|provider| !provider.is_empty())
|
||||
.map(fabro_model::ProviderId::new),
|
||||
&eligible_providers,
|
||||
eligible_providers,
|
||||
catalog_fallback,
|
||||
&input.catalog,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result<Transform
|
|||
options.eligible_providers.clone(),
|
||||
)
|
||||
.with_default_provider(options.default_provider.clone())
|
||||
.with_catalog_fallback(options.catalog_fallback)
|
||||
.apply(graph)?;
|
||||
|
||||
// Custom transforms
|
||||
|
|
@ -120,6 +121,7 @@ mod tests {
|
|||
catalog: test_catalog(),
|
||||
default_provider: None,
|
||||
eligible_providers: Catalog::builtin().all_provider_ids(),
|
||||
catalog_fallback: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -184,6 +186,7 @@ mod tests {
|
|||
catalog: test_catalog(),
|
||||
default_provider: None,
|
||||
eligible_providers: Catalog::builtin().all_provider_ids(),
|
||||
catalog_fallback: false,
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -238,6 +241,7 @@ mod tests {
|
|||
catalog: test_catalog(),
|
||||
default_provider: None,
|
||||
eligible_providers: Catalog::builtin().all_provider_ids(),
|
||||
catalog_fallback: false,
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
|
|
@ -370,6 +374,7 @@ mod tests {
|
|||
catalog: test_catalog(),
|
||||
default_provider: None,
|
||||
eligible_providers: Catalog::builtin().all_provider_ids(),
|
||||
catalog_fallback: false,
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
|
|
|
|||
|
|
@ -332,6 +332,9 @@ pub struct TransformOptions {
|
|||
pub catalog: Arc<fabro_model::Catalog>,
|
||||
pub default_provider: Option<ProviderId>,
|
||||
pub eligible_providers: HashSet<ProviderId>,
|
||||
/// 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.
|
||||
|
|
|
|||
|
|
@ -51,6 +51,7 @@ mod tests {
|
|||
catalog: std::sync::Arc::clone(&catalog),
|
||||
default_provider: None,
|
||||
eligible_providers: catalog.all_provider_ids(),
|
||||
catalog_fallback: false,
|
||||
})
|
||||
.unwrap();
|
||||
validate(transformed, catalog.as_ref(), &[])
|
||||
|
|
|
|||
|
|
@ -9,10 +9,32 @@ use fabro_types::settings::run::RunGoal;
|
|||
use crate::error::Error;
|
||||
|
||||
pub fn materialize_run(
|
||||
mut settings: WorkflowSettings,
|
||||
settings: WorkflowSettings,
|
||||
graph: &Graph,
|
||||
catalog: &Catalog,
|
||||
configured_providers: &[ProviderId],
|
||||
) -> Result<WorkflowSettings, Error> {
|
||||
materialize_run_with_eligible_providers(settings, graph, catalog, configured_providers, false)
|
||||
}
|
||||
|
||||
/// 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,
|
||||
ready_providers: &[ProviderId],
|
||||
) -> Result<WorkflowSettings, Error> {
|
||||
materialize_run_with_eligible_providers(settings, graph, catalog, ready_providers, true)
|
||||
}
|
||||
|
||||
fn materialize_run_with_eligible_providers(
|
||||
mut settings: WorkflowSettings,
|
||||
graph: &Graph,
|
||||
catalog: &Catalog,
|
||||
eligible_providers: &[ProviderId],
|
||||
catalog_fallback: bool,
|
||||
) -> Result<WorkflowSettings, Error> {
|
||||
let configured_model = settings.run.model.name.take();
|
||||
let configured_provider = settings.run.model.provider.take();
|
||||
|
|
@ -29,9 +51,14 @@ 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::<HashSet<_>>();
|
||||
let (resolved_model, resolved_provider) =
|
||||
resolve_run_model(catalog, &eligible, model.as_deref(), provider.as_deref())?;
|
||||
let eligible = eligible_providers.iter().cloned().collect::<HashSet<_>>();
|
||||
let (resolved_model, resolved_provider) = resolve_run_model(
|
||||
catalog,
|
||||
&eligible,
|
||||
model.as_deref(),
|
||||
provider.as_deref(),
|
||||
catalog_fallback,
|
||||
)?;
|
||||
|
||||
settings.run.model.name = Some(resolved_model);
|
||||
settings.run.model.provider = Some(resolved_provider.into_inner());
|
||||
|
|
@ -60,10 +87,15 @@ pub(crate) fn resolve_run_model(
|
|||
eligible: &HashSet<ProviderId>,
|
||||
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))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ pub struct ModelResolutionTransform {
|
|||
catalog: Arc<Catalog>,
|
||||
default_provider: Option<ProviderId>,
|
||||
eligible_providers: HashSet<ProviderId>,
|
||||
catalog_fallback: bool,
|
||||
}
|
||||
|
||||
impl ModelResolutionTransform {
|
||||
|
|
@ -23,6 +24,7 @@ impl ModelResolutionTransform {
|
|||
catalog,
|
||||
default_provider: None,
|
||||
eligible_providers,
|
||||
catalog_fallback: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -32,6 +34,7 @@ impl ModelResolutionTransform {
|
|||
catalog,
|
||||
default_provider: None,
|
||||
eligible_providers,
|
||||
catalog_fallback: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -41,16 +44,29 @@ impl ModelResolutionTransform {
|
|||
self
|
||||
}
|
||||
|
||||
/// When enabled, provider-readiness selection failures fall back to the
|
||||
/// full catalog instead of erroring.
|
||||
#[must_use]
|
||||
pub fn with_catalog_fallback(mut self, catalog_fallback: bool) -> Self {
|
||||
self.catalog_fallback = catalog_fallback;
|
||||
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 = if self.catalog_fallback {
|
||||
self.catalog.resolve_selection_with_catalog_fallback(
|
||||
Some(model),
|
||||
explicit_provider,
|
||||
&self.eligible_providers,
|
||||
)
|
||||
} else {
|
||||
self.catalog
|
||||
.resolve_selection(Some(model), explicit_provider, &self.eligible_providers)
|
||||
}?;
|
||||
Ok((selected.model, selected.provider))
|
||||
}
|
||||
}
|
||||
|
|
@ -326,6 +342,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_catalog_fallback(true)
|
||||
.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");
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
catalog_fallback: false,
|
||||
})
|
||||
.unwrap();
|
||||
let validated = validate(transformed, catalog.as_ref(), &[]);
|
||||
|
|
|
|||
|
|
@ -1101,6 +1101,31 @@ impl Catalog {
|
|||
}
|
||||
}
|
||||
|
||||
/// Resolve a selection against a preferred provider snapshot, falling back
|
||||
/// 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_catalog_fallback(
|
||||
&self,
|
||||
selector: Option<&str>,
|
||||
explicit_provider: Option<&ProviderId>,
|
||||
preferred_providers: &HashSet<ProviderId>,
|
||||
) -> Result<SelectedModel, ModelSelectionError> {
|
||||
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, &self.all_provider_ids()),
|
||||
Err(error) => Err(error),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn is_model_selector(&self, selector: &str) -> bool {
|
||||
self.candidate_indices(selector).is_some()
|
||||
|
|
@ -4048,6 +4073,30 @@ 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 shared = catalog
|
||||
.resolve_selection_with_catalog_fallback(Some("portable"), None, &ready)
|
||||
.unwrap();
|
||||
assert_eq!(shared.provider, openrouter);
|
||||
|
||||
let pinned = catalog
|
||||
.resolve_selection_with_catalog_fallback(Some("portable"), Some(&openai), &ready)
|
||||
.unwrap();
|
||||
assert_eq!(pinned.provider, openai);
|
||||
|
||||
let unknown = catalog
|
||||
.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");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_builtin_selector_uses_readiness_priority_and_explicit_pins() {
|
||||
let catalog = portable_model_catalog();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue