diff --git a/lib/apps/fabro-server/src/petri_check.rs b/lib/apps/fabro-server/src/petri_check.rs index 973b1f171..a08cd516f 100644 --- a/lib/apps/fabro-server/src/petri_check.rs +++ b/lib/apps/fabro-server/src/petri_check.rs @@ -28,11 +28,12 @@ use lithos_llm::catalog::ProviderId; pub(crate) const NO_READY_PROVIDER_RULE: &str = "fabro.model.no_ready_provider"; /// The launch Fabro binds around the settings: the run's model and provider -/// below them, and the environment the run selected and the goal the run -/// resolved above them. When the settings name neither model nor provider, the -/// default offering of the eligible providers is bound as the launch model -/// alone: a node that names no model runs on it, and a node that names a model -/// the catalog lacks stays unqualified, so Petri's admission refuses it. +/// as the default below them, and the environment the run selected and the +/// goal the run resolved above them. When the settings name neither model nor +/// provider, the default offering of the eligible providers is bound as the +/// default model alone: a node that names no model runs on it, and a node that +/// names a model the catalog lacks stays unqualified, so Petri's admission +/// refuses it. pub(crate) fn launch( catalog: &Catalog, settings: &WorkflowSettings, @@ -50,8 +51,8 @@ pub(crate) fn launch( .map(|offering| offering.model.id().to_string()) }); Launch { - model, - provider: settings.run.model.provider.clone(), + default_model: model, + default_provider: settings.run.model.provider.clone(), environment: environment.map(str::to_owned), goal: launch_goal(settings), repository, @@ -83,8 +84,8 @@ fn launch_goal(settings: &WorkflowSettings) -> Option { /// name, for a check away from the server. pub(crate) fn launch_without_catalog(settings: &WorkflowSettings) -> Launch { Launch { - model: settings.run.model.name.clone(), - provider: settings.run.model.provider.clone(), + default_model: settings.run.model.name.clone(), + default_provider: settings.run.model.provider.clone(), environment: None, goal: launch_goal(settings), repository: None, @@ -92,15 +93,16 @@ pub(crate) fn launch_without_catalog(settings: &WorkflowSettings) -> Launch { } } -/// Preserve explicit model flags separately from defaults resolved from the -/// workflow and server, so they can outrank file layers during admission. +/// The run's explicit model flags (`--model`, `--provider`), kept apart from +/// the default the settings resolved, so they outrank the file layers and +/// the graph's defaults during admission. pub(crate) fn with_model_overrides( mut launch: Launch, overrides: Option<&RunModelLayer>, ) -> Launch { if let Some(overrides) = overrides { - launch.model_override.clone_from(&overrides.name); - launch.provider_override.clone_from(&overrides.provider); + launch.model.clone_from(&overrides.name); + launch.provider.clone_from(&overrides.provider); } launch } diff --git a/lib/components/fabro-petri/src/check.rs b/lib/components/fabro-petri/src/check.rs index 24e9ab63a..1d1ed66e7 100644 --- a/lib/components/fabro-petri/src/check.rs +++ b/lib/components/fabro-petri/src/check.rs @@ -11,10 +11,11 @@ //! path the map holds the file under. //! //! The launch binds the compile variables the Fabro frontend reads: -//! `petri.launch_model` and `petri.launch_provider` as the model default -//! below every file layer, `petri.model_override` and `petri.provider_override` -//! as explicit selections above file and graph defaults but below node and -//! stylesheet choices, `petri.launch_environment` as the environment +//! `petri.launch_model` and `petri.launch_provider` as the model the run's +//! flags ask for, over every file layer and the graph's defaults but below a +//! model a node names itself, `petri.default_model` and +//! `petri.default_provider` as the run's default model below every file +//! layer, `petri.launch_environment` as the environment //! the run selected over every file layer, `petri.launch_goal` as the goal //! the run resolved over every file layer and the graph's own, and //! `petri.repository` as the repository the root `start` stage checks out. A @@ -28,9 +29,8 @@ use std::path::PathBuf; use petri_frontend_attractor::kinds::{AGENT_KIND, PROMPT_KIND}; use petri_runtime::LoadError; use petri_runtime::frontend::{ - self, CompileInputs, LAUNCH_ENVIRONMENT_VAR, LAUNCH_GOAL_VAR, LAUNCH_MODEL_VAR, - LAUNCH_PROVIDER_VAR, MODEL_OVERRIDE_VAR, MapFiles, PROVIDER_OVERRIDE_VAR, REPOSITORY_VAR, - Severity, + self, CompileInputs, DEFAULT_MODEL_VAR, DEFAULT_PROVIDER_VAR, LAUNCH_ENVIRONMENT_VAR, + LAUNCH_GOAL_VAR, LAUNCH_MODEL_VAR, LAUNCH_PROVIDER_VAR, MapFiles, REPOSITORY_VAR, Severity, }; use petri_runtime::ir::Graph; use serde::{Deserialize, Serialize}; @@ -68,29 +68,34 @@ impl Bundle { } } -/// What the launch binds around the file layers: the model default below -/// them, the environment selection above them, and the repository. +/// What the launch binds around the file layers: the model the run's flags +/// ask for and the environment selection above them, the run's default model +/// below them, and the repository. #[derive(Clone, Debug, Default)] pub struct Launch { - pub model: Option, - pub provider: Option, - /// Explicit run overrides, above workflow and graph defaults but below - /// node attributes and stylesheets. Kept separate from catalog defaults. - pub model_override: Option, - pub provider_override: Option, + /// The model and provider the run's flags ask for (`--model`, + /// `--provider`), over every file layer and the graph's defaults but + /// below a model a node names itself. + pub model: Option, + pub provider: Option, + /// The run's default model and provider (the settings', else the + /// catalog's default), below every file layer: they fill what nothing + /// else names. + pub default_model: Option, + pub default_provider: Option, /// The environment the run selected, by its id in the server's /// catalog, over every layer's `[run.environment]`, as the intent's /// selection overrides the bundle in Fabro's own resolution; `None` /// leaves the layers to select. - pub environment: Option, + pub environment: Option, /// The goal the run resolved (the intent's override, else the settings' /// `[run] goal` from any layer), over the bundle's `[run] goal` and the /// graph's own `goal`, so the stages execute with the goal the run /// shows; `None` leaves the bundle's layers and the graph to state it. - pub goal: Option, + pub goal: Option, /// The local repository the root `start` stage checks out into the /// workspace; `None` starts the run from an empty workspace. - pub repository: Option, + pub repository: Option, } /// One check: the bundle, the run's inputs and variables, the launch and @@ -228,11 +233,10 @@ fn compile_inputs( .insert(LAUNCH_PROVIDER_VAR.into(), text(&launch.provider)); compile .vars - .insert(MODEL_OVERRIDE_VAR.into(), text(&launch.model_override)); - compile.vars.insert( - PROVIDER_OVERRIDE_VAR.into(), - text(&launch.provider_override), - ); + .insert(DEFAULT_MODEL_VAR.into(), text(&launch.default_model)); + compile + .vars + .insert(DEFAULT_PROVIDER_VAR.into(), text(&launch.default_provider)); if let Some(environment) = &launch.environment { compile.vars.insert( LAUNCH_ENVIRONMENT_VAR.into(), diff --git a/lib/components/fabro-petri/tests/check.rs b/lib/components/fabro-petri/tests/check.rs index bf68adff7..f990bb57a 100644 --- a/lib/components/fabro-petri/tests/check.rs +++ b/lib/components/fabro-petri/tests/check.rs @@ -140,8 +140,8 @@ async fn a_launch_binds_the_repository_and_the_model_default() { inputs: BTreeMap::new(), vars: BTreeMap::new(), launch: Launch { - model: Some("gpt-5.4".to_string()), - provider: None, + default_model: Some("gpt-5.4".to_string()), + default_provider: None, environment: None, goal: None, repository: Some(repository.path().to_path_buf()), @@ -154,7 +154,11 @@ async fn a_launch_binds_the_repository_and_the_model_default() { let admitted = check::check(&request).expect("the command bundle is admitted"); let launch = &admitted.graph.params["fabro.launch"]; - assert_eq!(launch["model"], "gpt-5.4"); + assert_eq!(launch["default_model"], "gpt-5.4"); + assert!( + launch["model"].is_null(), + "the default is not a launch flag: {launch}" + ); assert_eq!( launch["clone"]["repository"], repository.path().to_string_lossy().as_ref() diff --git a/lib/components/fabro-petri/tests/model.rs b/lib/components/fabro-petri/tests/model.rs index 7599826f8..c4dc064f1 100644 --- a/lib/components/fabro-petri/tests/model.rs +++ b/lib/components/fabro-petri/tests/model.rs @@ -67,7 +67,7 @@ async fn a_model_call_authenticates_through_the_vault_and_skills_read_the_home() let graphs = support::admit( &[("workflow.fabro", &workflow), ("workflow.toml", &settings)], Launch { - model: Some(OPENAI_MODEL.to_string()), + default_model: Some(OPENAI_MODEL.to_string()), ..Launch::default() }, &runtime,