diff --git a/docs/public/changelog/2026-08-25.mdx b/docs/public/changelog/2026-08-25.mdx index 7a031fdee..23c085465 100644 --- a/docs/public/changelog/2026-08-25.mdx +++ b/docs/public/changelog/2026-08-25.mdx @@ -1,5 +1,5 @@ --- -title: "Stop workflows when a node fails" +title: "Model stylesheet templates and failure routing" date: "2026-08-25" --- @@ -22,3 +22,11 @@ digraph Build { start -> plan -> implement -> verify -> exit } ``` + +## Model stylesheet templates + +The root graph's `model_stylesheet` now supports MiniJinja templates. A stylesheet can use typed run inputs and server-managed variables through `inputs` and `vars`. Conditions, loops, filters, macros, local values, and static includes use the same template engine as workflow goals and prompts. + +Fabro renders the stylesheet before parsing and applying its rules. Undefined values produce the existing `template_undefined_variable` diagnostic. Offline validation skips stylesheet syntax checks until those values are available, which avoids a second error from incomplete generated stylesheet text. + +Stylesheet templates do not expose `goal`, `env`, or `secrets`. Stylesheets on imported graphs remain ignored and now produce an `imported_model_stylesheet_ignored` warning. diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 5e7a0b8b0..0d99b89bc 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -444,16 +444,19 @@ repo_url = "https://github.com/fabro-sh/fabro" language = "rust" ``` -Inputs can be used in graph `goal` and node `prompt` attributes with `{{ inputs.name }}` syntax: +Inputs can be used in graph `goal`, root `model_stylesheet`, and node `prompt` attributes with `{{ inputs.name }}` syntax: ```dot title="c-i.fabro" digraph CI { - graph [goal="Run tests for {{ inputs.repo_name }}"] + graph [ + goal="Run tests for {{ inputs.repo_name }}", + model_stylesheet="{% if inputs.language == 'rust' %}* { reasoning_effort: high; }{% endif %}" + ] test [label="Test", prompt="Clone {{ inputs.repo_url }} and run the {{ inputs.language }} test suite."] } ``` -Inputs cannot parameterize workflow structure, file references such as node IDs, edges, `import` paths, `@file` paths, or child workflow paths, or any attribute besides `prompt` and `goal` — other attributes such as `script` and `label` are literal text. +Inputs cannot parameterize workflow structure, file references such as node IDs, edges, `import` paths, `@file` paths, or child workflow paths, or any full-template attribute besides `prompt`, `goal`, and the root `model_stylesheet`. Command `script` supports only simple value substitution. Other attributes such as `label` are literal text. If a workflow template references an undefined input like `{{ inputs.langauge }}`, `fabro validate` reports a warning. Run-style commands promote that diagnostic to an error before creating or starting a run. diff --git a/docs/public/reference/dot-language.mdx b/docs/public/reference/dot-language.mdx index ac04faeed..3642fa001 100644 --- a/docs/public/reference/dot-language.mdx +++ b/docs/public/reference/dot-language.mdx @@ -75,7 +75,7 @@ rankdir=LR |---|---|---| | `goal` | String | Workflow objective — guides agent behavior | | `rankdir` | Identifier | Layout direction: `LR` (left-to-right) or `TB` (top-to-bottom) | -| `model_stylesheet` | String | CSS-like rules for model assignment (see [Model Stylesheets](/workflows/stylesheets)) | +| `model_stylesheet` | String | CSS-like rules for model assignment. The root value supports a MiniJinja template with `inputs` and `vars` (see [Model Stylesheets](/workflows/stylesheets)) | | `default_max_retries` | Integer | Default retry count for all nodes (default: 0) | | `on_failure` | String | Failed-node routing policy: `route` (default) or `exit` | | `retry_target` | String | Default node ID to jump to on retry | diff --git a/docs/public/workflows/stylesheets.mdx b/docs/public/workflows/stylesheets.mdx index e23a224c3..9a335dc3b 100644 --- a/docs/public/workflows/stylesheets.mdx +++ b/docs/public/workflows/stylesheets.mdx @@ -33,10 +33,70 @@ digraph Example { ``` In this example: + - **spec** gets Haiku (matches `*`) - **implement** and **test** get Sonnet with high reasoning (match `.coding`) - **review** gets Gemini Pro (matches `#review`) +## Template stylesheets + +The root graph's `model_stylesheet` is a [MiniJinja template](/workflows/variables). It can read typed run inputs and server-managed variables through `inputs` and `vars`: + +```dot title="variable-effort.fabro" +digraph Review { + graph [ + model_stylesheet=" + * { reasoning_effort: low; } + + {% if inputs.effort == 'deep' %} + .variable-effort { reasoning_effort: high; } + {% elif inputs.effort == 'balanced' %} + .variable-effort { reasoning_effort: medium; } + {% endif %} + " + ] + + triage [prompt="Triage the change"] + review [prompt="Review the change", class="variable-effort"] +} +``` + +Stylesheet templates support expressions, conditionals, loops, filters, macros, `{% set %}`, and normal local values such as `loop`. They do not expose `goal`, `env`, or `secrets`. + +Fabro renders a stylesheet once. If an input or variable contains `{{ ... }}` or `{% ... %}`, that text stays literal. Fabro does not render it again. + +Template output is not escaped as stylesheet syntax. Map user-facing choices to fixed declarations instead of inserting unrestricted text directly: + +```dot +model_stylesheet=" + {% set efforts = {'quick': 'low', 'thorough': 'high'} %} + .review { reasoning_effort: {{ efforts[inputs.review_mode] }}; } +" +``` + +Use single quotes inside MiniJinja expressions when possible. A double quote must follow normal DOT string escaping because the surrounding graph attribute uses double quotes. MiniJinja braces need no extra escaping inside the quoted DOT attribute. + +Static template includes are supported and resolve relative to the workflow template root: + +```dot +graph [model_stylesheet="{% include 'styles/models.partial' %}"] +``` + +Include paths must be literal. Dynamic or root-escaping include paths fail validation. `model_stylesheet` does not support the `@file` shorthand. + +Fabro uses this order: + +1. Parse the DOT source. +2. Expand workflow imports and supported file references. +3. Render the root `model_stylesheet` with `{ inputs, vars }`. +4. Parse and apply the rendered stylesheet. +5. Resolve model and provider selectors. +6. Validate the transformed graph. + +A `model_stylesheet` on an imported graph is ignored and produces an `imported_model_stylesheet_ignored` warning. Put the stylesheet on the root graph. A root stylesheet can target imported nodes by their generated IDs, classes, or shapes. + +If an input or variable is unavailable, `fabro validate` reports `template_undefined_variable`. It skips stylesheet syntax and model checks for that validation pass. Run-style commands treat the same diagnostic as an error before they create or start a run. + ## Selectors Each rule starts with a selector that determines which nodes it applies to: @@ -60,7 +120,7 @@ This node matches both `.coding` and `.critical` rules. ## Properties -Stylesheets support four properties: +Stylesheets support five properties: | Property | Description | Example | |---|---|---| diff --git a/docs/public/workflows/variables.mdx b/docs/public/workflows/variables.mdx index 6093a6766..6624a8c92 100644 --- a/docs/public/workflows/variables.mdx +++ b/docs/public/workflows/variables.mdx @@ -3,7 +3,7 @@ title: "Variables" description: "Using templates in workflows" --- -Fabro renders `{{ ... }}` templates in exactly two workflow attributes: the graph `goal` and node `prompt`s. A command node's `script` gets narrower treatment — [simple value substitution](#command-node-scripts), not templating. Every other attribute is literal text. +Fabro renders full MiniJinja templates in three workflow attributes: the graph `goal`, the root graph's `model_stylesheet`, and node `prompt`s. A command node's `script` gets narrower treatment — [simple value substitution](#command-node-scripts), not templating. Every other attribute is literal text. ## Template context @@ -15,7 +15,9 @@ Goal templates can reference inputs and server-managed variables. Prompt templat | `{{ inputs.name }}` | A value from `[run.inputs]`, optionally overridden by CLI input flags | | `{{ vars.NAME }}` | A server-managed variable snapshotted when the run is created | -Secrets are **not** available in goal or prompt templates. Use `{{ secrets.NAME }}` only in the configuration fields that support run-boundary interpolation. +The root `model_stylesheet` receives only `inputs` and `vars`. It does not receive `goal`. See [Model Stylesheets](/workflows/stylesheets#template-stylesheets) for examples and output safety guidance. + +Secrets are **not** available in goal, prompt, or model stylesheet templates. Use `{{ secrets.NAME }}` only in the configuration fields that support run-boundary interpolation. ## Run config inputs @@ -36,7 +38,7 @@ repo_url = "https://github.com/fabro-sh/fabro" language = "rust" ``` -These values are available in the graph `goal` and node `prompt` attributes: +These values are available in the graph `goal`, root `model_stylesheet`, and node `prompt` attributes: ```dot title="check.fabro" digraph Check { @@ -51,7 +53,7 @@ digraph Check { } ``` -Other attributes — `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt` or `goal`. +Other attributes — `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt`, `goal`, or `model_stylesheet`. Override individual inputs at run time with repeatable `-I` / `--input` flags: @@ -125,7 +127,7 @@ Use server-managed variables for non-sensitive values that should be shared acro fabro variable set DEPLOY_ENV staging --description "Deployment target" ``` -Run configuration strings, graph goals, and node prompts can reference these values with `{{ vars.NAME }}`: +Run configuration strings, graph goals, root model stylesheets, and node prompts can reference these values with `{{ vars.NAME }}`: ```toml title="workflow.toml" _version = 1 @@ -171,9 +173,10 @@ Fabro keeps workflow structure static and renders workflow templates once: 2. Literal `import`, `@file`, graph-goal file, and child-workflow references are resolved. 3. The graph `goal` is rendered with the `{ inputs, vars }` context. 4. Node `prompt` attributes are rendered with the `{ goal, inputs, vars }` context. -5. Node `script` attributes have their `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` values substituted. +5. The root `model_stylesheet` is rendered with the `{ inputs, vars }` context, then parsed and applied. +6. Node `script` attributes have their `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` values substituted. -Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt` and `goal` — and `script`, which takes value substitution rather than templates. +Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt`, `goal`, and the root `model_stylesheet` — and `script`, which takes value substitution rather than templates. Command `stdin_source` values are literal context keys. Fabro resolves them at stage execution time, after upstream nodes have updated the workflow context. @@ -188,9 +191,9 @@ In a `script`, an undefined value records the same diagnostic but leaves the tok ## Template includes -Prompt and goal templates support static MiniJinja loader dependencies such as `{% include "partial.md" %}`. Includes are resolved relative to the template file being rendered and can be nested. +Prompt, goal, and root model stylesheet templates support static MiniJinja loader dependencies such as `{% include "partial.md" %}`. Includes are resolved relative to the template file being rendered and can be nested. -Fabro discovers those static dependencies while building the run manifest so sandbox providers receive every required prompt file. Dynamic loader expressions such as `{% include inputs.partial %}` are rejected; use a literal include path and choose content with normal template conditionals instead. +Fabro discovers those static dependencies while building the run manifest so sandbox providers receive every required template file. Dynamic loader expressions such as `{% include inputs.partial %}` are rejected; use a literal include path and choose content with normal template conditionals instead. ## Escaping diff --git a/lib/apps/fabro-cli/tests/it/cmd/preflight.rs b/lib/apps/fabro-cli/tests/it/cmd/preflight.rs index 4a2e44c8a..b76246318 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/preflight.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/preflight.rs @@ -76,9 +76,9 @@ fn preflight_rejects_unbound_template_inputs() { Goal: Demo error: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable) - fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=` + fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` error: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=` + fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` × Validation failed "); } diff --git a/lib/apps/fabro-cli/tests/it/cmd/validate.rs b/lib/apps/fabro-cli/tests/it/cmd/validate.rs index 1f67ebe3f..17fe31ac4 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/validate.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/validate.rs @@ -199,9 +199,27 @@ fn bare_fabro_with_unbound_inputs_validates_structurally_with_warning() { Workflow: TemplatedUnbound (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound.fabro warning: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable) - fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=` + fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` warning: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=` + fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` + Validation: OK + "); +} + +#[test] +fn unbound_model_stylesheet_input_warns_without_css_error() { + let context = test_context!(); + let mut cmd = context.validate(); + cmd.arg(fixture("model_stylesheet_unbound.fabro")); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + ----- stderr ----- + Workflow: ModelStylesheetUnbound (3 nodes, 2 edges) + Graph: [FIXTURES]/model_stylesheet_unbound.fabro + warning: [FIXTURES]/model_stylesheet_unbound.fabro:4:38: undefined template variable `inputs.effort` in graph attribute `model_stylesheet` (template_undefined_variable) + fix: bind `effort` via `[run.inputs]` in workflow.toml, or pass `--input effort=` Validation: OK "); } @@ -224,7 +242,7 @@ fn bare_fabro_with_unbound_inputs_in_imported_prompt_validates_structurally_with Workflow: TemplatedUnboundImported (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound_imported/workflow.fabro warning: [FIXTURES]/templated_unbound_imported/work.md:1:12: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable) - fix: bind `inputs.app_dir` via `[run.inputs]` in workflow.toml, or pass `--input inputs.app_dir=` + fix: bind `app_dir` via `[run.inputs]` in workflow.toml, or pass `--input app_dir=` Validation: OK "); } @@ -246,7 +264,7 @@ fn bare_fabro_with_unbound_inputs_in_template_partial_validates_structurally_wit Workflow: TemplatedUnboundPartial (3 nodes, 2 edges) Graph: [FIXTURES]/templated_unbound_partial/workflow.fabro warning: [FIXTURES]/templated_unbound_partial/test-include.partial.md:1:4: undefined template variable `inputs.hello` in node `test_imported_include` attribute `prompt` [node: test_imported_include] (template_undefined_variable) - fix: bind `inputs.hello` via `[run.inputs]` in workflow.toml, or pass `--input inputs.hello=` + fix: bind `hello` via `[run.inputs]` in workflow.toml, or pass `--input hello=` Validation: OK "); } diff --git a/lib/components/fabro-manifest/src/workflow_bundler.rs b/lib/components/fabro-manifest/src/workflow_bundler.rs index bd031df0b..e1907c37b 100644 --- a/lib/components/fabro-manifest/src/workflow_bundler.rs +++ b/lib/components/fabro-manifest/src/workflow_bundler.rs @@ -8,9 +8,9 @@ use fabro_config::project::WorkflowLocation; use fabro_config::{EnvironmentDockerfileLayer, EnvironmentImageLayer, SettingsLayer}; use fabro_graphviz::parser; use fabro_template::{ - BundleTemplateStore, FilesystemTemplateStore, GraphReference, GraphReferenceError, - RecordingTemplateStore, TemplateContext, TemplateDependencyClosure, TemplateRenderMode, - TemplateSource, validate_static_reference, visit_graph_references, + BundleTemplateStore, FilesystemTemplateStore, GraphPosition, GraphReference, + GraphReferenceError, RecordingTemplateStore, TemplateContext, TemplateDependencyClosure, + TemplateRenderMode, TemplateSource, validate_static_reference, visit_graph_references, }; use fabro_types::ManifestPath; use fabro_types::graph::ReferenceKind; @@ -87,7 +87,12 @@ impl<'a> WorkflowBundler<'a> { .ok_or_else(|| anyhow!("invalid manifest workflow config path: {}", config.path))?; self.collect_config_dockerfile(&config_path, &config.source, &mut files)?; } - self.collect_workflow_files(&scan, &mut files, &mut visited_imports)?; + self.collect_workflow_files( + &scan, + &mut files, + &mut visited_imports, + GraphPosition::Entrypoint, + )?; self.workflows .insert(dot_key.clone(), types::ManifestWorkflow { @@ -123,6 +128,7 @@ impl<'a> WorkflowBundler<'a> { workflow: &WorkflowScanInput, files: &mut HashMap, visited_imports: &mut HashSet, + position: GraphPosition, ) -> Result<()> { let graph = parser::parse(&workflow.source) .with_context(|| format!("Failed to parse {}", workflow.absolute_dot_path.display()))?; @@ -137,7 +143,7 @@ impl<'a> WorkflowBundler<'a> { let mut imports = Vec::new(); let mut children = Vec::new(); - visit_graph_references(&graph, |reference| -> Result<()> { + visit_graph_references(&graph, position, |reference| -> Result<()> { match reference { GraphReference::GoalFile { reference } => { let bundled = self.collect_bundled_file( @@ -151,15 +157,17 @@ impl<'a> WorkflowBundler<'a> { self.collect_bundled_template_includes(files, &bundled, &workflow_template_root) } GraphReference::GoalInline { content } - | GraphReference::InlinePrompt { content } => self.collect_template_include_files( - files, - TemplateSource::new( - workflow.dot_path.clone(), - workflow_template_root.clone(), - content.to_owned(), + | GraphReference::InlinePrompt { content } + | GraphReference::ModelStylesheetInline { content } => self + .collect_template_include_files( + files, + TemplateSource::new( + workflow.dot_path.clone(), + workflow_template_root.clone(), + content.to_owned(), + ), + Some(&workflow.dot_path), ), - Some(&workflow.dot_path), - ), GraphReference::FileInline { key, reference } => { let bundled = self.collect_bundled_file( files, @@ -212,7 +220,12 @@ impl<'a> WorkflowBundler<'a> { dot_path: imported.path, source: imported_source, }; - self.collect_workflow_files(&imported_scan, files, visited_imports)?; + self.collect_workflow_files( + &imported_scan, + files, + visited_imports, + GraphPosition::Imported, + )?; } } for child in children { @@ -474,6 +487,101 @@ mod tests { assert_eq!(goal.ref_.original, "@goal.md"); } + #[test] + fn root_model_stylesheet_bundles_nested_static_includes() { + let temp = tempfile::tempdir().expect("temp directory should be created"); + let graph = temp.path().join("workflow.fabro"); + write_file( + &graph, + r#"digraph Root { + graph [model_stylesheet="{% include 'styles/base.css' %}"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#, + ); + write_file( + &temp.path().join("styles/base.css"), + "{% include 'nested.css' %}", + ); + write_file( + &temp.path().join("styles/nested.css"), + "* { reasoning_effort: low; }", + ); + + let workflows = bundle_graph(temp.path(), &graph).expect("workflow should bundle"); + let files = &workflows["workflow.fabro"].files; + + assert_eq!( + files["styles/base.css"].content, + "{% include 'nested.css' %}" + ); + assert_eq!( + files["styles/nested.css"].content, + "* { reasoning_effort: low; }" + ); + } + + #[test] + fn root_model_stylesheet_rejects_invalid_includes() { + for template in [ + "{% include 'missing.css' %}", + "{% include inputs.stylesheet %}", + "{% include '../outside.css' %}", + ] { + let temp = tempfile::tempdir().expect("temp directory should be created"); + let graph = temp.path().join("workflow.fabro"); + write_file( + &graph, + &format!( + r#"digraph Root {{ + graph [model_stylesheet="{template}"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }}"#, + ), + ); + + let error = bundle_graph(temp.path(), &graph) + .expect_err("invalid stylesheet include should fail bundling"); + assert!( + error.to_string().contains("template dependencies"), + "template: {template}; error: {error:#}" + ); + } + } + + #[test] + fn imported_model_stylesheet_includes_are_not_bundled() { + let temp = tempfile::tempdir().expect("temp directory should be created"); + let graph = temp.path().join("workflow.fabro"); + write_file( + &graph, + r#"digraph Root { + start [shape=Mdiamond] + child [import="child.fabro"] + exit [shape=Msquare] + start -> child -> exit + }"#, + ); + write_file( + &temp.path().join("child.fabro"), + r#"digraph Child { + graph [model_stylesheet="{% include 'missing.css' %}"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#, + ); + + let workflows = bundle_graph(temp.path(), &graph).expect("workflow should bundle"); + let files = &workflows["workflow.fabro"].files; + + assert!(files.contains_key("child.fabro")); + assert!(!files.contains_key("missing.css")); + } + #[test] fn parse_errors_keep_the_graphviz_error_in_the_source_chain() { let temp = tempfile::tempdir().expect("temp directory should be created"); diff --git a/lib/components/fabro-workflow-version/src/lib.rs b/lib/components/fabro-workflow-version/src/lib.rs index 73a282e79..ff0bd1e1a 100644 --- a/lib/components/fabro-workflow-version/src/lib.rs +++ b/lib/components/fabro-workflow-version/src/lib.rs @@ -14,7 +14,7 @@ use fabro_config::{ }; use fabro_graphviz::parser; use fabro_template::{ - BundleTemplateStore, GraphReference, GraphReferenceError, StaticReferenceError, + BundleTemplateStore, GraphPosition, GraphReference, GraphReferenceError, StaticReferenceError, TemplateDiscoveryError, TemplateSource, discover_static_dependency_closure, validate_static_reference, visit_graph_references, }; @@ -278,8 +278,13 @@ fn validate_graph_closure( path: path.clone(), source, })?; + let position = if &path == version.entrypoint() { + GraphPosition::Entrypoint + } else { + GraphPosition::Imported + }; - visit_graph_references(&graph, |reference| match reference { + visit_graph_references(&graph, position, |reference| match reference { GraphReference::GoalFile { reference } => { let target = resolve_reference(&path, ReferenceKind::GraphGoalFile, reference)?; let content = @@ -287,7 +292,9 @@ fn validate_graph_closure( template_roots.push(&target, content); Ok(()) } - GraphReference::GoalInline { content } | GraphReference::InlinePrompt { content } => { + GraphReference::GoalInline { content } + | GraphReference::InlinePrompt { content } + | GraphReference::ModelStylesheetInline { content } => { template_roots.push(&path, content); Ok(()) } @@ -752,6 +759,69 @@ mod tests { )); } + #[test] + fn validates_root_model_stylesheet_template_closure() { + let version = version_with( + [ + ( + "workflow.fabro", + r#"digraph W { + graph [model_stylesheet="{% include 'styles/base.css' %}"] + }"#, + ), + ("styles/base.css", "{% include 'nested.css' %}"), + ("styles/nested.css", "* { reasoning_effort: low; }"), + ], + [], + ) + .unwrap(); + + assert_eq!(version.version().files().len(), 3); + + for template in [ + "{% include 'missing.css' %}", + "{% include inputs.stylesheet %}", + "{% include '../outside.css' %}", + ] { + let graph = format!(r#"digraph W {{ graph [model_stylesheet="{template}"] }}"#); + let error = ValidatedWorkflowVersion::new( + WorkflowVersion::new( + path("workflow.fabro"), + BTreeMap::from([(path("workflow.fabro"), graph)]), + BTreeMap::default(), + ) + .unwrap(), + ) + .unwrap_err(); + assert!( + matches!(error, WorkflowVersionError::Template { .. }), + "template: {template}; error: {error:?}" + ); + } + } + + #[test] + fn ignores_imported_model_stylesheet_template_closure() { + let version = version_with( + [ + ( + "workflow.fabro", + r#"digraph W { imported [import="child.fabro"] }"#, + ), + ( + "child.fabro", + r#"digraph I { + graph [model_stylesheet="{% include 'missing.css' %}"] + }"#, + ), + ], + [], + ) + .unwrap(); + + assert_eq!(version.version().files().len(), 2); + } + #[test] fn validates_all_inline_graph_roots_that_share_the_graph_path() { let error = version_with( diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index e15016c61..db272baac 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -995,6 +995,50 @@ reasoning = false assert!(validated.has_errors()); } + #[test] + fn unknown_input_in_model_stylesheet_warns_then_errors_at_run_create() { + let dot = r#"digraph Test { + graph [model_stylesheet="* { reasoning_effort: {{ inputs.effort }}; }"] + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + work [label="Work", prompt="Do work"] + start -> work -> exit + }"#; + let mut validated = validate_dot(dot, WorkflowSettings::default()); + + let diagnostic = validated + .diagnostics() + .iter() + .find(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) + .expect("expected a template_undefined_variable diagnostic"); + assert_eq!(diagnostic.severity, Severity::Warning); + assert!( + diagnostic + .message + .contains("graph attribute `model_stylesheet`"), + "message: {}", + diagnostic.message + ); + assert!( + validated + .diagnostics() + .iter() + .all(|diagnostic| diagnostic.rule != "stylesheet_syntax") + ); + + validated.promote_template_undefined_variables_to_errors(); + assert!(validated.has_errors()); + assert_eq!( + validated + .diagnostics() + .iter() + .find(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) + .unwrap() + .severity, + Severity::Error + ); + } + #[test] fn vars_resolve_in_command_script_through_create_pipeline() { let dot = r#"digraph Test { diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index 94ac33802..542d52c43 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -3,8 +3,8 @@ use std::sync::Arc; use super::types::{Parsed, TransformOptions, Transformed}; use crate::error::Error; use crate::transforms::{ - FileInliningTransform, ImportTransform, ScriptInterpolationTransform, - StylesheetApplicationTransform, TemplateTransform, Transform, + FileInliningTransform, ImportTransform, ModelStylesheetTemplateTransform, + ScriptInterpolationTransform, StylesheetApplicationTransform, TemplateTransform, Transform, }; /// TRANSFORM phase: apply built-in and custom transforms to a parsed graph. @@ -62,6 +62,23 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result baseline -> selected -> explicit -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([ + ( + "effort".to_string(), + toml::Value::String("deep".to_string()), + ), + ])), + ..transform_options() + }) + .unwrap(); + + assert_eq!( + transformed.graph.nodes["baseline"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + Some("low") + ); + assert_eq!( + transformed.graph.nodes["selected"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + Some("high") + ); + assert_eq!( + transformed.graph.nodes["selected"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("claude-sonnet-5") + ); + assert_eq!( + transformed.graph.nodes["explicit"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + Some("medium") + ); + assert!( + transformed + .diagnostics + .iter() + .all(|diagnostic| diagnostic.rule != "detemplated_attribute"), + "{:?}", + transformed.diagnostics + ); + } + + #[test] + fn transform_renders_model_stylesheet_static_include() { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("styles.partial"), + ".selected { model: sonnet; }", + ); + let source_name = dir.path().join("workflow.fabro"); + let dot = r#"digraph Test { + graph [model_stylesheet="{% include 'styles.partial' %}"] + start [shape=Mdiamond] + selected [prompt="Selected", class="selected"] + exit [shape=Msquare] + start -> selected -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + current_dir: Some(dir.path().to_path_buf()), + file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))), + source_name: Some(source_name.display().to_string()), + ..transform_options() + }) + .unwrap(); + + assert_eq!( + transformed.graph.nodes["selected"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("claude-sonnet-5") + ); + } + + #[test] + fn structural_model_stylesheet_undefined_value_skips_stylesheet_parsing() { + let dot = r#"digraph Test { + graph [model_stylesheet="* { reasoning_effort: {{ inputs.effort }}; }"] + start [shape=Mdiamond] + work [prompt="Work"] + exit [shape=Msquare] + start -> work -> exit + }"#; + let parsed = parse(dot).unwrap(); + let transformed = transform(parsed, &TransformOptions { + render_mode: crate::operations::RenderMode::Structural, + model_resolution: None, + ..transform_options() + }) + .unwrap(); + + assert_eq!(transformed.graph.model_stylesheet(), ""); + assert_eq!( + transformed + .diagnostics + .iter() + .filter(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE) + .count(), + 1 + ); + assert!( + transformed + .diagnostics + .iter() + .all(|diagnostic| diagnostic.rule != "detemplated_attribute") + ); + } + #[test] fn transform_inlines_files_before_variable_expansion() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/components/fabro-workflow/src/pipeline/validate.rs b/lib/components/fabro-workflow/src/pipeline/validate.rs index 32bfaf69c..3c550f5df 100644 --- a/lib/components/fabro-workflow/src/pipeline/validate.rs +++ b/lib/components/fabro-workflow/src/pipeline/validate.rs @@ -30,14 +30,15 @@ pub fn validate( #[cfg(test)] mod tests { + use std::collections::HashMap; + use super::*; use crate::pipeline::parse::parse; use crate::pipeline::transform; use crate::pipeline::types::TransformOptions; - fn run_pipeline(dot: &str) -> Validated { - let parsed = parse(dot).unwrap(); - let transformed = transform::transform(parsed, &TransformOptions { + fn transform_options() -> TransformOptions { + TransformOptions { current_dir: None, file_resolver: None, template_context: fabro_template::TemplateContext::new(), @@ -45,8 +46,12 @@ mod tests { render_mode: crate::operations::RenderMode::Strict, custom_transforms: vec![], model_resolution: None, - }) - .unwrap(); + } + } + + fn run_pipeline(dot: &str) -> Validated { + let parsed = parse(dot).unwrap(); + let transformed = transform::transform(parsed, &transform_options()).unwrap(); validate(transformed, None, &[]) } @@ -106,4 +111,61 @@ mod tests { // Then raise assert!(validated.raise_on_errors().is_err()); } + + #[test] + fn validates_fully_rendered_model_stylesheet_syntax() { + let dot = r#"digraph Test { + graph [model_stylesheet="* { {{ inputs.declaration }} }"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#; + let transformed = transform::transform(parse(dot).unwrap(), &TransformOptions { + template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([ + ( + "declaration".to_string(), + toml::Value::String("garbage garbage".to_string()), + ), + ])), + source_name: Some("workflow.fabro".to_string()), + render_mode: crate::operations::RenderMode::Structural, + ..transform_options() + }) + .unwrap(); + let validated = validate(transformed, None, &[]); + + assert!( + validated + .diagnostics() + .iter() + .any(|diagnostic| diagnostic.rule == "stylesheet_syntax"), + "{:?}", + validated.diagnostics() + ); + } + + #[test] + fn unresolved_model_stylesheet_skips_stylesheet_rules() { + let dot = r#"digraph Test { + graph [model_stylesheet="* { model: {{ vars.MODEL }}; }"] + start [shape=Mdiamond] + exit [shape=Msquare] + start -> exit + }"#; + let transformed = transform::transform(parse(dot).unwrap(), &TransformOptions { + source_name: Some("workflow.fabro".to_string()), + render_mode: crate::operations::RenderMode::Structural, + ..transform_options() + }) + .unwrap(); + let validated = validate(transformed, None, &[]); + + assert!(validated.diagnostics().iter().any(|diagnostic| { + diagnostic.rule == "template_undefined_variable" + && diagnostic.message.contains("vars.MODEL") + })); + assert!(validated.diagnostics().iter().all(|diagnostic| { + diagnostic.rule != "stylesheet_syntax" && diagnostic.rule != "stylesheet_model_known" + })); + } } diff --git a/lib/components/fabro-workflow/src/transforms/file_inlining.rs b/lib/components/fabro-workflow/src/transforms/file_inlining.rs index ff599824b..565edf7d8 100644 --- a/lib/components/fabro-workflow/src/transforms/file_inlining.rs +++ b/lib/components/fabro-workflow/src/transforms/file_inlining.rs @@ -24,13 +24,14 @@ pub(crate) fn template_render_store( current_dir: &Path, resolver: Arc, source_name: Option<&str>, - content: &str, ) -> Result { let root = template_root_for_current_dir(current_dir)?; let source_path = template_source_path_for_current_dir(current_dir, source_name, &root)?; let base_dir = template_store_base_dir(current_dir); + // The store's render substitutes the text being rendered, so the source + // carries only its path and template root. Ok(TemplateRenderStore::new( - TemplateSource::new(source_path, root, content.to_owned()), + TemplateSource::new(source_path, root, String::new()), Arc::new(FileResolverTemplateStore::new(base_dir, resolver)), )) } @@ -184,7 +185,6 @@ impl FileInliningTransform { &self.current_dir, Arc::clone(&self.resolver), self.source_name.as_deref(), - &attr_value, )?); let rendered = render_template_for_target( &attr_value, @@ -232,7 +232,6 @@ impl FileInliningTransform { &self.current_dir, Arc::clone(&self.resolver), self.source_name.as_deref(), - goal, )?); let rendered = render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)?; diff --git a/lib/components/fabro-workflow/src/transforms/import.rs b/lib/components/fabro-workflow/src/transforms/import.rs index bab7ee1b1..6317f1778 100644 --- a/lib/components/fabro-workflow/src/transforms/import.rs +++ b/lib/components/fabro-workflow/src/transforms/import.rs @@ -6,7 +6,7 @@ use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_graphviz::parser; use fabro_template::{TemplateContext, validate_static_reference}; use fabro_types::graph::ReferenceKind; -use fabro_validate::Diagnostic; +use fabro_validate::{Diagnostic, Severity}; use super::file_inlining::template_render_store; use super::{FileInliningTransform, Transform}; @@ -45,6 +45,23 @@ enum ImportPrepareError { Soft(String), } +const IMPORTED_MODEL_STYLESHEET_IGNORED_RULE: &str = "imported_model_stylesheet_ignored"; + +fn imported_model_stylesheet_ignored_diagnostic(source_name: &str) -> Diagnostic { + Diagnostic { + rule: IMPORTED_MODEL_STYLESHEET_IGNORED_RULE.to_string(), + severity: Severity::Warning, + message: "imported graph attribute `model_stylesheet` is ignored; only the root graph stylesheet is applied" + .to_string(), + fix: Some( + "move the stylesheet to the root graph; it can target imported nodes by ID, class, or shape" + .to_string(), + ), + source_path: Some(source_name.to_string()), + ..Diagnostic::default() + } +} + impl From for ImportPrepareError { fn from(error: Error) -> Self { Self::Hard(error) @@ -198,6 +215,10 @@ impl ImportTransform { )) })?; + if !graph.model_stylesheet().is_empty() { + diagnostics.push(imported_model_stylesheet_ignored_diagnostic(&source_name)); + } + let import_base_dir = resolved_file .path .parent() @@ -665,7 +686,6 @@ impl ImportTransform { &self.current_dir, Arc::clone(&self.resolver), self.source_name.as_deref(), - graph.goal(), )?); let parent_goal = render_template_for_target( graph.goal(), @@ -822,6 +842,69 @@ mod tests { assert!(!rendered.contains(""), "{rendered}"); } + #[test] + fn imported_model_stylesheets_are_ignored_with_a_warning() { + for stylesheet in [ + "* { reasoning_effort: high; }", + "{% if inputs.deep %}* { reasoning_effort: high; }{% endif %}", + ] { + let dir = tempfile::tempdir().unwrap(); + write_file( + &dir.path().join("child.fabro"), + &format!( + r#"digraph Child {{ + graph [model_stylesheet="{stylesheet}"] + start [shape=Mdiamond] + work [prompt="Do it"] + exit [shape=Msquare] + start -> work -> exit + }}"#, + ), + ); + let graph = parse_graph( + r#"digraph Test { + start [shape=Mdiamond] + child [import="./child.fabro"] + exit [shape=Msquare] + start -> child -> exit + }"#, + ); + + let (graph, diagnostics) = ImportTransform::new( + dir.path().to_path_buf(), + Arc::new(FilesystemFileResolver::new(None)), + TemplateContext::new(), + ) + .apply_with_diagnostics(graph) + .unwrap(); + + assert_eq!( + graph.nodes["child.work"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + None + ); + let warning = diagnostics + .iter() + .find(|diagnostic| diagnostic.rule == IMPORTED_MODEL_STYLESHEET_IGNORED_RULE) + .expect("expected ignored imported stylesheet warning"); + assert!( + warning + .source_path + .as_deref() + .is_some_and(|path| path.ends_with("child.fabro")), + "{warning:?}" + ); + assert!( + diagnostics + .iter() + .all(|diagnostic| diagnostic.rule != "detemplated_attribute"), + "{diagnostics:?}" + ); + } + } + fn basic_import_source() -> &'static str { r#"digraph validate { start [shape=Mdiamond] diff --git a/lib/components/fabro-workflow/src/transforms/mod.rs b/lib/components/fabro-workflow/src/transforms/mod.rs index 8246ee750..d9ec04868 100644 --- a/lib/components/fabro-workflow/src/transforms/mod.rs +++ b/lib/components/fabro-workflow/src/transforms/mod.rs @@ -12,6 +12,7 @@ mod file_inlining; mod import; mod importable_field; mod model_resolution; +mod model_stylesheet_template; mod preamble; pub mod stylesheet; mod stylesheet_application; @@ -20,6 +21,7 @@ pub mod variable_expansion; pub use file_inlining::FileInliningTransform; pub use import::ImportTransform; pub use model_resolution::ModelResolutionTransform; +pub(crate) use model_stylesheet_template::ModelStylesheetTemplateTransform; pub use preamble::PreambleTransform; pub use stylesheet_application::StylesheetApplicationTransform; pub use variable_expansion::{RenderMode, ScriptInterpolationTransform, TemplateTransform}; diff --git a/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs b/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs new file mode 100644 index 000000000..7071091d4 --- /dev/null +++ b/lib/components/fabro-workflow/src/transforms/model_stylesheet_template.rs @@ -0,0 +1,225 @@ +use std::path::PathBuf; +use std::sync::Arc; + +use fabro_graphviz::graph::{AttrValue, Graph}; +use fabro_template::TemplateContext; +use fabro_validate::Diagnostic; + +use super::file_inlining::template_render_store; +use super::variable_expansion::{ + RenderMode, TemplateRenderOutcome, TemplateRenderTarget, render_template_for_target_outcome, +}; +use crate::error::Error; +use crate::file_resolver::FileResolver; + +/// Renders the root graph's `model_stylesheet` with its restricted template +/// context after imports are expanded and before stylesheet parsing. +pub(crate) struct ModelStylesheetTemplateTransform { + pub context: TemplateContext, + pub source_name: Option, + pub source_text: Option, + pub render_mode: RenderMode, + /// Enables `{% include %}` resolution; without it the stylesheet renders + /// from its inline text alone. + pub file_resolution: Option<(PathBuf, Arc)>, +} + +impl ModelStylesheetTemplateTransform { + pub(crate) fn apply_with_diagnostics( + &self, + mut graph: Graph, + ) -> Result<(Graph, Vec), Error> { + let stylesheet = graph.model_stylesheet(); + if stylesheet.is_empty() { + return Ok((graph, Vec::new())); + } + + let mut target = + TemplateRenderTarget::graph_attr(self.source_name.clone(), "model_stylesheet") + .with_source_origin(self.source_text.as_deref(), stylesheet) + .with_restricted_namespace_fix( + "`model_stylesheet` templates expose only `inputs` and `vars`; use one of \ + those values or a MiniJinja local value", + ); + if let Some((current_dir, resolver)) = &self.file_resolution { + target = target.with_template_store(template_render_store( + current_dir, + Arc::clone(resolver), + self.source_name.as_deref(), + )?); + } + + let mut diagnostics = Vec::new(); + let outcome = render_template_for_target_outcome( + stylesheet, + &self.context.for_model_stylesheet(), + self.render_mode, + &target, + &mut diagnostics, + )?; + let rendered = match outcome { + TemplateRenderOutcome::Rendered(rendered) => rendered, + // Do not feed raw or partly rendered MiniJinja source to the + // stylesheet parser during structural validation. + TemplateRenderOutcome::Unresolved => String::new(), + }; + + graph + .attrs + .insert("model_stylesheet".to_string(), AttrValue::String(rendered)); + Ok((graph, diagnostics)) + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use fabro_graphviz::graph::Graph; + use fabro_util::error::collect_chain; + + use super::*; + + fn graph_with_stylesheet(stylesheet: &str) -> Graph { + let mut graph = Graph::new("test"); + graph.attrs.insert( + "model_stylesheet".to_string(), + AttrValue::String(stylesheet.to_string()), + ); + graph + } + + fn transform( + context: TemplateContext, + stylesheet: &str, + render_mode: RenderMode, + ) -> Result<(Graph, Vec), Error> { + ModelStylesheetTemplateTransform { + context, + source_name: Some("workflow.fabro".to_string()), + source_text: Some(format!( + "digraph Test {{ graph [model_stylesheet=\"{stylesheet}\"] }}" + )), + render_mode, + file_resolution: None, + } + .apply_with_diagnostics(graph_with_stylesheet(stylesheet)) + } + + #[test] + fn preserves_static_stylesheet_bytes() { + let stylesheet = "\n * { reasoning_effort: low; }\n"; + + let (graph, diagnostics) = + transform(TemplateContext::new(), stylesheet, RenderMode::Strict).unwrap(); + + assert!(diagnostics.is_empty()); + assert_eq!(graph.model_stylesheet(), stylesheet); + } + + #[test] + fn renders_inputs_vars_control_flow_and_locals_once() { + let stylesheet = r"{% set prefix = '.tier-' %} +{% for effort in inputs.efforts %} +{{ prefix }}{{ loop.index }} { reasoning_effort: {{ effort }}; } +{% endfor %} +.selected { model: {{ vars.MODEL }}; } +.literal { model: {{ inputs.literal }}; }"; + let context = TemplateContext::new() + .with_goal("must stay unavailable") + .with_inputs(HashMap::from([ + ( + "efforts".to_string(), + toml::Value::Array(vec![ + toml::Value::String("low".to_string()), + toml::Value::String("high".to_string()), + ]), + ), + ( + "literal".to_string(), + toml::Value::String("{{ vars.MODEL }}".to_string()), + ), + ])) + .with_vars(HashMap::from([("MODEL".to_string(), "sonnet".to_string())])); + + let (graph, diagnostics) = transform(context, stylesheet, RenderMode::Strict).unwrap(); + + assert!(diagnostics.is_empty()); + assert!( + graph + .model_stylesheet() + .contains(".tier-1 { reasoning_effort: low; }") + ); + assert!( + graph + .model_stylesheet() + .contains(".tier-2 { reasoning_effort: high; }") + ); + assert!( + graph + .model_stylesheet() + .contains(".selected { model: sonnet; }") + ); + assert!( + graph + .model_stylesheet() + .contains(".literal { model: {{ vars.MODEL }}; }") + ); + } + + #[test] + fn structural_undefined_value_clears_stylesheet_and_reports_context() { + for (expression, expected_fix) in [ + ("inputs.effort", "[run.inputs]"), + ("vars.MODEL", "fabro variable set MODEL"), + ("goal", "expose only `inputs` and `vars`"), + ("env.MODEL", "expose only `inputs` and `vars`"), + ("secrets.MODEL", "expose only `inputs` and `vars`"), + ] { + let stylesheet = format!("* {{ model: {{{{ {expression} }}}}; }}"); + let (graph, diagnostics) = transform( + TemplateContext::new().with_goal("hidden"), + &stylesheet, + RenderMode::Structural, + ) + .unwrap(); + + assert_eq!(graph.model_stylesheet(), "", "expression: {expression}"); + assert_eq!(diagnostics.len(), 1, "expression: {expression}"); + assert_eq!(diagnostics[0].rule, "template_undefined_variable"); + assert!( + diagnostics[0] + .message + .contains("graph attribute `model_stylesheet`"), + "{:?}", + diagnostics[0] + ); + assert!( + diagnostics[0] + .fix + .as_deref() + .is_some_and(|fix| fix.contains(expected_fix)), + "{:?}", + diagnostics[0] + ); + } + } + + #[test] + fn syntax_error_preserves_owner_and_source_chain() { + let error = transform( + TemplateContext::new(), + "* { model: {% if %}; }", + RenderMode::Strict, + ) + .unwrap_err(); + let chain = collect_chain(&error).join(": "); + + assert!( + chain.contains("graph attribute `model_stylesheet`"), + "{chain}" + ); + assert!(chain.contains("template syntax error"), "{chain}"); + assert!(chain.contains("workflow.fabro"), "{chain}"); + } +} diff --git a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs index eb33f250f..a5872b2b3 100644 --- a/lib/components/fabro-workflow/src/transforms/variable_expansion.rs +++ b/lib/components/fabro-workflow/src/transforms/variable_expansion.rs @@ -38,12 +38,15 @@ pub enum RenderMode { #[derive(Clone)] pub(crate) struct TemplateRenderTarget { - pub source_name: Option, - pub node_id: Option, - pub edge: Option<(String, String)>, - pub owner: String, - source_origin: Option, - template_store: Option, + pub source_name: Option, + pub node_id: Option, + pub edge: Option<(String, String)>, + pub owner: String, + /// Fix text for undefined variables outside `inputs`/`vars`, set by + /// targets whose template context is a restricted projection. + restricted_namespace_fix: Option, + source_origin: Option, + template_store: Option, } #[derive(Clone)] @@ -83,6 +86,7 @@ impl TemplateRenderTarget { node_id: None, edge: None, owner: format!("graph attribute `{attr_name}`"), + restricted_namespace_fix: None, source_origin: None, template_store: None, } @@ -101,6 +105,7 @@ impl TemplateRenderTarget { node_id: Some(node_id.clone()), edge: None, owner: format!("node `{node_id}` attribute `{attr_name}`"), + restricted_namespace_fix: None, source_origin: None, template_store: None, } @@ -121,6 +126,7 @@ impl TemplateRenderTarget { node_id: None, edge: Some((from.clone(), to.clone())), owner: format!("edge `{from} -> {to}` attribute `{attr_name}`"), + restricted_namespace_fix: None, source_origin: None, template_store: None, } @@ -146,6 +152,12 @@ impl TemplateRenderTarget { self } + #[must_use] + pub(crate) fn with_restricted_namespace_fix(mut self, fix: impl Into) -> Self { + self.restricted_namespace_fix = Some(fix.into()); + self + } + #[must_use] fn template_source_name(&self) -> String { self.source_name @@ -154,6 +166,11 @@ impl TemplateRenderTarget { } } +pub(crate) enum TemplateRenderOutcome { + Rendered(String), + Unresolved, +} + pub(crate) fn render_template_for_target( text: &str, ctx: &TemplateContext, @@ -161,18 +178,34 @@ pub(crate) fn render_template_for_target( target: &TemplateRenderTarget, diagnostics: &mut Vec, ) -> Result { + match render_template_for_target_outcome(text, ctx, render_mode, target, diagnostics)? { + TemplateRenderOutcome::Rendered(rendered) => Ok(rendered), + TemplateRenderOutcome::Unresolved => { + render_template_with_mode(text, ctx, TemplateRenderMode::Lenient, target) + .map_err(|err| template_error_for_target(target, err)) + } + } +} + +pub(crate) fn render_template_for_target_outcome( + text: &str, + ctx: &TemplateContext, + render_mode: RenderMode, + target: &TemplateRenderTarget, + diagnostics: &mut Vec, +) -> Result { match render_mode { RenderMode::Strict => { render_template_with_mode(text, ctx, TemplateRenderMode::Strict, target) + .map(TemplateRenderOutcome::Rendered) .map_err(|err| template_error_for_target(target, err)) } RenderMode::Structural => { match render_template_with_mode(text, ctx, TemplateRenderMode::Strict, target) { - Ok(rendered) => Ok(rendered), + Ok(rendered) => Ok(TemplateRenderOutcome::Rendered(rendered)), Err(err @ TemplateError::UndefinedVariable { .. }) => { diagnostics.push(template_diagnostic(&err, target)); - render_template_with_mode(text, ctx, TemplateRenderMode::Lenient, target) - .map_err(|err| template_error_for_target(target, err)) + Ok(TemplateRenderOutcome::Unresolved) } Err(err) => Err(template_error_for_target(target, err)), } @@ -210,7 +243,6 @@ fn template_error_for_target(target: &TemplateRenderTarget, err: TemplateError) fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> Diagnostic { let expression = error.expression(); - let name = expression.unwrap_or(""); let mut message = match expression { Some(expr) => format!("undefined template variable `{expr}`"), None => "undefined template variable".to_string(), @@ -225,7 +257,7 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> message, node_id: target.node_id.clone(), edge: target.edge.clone(), - fix: Some(input_binding_fix(name)), + fix: Some(template_variable_fix(expression, target)), source_path: location.source_name.or_else(|| target.source_name.clone()), line: location.line, column: location.column, @@ -235,10 +267,36 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> } } +fn template_variable_fix(expression: Option<&str>, target: &TemplateRenderTarget) -> String { + let mut parts = expression.unwrap_or_default().split('.'); + let namespace = parts.next().unwrap_or_default().parse::(); + let name = parts.next().unwrap_or(""); + + match (namespace, &target.restricted_namespace_fix) { + (Ok(Namespace::Inputs), _) => input_binding_fix(name), + (Ok(Namespace::Vars), _) => variable_binding_fix(name), + (_, Some(fix)) => fix.clone(), + (Ok(Namespace::Goal), None) => GOAL_BINDING_FIX.to_string(), + (Ok(namespace), None) => format!("`{namespace}` is not available in workflow templates"), + (Err(_), None) => { + format!( + "define `{}` in the template context", + expression.unwrap_or("the value") + ) + } + } +} + fn input_binding_fix(name: &str) -> String { format!("bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=`") } +fn variable_binding_fix(name: &str) -> String { + format!("set it with `fabro variable set {name} `") +} + +const GOAL_BINDING_FIX: &str = "set a graph `goal` on the workflow"; + /// Substitutes `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` in one /// command node `script`. /// @@ -351,7 +409,7 @@ fn script_interpolation_fix(err: &ResolveError, language: Option<&str>) -> Strin let name = &err.name; match err.namespace { Namespace::Inputs => input_binding_fix(name), - Namespace::Vars => format!("set it with `fabro variable set {name} `"), + Namespace::Vars => variable_binding_fix(name), Namespace::Env if language == Some("python") => format!( "`script` does not interpolate environment variables; read it in Python as \ `os.environ[\"{name}\"]` instead" @@ -368,7 +426,7 @@ fn script_interpolation_fix(err: &ResolveError, language: Option<&str>) -> Strin "`script` does not interpolate secrets; expose `{name}` to the sandbox through \ `[environments..env]` and read it in the shell as `${name}`" ), - Namespace::Goal => "set a graph `goal` on the workflow".to_string(), + Namespace::Goal => GOAL_BINDING_FIX.to_string(), } } @@ -382,16 +440,16 @@ fn detemplated_attribute_diagnostic(attr_name: &str, target: &TemplateRenderTarg severity: Severity::Warning, message: format!( "`{attr_name}` in {} is no longer a template; `{{{{ … }}}}` / `{{% … %}}` is treated \ - as literal text. Only node `prompt` and graph `goal` support templating, and node \ - command `script` supports `{{{{ goal }}}}`, `{{{{ inputs.* }}}}`, and \ - `{{{{ vars.* }}}}` interpolation.", + as literal text. Node `prompt`, graph `goal`, and graph `model_stylesheet` support \ + templating. Node command `script` supports `{{{{ goal }}}}`, \ + `{{{{ inputs.* }}}}`, and `{{{{ vars.* }}}}` interpolation.", target.owner ), node_id: target.node_id.clone(), edge: target.edge.clone(), fix: Some(format!( "remove the template syntax from `{attr_name}`, or move the dynamic value into a \ - `prompt`/`goal`" + `prompt`/`goal`/`model_stylesheet`" )), source_path: target.source_name.clone(), ..Diagnostic::default() @@ -510,6 +568,12 @@ impl TemplateTransform { if matches!(scope, AttributeScope::Graph) && attr_name == "goal" { continue; } + // The root model stylesheet has its own restricted template + // pass after imports are expanded. Imported stylesheets stay + // ignored and are diagnosed by ImportTransform. + if matches!(scope, AttributeScope::Graph) && attr_name == "model_stylesheet" { + continue; + } if attr_name == "stack.child_dot_source" { continue; } diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 7786dd49d..e3481d055 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -1691,6 +1691,66 @@ fn stylesheet_comments_apply_via_parsed_graph() { ); } +#[test] +fn model_stylesheet_template_renders_through_pipeline() { + use fabro_workflow::pipeline::{TransformOptions, transform, validate}; + + let input = r#"digraph StyleTemplate { + graph [ + goal="Review the change", + model_stylesheet=" + * { reasoning_effort: low; } + {% for effort in inputs.efforts %} + .tier-{{ loop.index }} { reasoning_effort: {{ effort }}; } + {% endfor %} + " + ] + start [shape=Mdiamond] + baseline [prompt="Baseline"] + selected [prompt="Selected", class="tier-2"] + exit [shape=Msquare] + start -> baseline -> selected -> exit + }"#; + let parsed = fabro_workflow::pipeline::parse(input).expect("parse should succeed"); + let transformed = transform(parsed, &TransformOptions { + current_dir: None, + file_resolver: None, + template_context: fabro_template::TemplateContext::new().with_inputs( + std::collections::HashMap::from([( + "efforts".to_string(), + toml::Value::Array(vec![ + toml::Value::String("medium".to_string()), + toml::Value::String("high".to_string()), + ]), + )]), + ), + source_name: Some("style-template.fabro".to_string()), + render_mode: fabro_workflow::operations::RenderMode::Structural, + custom_transforms: vec![], + model_resolution: None, + }) + .expect("transform should succeed"); + let validated = validate(transformed, None, &[]); + validated + .raise_on_errors() + .expect("rendered stylesheet should validate"); + + assert_eq!( + validated.graph().nodes["baseline"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + Some("low") + ); + assert_eq!( + validated.graph().nodes["selected"] + .attrs + .get("reasoning_effort") + .and_then(AttrValue::as_str), + Some("high") + ); +} + #[test] fn stylesheet_application_matches_space_separated_classes_from_dot() { let input = r#"digraph StyleTest { diff --git a/lib/foundation/fabro-template/src/lib.rs b/lib/foundation/fabro-template/src/lib.rs index 9fc89697b..0c3c4ba78 100644 --- a/lib/foundation/fabro-template/src/lib.rs +++ b/lib/foundation/fabro-template/src/lib.rs @@ -17,8 +17,8 @@ pub use dependency::{ extract_template_dependencies, }; pub use static_reference::{ - GraphReference, GraphReferenceError, StaticReferenceError, validate_static_reference, - visit_graph_references, + GraphPosition, GraphReference, GraphReferenceError, StaticReferenceError, + validate_static_reference, visit_graph_references, }; pub use store::{ BundleTemplateStore, CachedTemplateStore, FilesystemTemplateStore, RecordingTemplateStore, @@ -94,6 +94,21 @@ impl TemplateContext { self } + /// Context exposed to a graph `model_stylesheet` template. + /// + /// Stylesheets can use typed run inputs and run-scoped variables, but they + /// cannot read the rendered graph goal. Keep this as an explicit + /// projection so callers do not depend on when the goal enters the wider + /// workflow template context. + #[must_use] + pub fn for_model_stylesheet(&self) -> Self { + Self { + goal: None, + inputs: self.inputs.clone(), + vars: self.vars.clone(), + } + } + /// Context that interpolates inputs but leaves `{{ goal }}` as a literal /// pass-through — used for structural pre-rendering before the goal is /// known (e.g. manifest scanning, import resolution). @@ -867,6 +882,42 @@ mod tests { ); } + #[test] + fn model_stylesheet_context_exposes_only_inputs_and_vars() { + let ctx = TemplateContext::new() + .with_goal("Ship it") + .with_inputs(HashMap::from([( + "policy".to_string(), + toml::Value::Table(Map::from_iter([( + "effort".to_string(), + toml::Value::String("high".to_string()), + )])), + )])) + .with_vars(HashMap::from([("MODEL".to_string(), "sonnet".to_string())])); + let stylesheet_ctx = ctx.for_model_stylesheet(); + + assert_eq!( + render( + "{{ inputs.policy.effort }} {{ vars.MODEL }}", + &stylesheet_ctx, + ) + .unwrap(), + "high sonnet" + ); + assert!(matches!( + render("{{ goal }}", &stylesheet_ctx), + Err(TemplateError::UndefinedVariable { .. }) + )); + } + + #[test] + fn model_stylesheet_context_preserves_template_locals_and_control_flow() { + let ctx = TemplateContext::new().for_model_stylesheet(); + let template = "{% set efforts = ['low', 'high'] %}{% for effort in efforts %}{{ loop.index }}={{ effort }}{% if not loop.last %};{% endif %}{% endfor %}"; + + assert_eq!(render(template, &ctx).unwrap(), "1=low;2=high"); + } + #[test] fn references_top_level_variable_detects_goal_self_reference() { assert!(references_top_level_variable("Do {{ goal }} now", "goal")); diff --git a/lib/foundation/fabro-template/src/static_reference.rs b/lib/foundation/fabro-template/src/static_reference.rs index e2678ea65..6dc2a310d 100644 --- a/lib/foundation/fabro-template/src/static_reference.rs +++ b/lib/foundation/fabro-template/src/static_reference.rs @@ -2,9 +2,9 @@ //! //! Workflow graphs name other files through a fixed attribute vocabulary //! (`import`, `stack.child_workflow`, `@`-prefixed `prompt`/`output_schema` -//! values, and the graph `goal`). These references are *static*: they may not -//! contain template syntax, because they are resolved before any template -//! rendering happens. +//! values, the graph `goal`, and inline root template fields such as +//! `model_stylesheet`). File references are *static*: they may not contain +//! template syntax, because they are resolved before template rendering. //! //! [`visit_graph_references`] is the one walker over that vocabulary. The //! manifest bundler and workflow-version validation both consume it, so a new @@ -68,6 +68,12 @@ pub enum GraphReference<'graph> { GoalFile { reference: &'graph str }, /// A non-`@` graph `goal`: inline template content. GoalInline { content: &'graph str }, + /// The entrypoint graph's inline `model_stylesheet` template content. + /// + /// Emitted only when the walked graph is [`GraphPosition::Entrypoint`]; + /// imported stylesheets are ignored at runtime, so they are never + /// template roots. + ModelStylesheetInline { content: &'graph str }, /// `node [import=""]` — another graph file to walk. Import { reference: &'graph str }, /// `node [stack.child_workflow=""]`. @@ -91,13 +97,28 @@ pub enum GraphReferenceError { Visit(E), } +/// Whether the walked graph is the workflow's entrypoint or was reached +/// through an `import`/`stack.child_workflow` reference. +/// +/// Position-dependent reference semantics (today: `model_stylesheet` is a +/// template root only on the entrypoint) live in the walker, so every +/// consumer applies the same rule. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum GraphPosition { + Entrypoint, + Imported, +} + /// Walk every static file reference and inline template in one parsed graph, /// validating that file references are template-free before emitting them. /// /// The walker covers a single graph; recursion into `Import` targets and /// resolution of references against a file source are the consumer's job. +/// `position` tells the walker whether this graph is the workflow entrypoint, +/// which gates position-dependent references such as `model_stylesheet`. pub fn visit_graph_references<'graph, E>( graph: &'graph Graph, + position: GraphPosition, mut visit: impl FnMut(GraphReference<'graph>) -> Result<(), E>, ) -> Result<(), GraphReferenceError> { let goal = graph.goal(); @@ -112,6 +133,14 @@ pub fn visit_graph_references<'graph, E>( } } + let model_stylesheet = graph.model_stylesheet(); + if position == GraphPosition::Entrypoint && !model_stylesheet.is_empty() { + visit(GraphReference::ModelStylesheetInline { + content: model_stylesheet, + }) + .map_err(GraphReferenceError::Visit)?; + } + for node in graph.nodes.values() { for (key, value) in &node.attrs { let Some(value) = value.as_str() else { @@ -152,7 +181,7 @@ mod tests { use fabro_types::graph::{AttrValue, Graph, Node, ReferenceKind}; - use super::{GraphReference, GraphReferenceError, validate_static_reference}; + use super::{GraphPosition, GraphReference, GraphReferenceError, validate_static_reference}; #[test] fn static_reference_rejects_template_syntax() { @@ -191,6 +220,10 @@ mod tests { "goal".to_string(), AttrValue::String("@goal.md".to_string()), ); + graph.attrs.insert( + "model_stylesheet".to_string(), + AttrValue::String("{% include 'styles.partial' %}".to_string()), + ); for node in [ node_with("imported", &[("import", "graphs/child.fabro")]), node_with("child", &[("stack.child_workflow", "children/check.fabro")]), @@ -203,10 +236,14 @@ mod tests { let mut seen = BTreeSet::new(); super::visit_graph_references( &graph, + GraphPosition::Entrypoint, |reference| -> Result<(), std::convert::Infallible> { seen.insert(match reference { GraphReference::GoalFile { reference } => format!("goal-file:{reference}"), GraphReference::GoalInline { content } => format!("goal-inline:{content}"), + GraphReference::ModelStylesheetInline { content } => { + format!("stylesheet-inline:{content}") + } GraphReference::Import { reference } => format!("import:{reference}"), GraphReference::ChildWorkflow { reference } => format!("child:{reference}"), GraphReference::FileInline { key, reference } => { @@ -227,10 +264,29 @@ mod tests { "child:children/check.fabro".to_string(), "file:prompt:prompts/task.md".to_string(), "inline:Do the {{ thing }}".to_string(), + "stylesheet-inline:{% include 'styles.partial' %}".to_string(), ]) ); } + #[test] + fn imported_graphs_do_not_emit_model_stylesheet() { + let mut graph = Graph::new("test"); + graph.attrs.insert( + "model_stylesheet".to_string(), + AttrValue::String("* { reasoning_effort: low; }".to_string()), + ); + + super::visit_graph_references( + &graph, + GraphPosition::Imported, + |reference| -> Result<(), std::convert::Infallible> { + panic!("imported graph emitted {reference:?}") + }, + ) + .unwrap(); + } + #[test] fn rejects_template_syntax_in_references_before_visiting() { let mut graph = Graph::new("test"); @@ -239,11 +295,14 @@ mod tests { node_with("imported", &[("import", "graphs/{{ name }}.fabro")]), ); - let error = - super::visit_graph_references(&graph, |_| -> Result<(), std::convert::Infallible> { + let error = super::visit_graph_references( + &graph, + GraphPosition::Entrypoint, + |_| -> Result<(), std::convert::Infallible> { panic!("references with template syntax must not be visited") - }) - .unwrap_err(); + }, + ) + .unwrap_err(); assert!(matches!(error, GraphReferenceError::StaticReference(_))); } } diff --git a/test/model_stylesheet_unbound.fabro b/test/model_stylesheet_unbound.fabro new file mode 100644 index 000000000..4f469a658 --- /dev/null +++ b/test/model_stylesheet_unbound.fabro @@ -0,0 +1,12 @@ +digraph ModelStylesheetUnbound { + graph [ + model_stylesheet=" + * { reasoning_effort: {{ inputs.effort }}; } + " + ] + + start [shape=Mdiamond] + work [prompt="Do work"] + exit [shape=Msquare] + start -> work -> exit +}