mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
feat(template): add source-aware diagnostics (#292)
## Summary Template failures from `fabro run` and structural warnings from `fabro validate` now preserve source provenance through rendering, workflow transforms, API serialization, and CLI display. Diagnostics can point at the actual workflow, import, or prompt file with node/attribute context instead of surfacing MiniJinja's generic `<string>` source. ## What Changed - Added named MiniJinja render APIs plus miette-aware `TemplateError` metadata for source names, source text, spans, and labels. - Reworked workflow template expansion so inline attributes, imported workflows, and `@prompt` files render with file and owner context. - Split strict run behavior from structural validate behavior: run-start still hard-fails on missing inputs, while validate emits source-aware warnings and continues linting. - Extended validation diagnostics through Rust structs, OpenAPI, server DTO mapping, and CLI rendering with optional source path, line, column, span, and related metadata. - Added regression coverage across template rendering, workflow transforms, CLI output, and the server validate endpoint. ## Verification - `cargo nextest run -p fabro-template` - `ulimit -n 4096 && cargo nextest run -p fabro-workflow --no-fail-fast` - `cargo nextest run -p fabro-cli bare_fabro_with_unbound_inputs_validates_structurally_with_warning run_rejects_unbound_template_inputs_before_creating_remote_run` - `cargo nextest run -p fabro-server validate_endpoint_returns_template_source_coordinates` - `cargo build -p fabro-api` - `cargo +nightly-2026-04-14 fmt --check --all` - `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` --- [](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 via [Codex](https://openai.com/codex) --------- Co-authored-by: Aleksi Asikainen <1086393+salieri@users.noreply.github.com>
This commit is contained in:
parent
e9387bf622
commit
2ba04be181
53 changed files with 2413 additions and 402 deletions
2
Cargo.lock
generated
2
Cargo.lock
generated
|
|
@ -2413,6 +2413,7 @@ version = "0.235.0-nightly.0"
|
|||
dependencies = [
|
||||
"anyhow",
|
||||
"fabro-util",
|
||||
"miette",
|
||||
"minijinja",
|
||||
"serde",
|
||||
"thiserror 2.0.18",
|
||||
|
|
@ -2564,6 +2565,7 @@ dependencies = [
|
|||
"hex",
|
||||
"httpmock",
|
||||
"md5",
|
||||
"miette",
|
||||
"mime_guess",
|
||||
"object_store",
|
||||
"predicates",
|
||||
|
|
|
|||
|
|
@ -6021,6 +6021,41 @@ components:
|
|||
type: string
|
||||
fix:
|
||||
type: ["string", "null"]
|
||||
source_path:
|
||||
type: ["string", "null"]
|
||||
line:
|
||||
type: ["integer", "null"]
|
||||
format: int32
|
||||
column:
|
||||
type: ["integer", "null"]
|
||||
format: int32
|
||||
span_start:
|
||||
type: ["integer", "null"]
|
||||
format: int64
|
||||
span_len:
|
||||
type: ["integer", "null"]
|
||||
format: int64
|
||||
related:
|
||||
type: array
|
||||
items:
|
||||
$ref: "#/components/schemas/RelatedWorkflowDiagnostic"
|
||||
default: []
|
||||
|
||||
RelatedWorkflowDiagnostic:
|
||||
type: object
|
||||
required:
|
||||
- message
|
||||
properties:
|
||||
message:
|
||||
type: string
|
||||
source_path:
|
||||
type: ["string", "null"]
|
||||
line:
|
||||
type: ["integer", "null"]
|
||||
format: int32
|
||||
column:
|
||||
type: ["integer", "null"]
|
||||
format: int32
|
||||
|
||||
PreflightCheckReport:
|
||||
type: object
|
||||
|
|
|
|||
|
|
@ -0,0 +1,647 @@
|
|||
# Source-Aware Template Diagnostics Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make `fabro run` and `fabro validate` template errors point to the real source file, line/column, and node context instead of MiniJinja's generic `<string>`.
|
||||
|
||||
**Architecture:** Preserve source provenance through template rendering instead of flattening errors to strings. `fabro-template` owns named MiniJinja rendering and typed span metadata; `fabro-workflow` owns workflow/node/attribute context; CLI and API layers render or serialize the structured diagnostics at their boundaries.
|
||||
|
||||
**Tech Stack:** Rust, MiniJinja, miette, thiserror, serde, OpenAPI/progenitor, cargo-nextest, insta snapshots.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- Modify `lib/crates/fabro-template/src/lib.rs` to add named render APIs and miette-aware `TemplateError` metadata.
|
||||
- Modify `lib/crates/fabro-workflow/src/operations/create.rs`, `lib/crates/fabro-workflow/src/transforms/variable_expansion.rs`, `lib/crates/fabro-workflow/src/transforms/file_inlining.rs`, and `lib/crates/fabro-workflow/src/transforms/import.rs` to render templates with source and owner context.
|
||||
- Modify `lib/crates/fabro-workflow/src/error.rs` to preserve source-aware template errors instead of converting them to `String`.
|
||||
- Modify `lib/crates/fabro-validate/src/lib.rs`, `docs/public/api-reference/fabro-api.yaml`, and `lib/crates/fabro-server/src/run_manifest.rs` to expose optional source coordinates in validation diagnostics.
|
||||
- Modify `lib/crates/fabro-cli/src/main.rs`, `lib/crates/fabro-cli/src/shared/utilities.rs`, and `lib/crates/fabro-cli/src/commands/run/output.rs` to render structured source metadata.
|
||||
- Update or add tests in `lib/crates/fabro-template/src/lib.rs`, `lib/crates/fabro-workflow/src/operations/create.rs`, `lib/crates/fabro-cli/tests/it/cmd/validate.rs`, `lib/crates/fabro-cli/tests/it/cmd/run.rs`, and `lib/crates/fabro-server/src/server/tests.rs`.
|
||||
|
||||
## Task 1: Add Named Template Rendering
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-template/src/lib.rs`
|
||||
|
||||
- [x] **Step 1: Add failing unit tests for named template diagnostics**
|
||||
|
||||
Add tests that call the new API names below:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn render_named_reports_source_name_expression_and_span() {
|
||||
let ctx = TemplateContext::new();
|
||||
let err = render_named("prompts/test.md", "Hello {{ inputs.foo }}", &ctx).unwrap_err();
|
||||
|
||||
let TemplateError::UndefinedVariable {
|
||||
expression,
|
||||
line,
|
||||
source_name,
|
||||
span,
|
||||
..
|
||||
} = err
|
||||
else {
|
||||
panic!("expected undefined variable error");
|
||||
};
|
||||
|
||||
assert_eq!(expression.as_deref(), Some("inputs.foo"));
|
||||
assert_eq!(line, Some(1));
|
||||
assert_eq!(source_name.as_deref(), Some("prompts/test.md"));
|
||||
assert!(span.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_lenient_named_preserves_source_name_for_syntax_errors() {
|
||||
let ctx = TemplateContext::new();
|
||||
let err = render_lenient_named("workflow.fabro", "{{ unterminated", &ctx).unwrap_err();
|
||||
|
||||
let TemplateError::Syntax {
|
||||
source_name, ..
|
||||
} = err
|
||||
else {
|
||||
panic!("expected syntax error");
|
||||
};
|
||||
|
||||
assert_eq!(source_name.as_deref(), Some("workflow.fabro"));
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run the tests and verify they fail**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-template render_named_reports_source_name_expression_and_span render_lenient_named_preserves_source_name_for_syntax_errors
|
||||
```
|
||||
|
||||
Expected: tests fail because `render_named` and `render_lenient_named` do not exist.
|
||||
|
||||
- [x] **Step 3: Implement named rendering APIs**
|
||||
|
||||
Add these public functions and route existing `render` functions through an internal helper:
|
||||
|
||||
```rust
|
||||
pub fn render(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(None, template, ctx, UndefinedBehavior::Strict)
|
||||
}
|
||||
|
||||
pub fn render_named(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(Some(name.into()), template, ctx, UndefinedBehavior::Strict)
|
||||
}
|
||||
|
||||
pub fn render_lenient(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(None, template, ctx, UndefinedBehavior::Chainable)
|
||||
}
|
||||
|
||||
pub fn render_lenient_named(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(Some(name.into()), template, ctx, UndefinedBehavior::Chainable)
|
||||
}
|
||||
```
|
||||
|
||||
In `render_with`, use `env.render_named_str(&name, template, ctx.clone().into_value())` when a name is present, and `env.render_str(...)` otherwise. Keep the current plain-text fast path.
|
||||
|
||||
- [x] **Step 4: Store source metadata on `TemplateError`**
|
||||
|
||||
Extend each `TemplateError` variant with:
|
||||
|
||||
```rust
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
span: Option<miette::SourceSpan>,
|
||||
```
|
||||
|
||||
In `From<minijinja::Error> for TemplateError`, populate:
|
||||
|
||||
```rust
|
||||
let source_name = error.name().map(str::to_owned);
|
||||
let source_text = error.template_source().map(str::to_owned);
|
||||
let span = error
|
||||
.range()
|
||||
.and_then(|range| {
|
||||
let start = range.start;
|
||||
let len = range.end.checked_sub(range.start)?;
|
||||
Some((start, len).into())
|
||||
});
|
||||
```
|
||||
|
||||
Preserve the existing `expression` and `line` behavior.
|
||||
|
||||
- [x] **Step 5: Derive or implement `miette::Diagnostic` for `TemplateError`**
|
||||
|
||||
Use `#[derive(miette::Diagnostic)]` if it stays readable. If derive becomes awkward because of enum variants, implement `miette::Diagnostic` manually so `source_code()` returns a `NamedSource<String>` when both `source_name` and `source_text` exist, and `labels()` returns a label for `span`.
|
||||
|
||||
- [x] **Step 6: Verify template tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-template
|
||||
```
|
||||
|
||||
Expected: all `fabro-template` tests pass.
|
||||
|
||||
## Task 2: Preserve Template Errors in Workflow Errors
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-workflow/src/error.rs`
|
||||
- Modify: `lib/crates/fabro-workflow/Cargo.toml`
|
||||
|
||||
- [x] **Step 1: Add a failing error-chain regression test**
|
||||
|
||||
Add a test showing that a workflow template error remains visible as a source cause:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn template_error_variant_preserves_source_chain() {
|
||||
let template_err = fabro_template::render_named(
|
||||
"workflow.fabro",
|
||||
"{{ inputs.missing }}",
|
||||
&fabro_template::TemplateContext::new(),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
let err = Error::template("template expansion failed", template_err);
|
||||
let chain = collect_chain(&err);
|
||||
|
||||
assert!(chain.iter().any(|part| part.contains("template expansion failed")));
|
||||
assert!(chain.iter().any(|part| part.contains("undefined template variable")));
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run the test and verify it fails**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow template_error_variant_preserves_source_chain
|
||||
```
|
||||
|
||||
Expected: test fails because `Error::template` does not exist.
|
||||
|
||||
- [x] **Step 3: Add a structured workflow error variant**
|
||||
|
||||
Add a new `Error` variant:
|
||||
|
||||
```rust
|
||||
#[error("{message}")]
|
||||
Template {
|
||||
message: String,
|
||||
#[source]
|
||||
source: fabro_template::TemplateError,
|
||||
}
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```rust
|
||||
pub fn template(message: impl Into<String>, source: fabro_template::TemplateError) -> Self {
|
||||
Self::Template {
|
||||
message: message.into(),
|
||||
source,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Update failure classification to treat `Template` as deterministic validation/parse failure.
|
||||
|
||||
- [x] **Step 4: Delegate miette metadata from workflow template errors**
|
||||
|
||||
Implement `miette::Diagnostic` for `Error` or add a wrapper diagnostic used by the CLI so `Error::Template { source, .. }` delegates `source_code`, `labels`, `code`, and `help` to the underlying `TemplateError`.
|
||||
|
||||
- [x] **Step 5: Replace string conversion helper**
|
||||
|
||||
Remove the current `template_parse_error(&TemplateError) -> Error` stringification path in `operations/create.rs` and convert call sites to:
|
||||
|
||||
```rust
|
||||
.map_err(|err| Error::template("template expansion failed", err))?
|
||||
```
|
||||
|
||||
- [x] **Step 6: Verify workflow error tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow template_error_variant_preserves_source_chain
|
||||
```
|
||||
|
||||
Expected: test passes.
|
||||
|
||||
## Task 3: Render DOT Attributes Source-Aware After Parse
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-workflow/src/operations/create.rs`
|
||||
- Modify: `lib/crates/fabro-workflow/src/transforms/variable_expansion.rs`
|
||||
- Modify: `lib/crates/fabro-workflow/src/pipeline/transform.rs`
|
||||
|
||||
- [x] **Step 1: Add a failing workflow test for inline prompt source context**
|
||||
|
||||
Add a test in `operations/create.rs`:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn strict_template_error_for_inline_prompt_names_workflow_file_and_node() {
|
||||
let dot = r#"digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_inline_prompt [label="moo" prompt="{{ inputs.foo }}"]
|
||||
start -> test_inline_prompt -> exit
|
||||
}"#;
|
||||
|
||||
let err = preprocess_and_validate(
|
||||
dot,
|
||||
Some(PathBuf::from(".")),
|
||||
None,
|
||||
Vec::new(),
|
||||
None,
|
||||
None,
|
||||
RenderMode::Strict,
|
||||
&test_catalog(),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
assert!(rendered.contains("test_inline_prompt"));
|
||||
assert!(rendered.contains("prompt"));
|
||||
assert!(!rendered.contains("<string>"));
|
||||
}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run the test and verify it fails**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow strict_template_error_for_inline_prompt_names_workflow_file_and_node
|
||||
```
|
||||
|
||||
Expected: test fails because the pre-parse render path still reports a generic template source.
|
||||
|
||||
- [x] **Step 3: Remove strict pre-parse DOT rendering**
|
||||
|
||||
In `preprocess_and_validate`, delete the initial strict whole-file `render_template(dot_source, ...)` phase. Always parse `dot_source` first, then apply transforms. For `RenderMode::Structural`, keep undefined input warnings by letting the template transform collect diagnostics instead of rendering the whole DOT file.
|
||||
|
||||
- [x] **Step 4: Add source options to the template transform**
|
||||
|
||||
Change `TemplateTransform` to carry:
|
||||
|
||||
```rust
|
||||
pub struct TemplateTransform {
|
||||
pub inputs: HashMap<String, toml::Value>,
|
||||
pub source_name: Option<String>,
|
||||
pub render_mode: RenderMode,
|
||||
}
|
||||
```
|
||||
|
||||
Render graph attrs, node attrs, and edge attrs one value at a time. Use names like:
|
||||
|
||||
```rust
|
||||
workflow.fabro graph attribute `goal`
|
||||
workflow.fabro node `test_inline_prompt` attribute `prompt`
|
||||
workflow.fabro edge `start -> test_inline_prompt` attribute `label`
|
||||
```
|
||||
|
||||
When source path is unknown, use `workflow`.
|
||||
|
||||
- [x] **Step 5: Attach owner context to errors and diagnostics**
|
||||
|
||||
For strict mode, wrap template errors with context:
|
||||
|
||||
```text
|
||||
template expansion failed in node `test_inline_prompt` attribute `prompt`
|
||||
```
|
||||
|
||||
For structural mode undefined variables, produce a warning diagnostic with `node_id` set to the owning node id and `message` including the source path and line when available.
|
||||
|
||||
- [x] **Step 6: Verify workflow transform tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow strict_template_error_for_inline_prompt_names_workflow_file_and_node
|
||||
cargo nextest run -p fabro-workflow template_transform
|
||||
```
|
||||
|
||||
Expected: source-aware test and existing template transform tests pass.
|
||||
|
||||
## Task 4: Make Prompt File Rendering First-Class
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-workflow/src/transforms/file_inlining.rs`
|
||||
- Modify: `lib/crates/fabro-workflow/src/transforms/variable_expansion.rs`
|
||||
|
||||
- [x] **Step 1: Add a failing test for imported prompt source attribution**
|
||||
|
||||
Add a test that builds a graph with `prompt="@test.md"` and a resolver returning `{{ inputs.foo }}` for `test.md`. Assert strict mode errors mention `test.md`, `test_imported_prompt`, and `prompt`.
|
||||
|
||||
- [x] **Step 2: Run the test and verify it fails**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow imported_prompt_template_error_names_prompt_file_and_node
|
||||
```
|
||||
|
||||
Expected: test fails because file inlining currently loses the prompt file source.
|
||||
|
||||
- [x] **Step 3: Render prompt attribute paths before resolving file references**
|
||||
|
||||
For node `prompt` and graph `goal`, first render the attribute value with the workflow source name so templated paths like `@{{ inputs.prompt_file }}` continue to work.
|
||||
|
||||
- [x] **Step 4: Resolve `@file` references and render file contents with resolved path**
|
||||
|
||||
When a prompt or goal value resolves to a file, render the file contents using:
|
||||
|
||||
```rust
|
||||
render_named(resolved.path.display().to_string(), &resolved.content, &ctx)
|
||||
```
|
||||
|
||||
Wrap failures with node/attr context from the referencing graph value.
|
||||
|
||||
- [x] **Step 5: Keep non-file string attributes on workflow source**
|
||||
|
||||
Only prompt and goal `@file` references switch to the resolved file source. Other string attributes render against their owning workflow/import file source.
|
||||
|
||||
- [x] **Step 6: Verify prompt source tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow imported_prompt_template_error_names_prompt_file_and_node
|
||||
```
|
||||
|
||||
Expected: test passes.
|
||||
|
||||
## Task 5: Make Import Rendering Source-Aware
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-workflow/src/transforms/import.rs`
|
||||
|
||||
- [x] **Step 1: Add failing tests for import source attribution**
|
||||
|
||||
Add tests for:
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn templated_import_path_error_names_importing_node() { /* import="{{ inputs.path }}" */ }
|
||||
|
||||
#[test]
|
||||
fn imported_workflow_template_error_names_imported_file() { /* imported file contains "{{ inputs.foo }}" */ }
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run the tests and verify they fail**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow templated_import_path_error_names_importing_node imported_workflow_template_error_names_imported_file
|
||||
```
|
||||
|
||||
Expected: tests fail with stringified or generic template errors.
|
||||
|
||||
- [x] **Step 3: Render import attributes with node context**
|
||||
|
||||
Before resolving an import path, render the placeholder node's `import` attr with a named template source and wrap failures with the placeholder node id.
|
||||
|
||||
- [x] **Step 4: Parse imported workflows from their own source name**
|
||||
|
||||
When rendering imported workflow content for parse preparation, call `render_named(resolved_file.path.display().to_string(), ...)`. Preserve errors as `Error::Template`.
|
||||
|
||||
- [x] **Step 5: Verify import tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-workflow templated_import_path_error_names_importing_node imported_workflow_template_error_names_imported_file
|
||||
```
|
||||
|
||||
Expected: tests pass.
|
||||
|
||||
## Task 6: Serialize Source Coordinates in Validation Diagnostics
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-validate/src/lib.rs`
|
||||
- Modify: `docs/public/api-reference/fabro-api.yaml`
|
||||
- Modify: `lib/crates/fabro-server/src/run_manifest.rs`
|
||||
- Modify: `lib/crates/fabro-cli/src/commands/run/output.rs`
|
||||
|
||||
- [x] **Step 1: Add optional fields to `fabro_validate::Diagnostic`**
|
||||
|
||||
Add:
|
||||
|
||||
```rust
|
||||
pub source_path: Option<String>,
|
||||
pub line: Option<u32>,
|
||||
pub column: Option<u32>,
|
||||
pub span_start: Option<usize>,
|
||||
pub span_len: Option<usize>,
|
||||
pub related: Vec<RelatedDiagnostic>,
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```rust
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct RelatedDiagnostic {
|
||||
pub message: String,
|
||||
pub source_path: Option<String>,
|
||||
pub line: Option<u32>,
|
||||
pub column: Option<u32>,
|
||||
}
|
||||
```
|
||||
|
||||
Update all existing diagnostic constructors with `..Diagnostic::default()` by deriving or implementing `Default` for `Diagnostic`.
|
||||
|
||||
- [x] **Step 2: Update OpenAPI schema**
|
||||
|
||||
Add the same optional fields to `WorkflowDiagnostic` in `docs/public/api-reference/fabro-api.yaml`, and add a `RelatedWorkflowDiagnostic` schema.
|
||||
|
||||
- [x] **Step 3: Regenerate Rust API types**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo build -p fabro-api
|
||||
```
|
||||
|
||||
Expected: build succeeds and generated types include the new optional fields.
|
||||
|
||||
- [x] **Step 4: Map diagnostics to and from API DTOs**
|
||||
|
||||
Update `diagnostics_to_api` and CLI `api_diagnostic_to_local` to preserve all new optional fields and related diagnostics.
|
||||
|
||||
- [x] **Step 5: Verify API build**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo build -p fabro-api
|
||||
```
|
||||
|
||||
Expected: build succeeds.
|
||||
|
||||
## Task 7: Render CLI Output with Source Context
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-cli/src/main.rs`
|
||||
- Modify: `lib/crates/fabro-cli/src/shared/utilities.rs`
|
||||
- Modify: `lib/crates/fabro-cli/tests/it/cmd/validate.rs`
|
||||
- Modify: `lib/crates/fabro-cli/tests/it/cmd/run.rs`
|
||||
|
||||
- [x] **Step 1: Add CLI snapshot coverage for issue #287**
|
||||
|
||||
Add fixtures or temp workflows for:
|
||||
|
||||
```dot
|
||||
digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_inline_prompt [label="moo" prompt="{{ inputs.foo }}"]
|
||||
start -> test_inline_prompt -> exit
|
||||
}
|
||||
```
|
||||
|
||||
and:
|
||||
|
||||
```dot
|
||||
digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_imported_prompt [label="moo" prompt="@test.md"]
|
||||
start -> test_imported_prompt -> exit
|
||||
}
|
||||
```
|
||||
|
||||
with `test.md` containing:
|
||||
|
||||
```markdown
|
||||
{{ inputs.foo }}
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run snapshots and verify they fail**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-cli validate_template_source_attribution run_rejects_unbound_template_inputs_before_creating_remote_run
|
||||
```
|
||||
|
||||
Expected: snapshots fail because output still contains `<string>` or lacks source context.
|
||||
|
||||
- [x] **Step 3: Delegate fatal miette diagnostics through CLI wrapper**
|
||||
|
||||
Update `CliDiagnostic` so `impl miette::Diagnostic for CliDiagnostic` delegates `source_code`, `labels`, `code`, and `help` to the wrapped error when the wrapped error chain contains a `miette::Diagnostic`.
|
||||
|
||||
- [x] **Step 4: Print validation diagnostics with source coordinates**
|
||||
|
||||
Update `print_diagnostics` so diagnostics with `source_path`, `line`, and `column` print a compact prefix:
|
||||
|
||||
```text
|
||||
warning: test.md:1:1: undefined template variable `inputs.foo` (template_undefined_variable)
|
||||
```
|
||||
|
||||
Keep the existing message shape for diagnostics without source coordinates.
|
||||
|
||||
- [x] **Step 5: Verify CLI snapshots**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-cli validate_template_source_attribution run_rejects_unbound_template_inputs_before_creating_remote_run
|
||||
cargo insta pending-snapshots
|
||||
```
|
||||
|
||||
Expected: only the intended snapshots are pending. Inspect them, then accept only the intended updates:
|
||||
|
||||
```bash
|
||||
cargo insta accept
|
||||
```
|
||||
|
||||
## Task 8: Add API Validation Coverage
|
||||
|
||||
**Files:**
|
||||
- Modify: `lib/crates/fabro-server/src/server/tests.rs`
|
||||
|
||||
- [x] **Step 1: Add a validation endpoint test for source fields**
|
||||
|
||||
Add a test that posts a run manifest containing an imported prompt with an undefined template variable and asserts the first diagnostic includes:
|
||||
|
||||
```rust
|
||||
assert_eq!(diagnostic["source_path"], "test.md");
|
||||
assert_eq!(diagnostic["line"], 1);
|
||||
assert!(diagnostic["node_id"].as_str().unwrap().contains("test_imported_prompt"));
|
||||
```
|
||||
|
||||
- [x] **Step 2: Run the server test**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-server validate_endpoint_returns_template_source_coordinates
|
||||
```
|
||||
|
||||
Expected: test passes.
|
||||
|
||||
## Task 9: Final Verification
|
||||
|
||||
**Files:**
|
||||
- No additional edits.
|
||||
|
||||
- [x] **Step 1: Run focused test suite**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo nextest run -p fabro-template
|
||||
cargo nextest run -p fabro-workflow
|
||||
cargo nextest run -p fabro-cli validate_template_source_attribution run_rejects_unbound_template_inputs_before_creating_remote_run
|
||||
cargo nextest run -p fabro-server validate_endpoint_returns_template_source_coordinates
|
||||
```
|
||||
|
||||
Expected: all tests pass.
|
||||
|
||||
- [x] **Step 2: Run API build**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo build -p fabro-api
|
||||
```
|
||||
|
||||
Expected: build succeeds.
|
||||
|
||||
- [x] **Step 3: Run formatting check**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo +nightly-2026-04-14 fmt --check --all
|
||||
```
|
||||
|
||||
Expected: formatting check passes.
|
||||
|
||||
- [x] **Step 4: Run clippy if the touched crates compile cleanly**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings
|
||||
```
|
||||
|
||||
Expected: clippy passes.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- This is a greenfield app, so internal workflow/template APIs and OpenAPI diagnostic DTOs may change.
|
||||
- Template support after this change is scoped to parsed string attributes and referenced workflow/prompt files, not arbitrary structural DOT templating.
|
||||
- API responses expose coordinates and context, not full source text.
|
||||
- Rich source snippets are rendered in local CLI output through `miette`; server/API clients receive structured metadata.
|
||||
- Existing diagnostics without source metadata keep their current behavior.
|
||||
|
|
@ -70,19 +70,40 @@ pub(crate) fn print_workflow_summary(
|
|||
|
||||
fn api_diagnostic_to_local(diagnostic: &types::WorkflowDiagnostic) -> fabro_validate::Diagnostic {
|
||||
fabro_validate::Diagnostic {
|
||||
rule: diagnostic.rule.clone(),
|
||||
severity: match diagnostic.severity {
|
||||
rule: diagnostic.rule.clone(),
|
||||
severity: match diagnostic.severity {
|
||||
types::WorkflowDiagnosticSeverity::Error => fabro_validate::Severity::Error,
|
||||
types::WorkflowDiagnosticSeverity::Warning => fabro_validate::Severity::Warning,
|
||||
types::WorkflowDiagnosticSeverity::Info => fabro_validate::Severity::Info,
|
||||
},
|
||||
message: diagnostic.message.clone(),
|
||||
node_id: diagnostic.node_id.clone(),
|
||||
edge: diagnostic
|
||||
message: diagnostic.message.clone(),
|
||||
node_id: diagnostic.node_id.clone(),
|
||||
edge: diagnostic
|
||||
.edge
|
||||
.as_ref()
|
||||
.map(|edge| (edge[0].clone(), edge[1].clone())),
|
||||
fix: diagnostic.fix.clone(),
|
||||
fix: diagnostic.fix.clone(),
|
||||
source_path: diagnostic.source_path.clone(),
|
||||
line: diagnostic.line.and_then(|value| u32::try_from(value).ok()),
|
||||
column: diagnostic
|
||||
.column
|
||||
.and_then(|value| u32::try_from(value).ok()),
|
||||
span_start: diagnostic
|
||||
.span_start
|
||||
.and_then(|value| usize::try_from(value).ok()),
|
||||
span_len: diagnostic
|
||||
.span_len
|
||||
.and_then(|value| usize::try_from(value).ok()),
|
||||
related: diagnostic
|
||||
.related
|
||||
.iter()
|
||||
.map(|related| fabro_validate::RelatedDiagnostic {
|
||||
message: related.message.clone(),
|
||||
source_path: related.source_path.clone(),
|
||||
line: related.line.and_then(|value| u32::try_from(value).ok()),
|
||||
column: related.column.and_then(|value| u32::try_from(value).ok()),
|
||||
})
|
||||
.collect(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -148,6 +148,13 @@ impl CliDiagnostic {
|
|||
show_auth_hint,
|
||||
}
|
||||
}
|
||||
|
||||
fn delegated_diagnostic(&self) -> Option<&dyn miette::Diagnostic> {
|
||||
self.err.chain().find_map(|err| {
|
||||
err.downcast_ref::<fabro_workflow::Error>()
|
||||
.map(|err| err as &dyn miette::Diagnostic)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl Display for CliDiagnostic {
|
||||
|
|
@ -169,13 +176,34 @@ impl std::error::Error for CliDiagnostic {
|
|||
}
|
||||
|
||||
impl miette::Diagnostic for CliDiagnostic {
|
||||
fn code<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
|
||||
self.delegated_diagnostic()
|
||||
.and_then(miette::Diagnostic::code)
|
||||
}
|
||||
|
||||
fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
|
||||
if self.show_auth_hint && exit::exit_class_for(&self.err) == Some(ExitClass::AuthRequired) {
|
||||
Some(Box::new("Run `fabro auth login` to authenticate."))
|
||||
} else {
|
||||
None
|
||||
self.delegated_diagnostic()
|
||||
.and_then(miette::Diagnostic::help)
|
||||
}
|
||||
}
|
||||
|
||||
fn source_code(&self) -> Option<&dyn miette::SourceCode> {
|
||||
self.delegated_diagnostic()
|
||||
.and_then(miette::Diagnostic::source_code)
|
||||
}
|
||||
|
||||
fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
|
||||
self.delegated_diagnostic()
|
||||
.and_then(miette::Diagnostic::labels)
|
||||
}
|
||||
|
||||
fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> {
|
||||
self.delegated_diagnostic()
|
||||
.and_then(miette::Diagnostic::diagnostic_source)
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
|
|
|
|||
|
|
@ -54,32 +54,78 @@ pub(crate) fn print_diagnostics(diagnostics: &[Diagnostic], styles: &Styles, pri
|
|||
(_, Some((from, to))) => format!(" [edge: {from} -> {to}]"),
|
||||
_ => String::new(),
|
||||
};
|
||||
let source_prefix = source_prefix(d);
|
||||
match d.severity {
|
||||
Severity::Error => fabro_util::printerr!(
|
||||
Severity::Error if source_prefix.is_empty() => fabro_util::printerr!(
|
||||
printer,
|
||||
"{}{location}: {} ({})",
|
||||
styles.red.apply_to("error"),
|
||||
d.message,
|
||||
styles.dim.apply_to(&d.rule),
|
||||
),
|
||||
Severity::Warning => fabro_util::printerr!(
|
||||
Severity::Error => fabro_util::printerr!(
|
||||
printer,
|
||||
"{}: {source_prefix}{}{location} ({})",
|
||||
styles.red.apply_to("error"),
|
||||
d.message,
|
||||
styles.dim.apply_to(&d.rule),
|
||||
),
|
||||
Severity::Warning if source_prefix.is_empty() => fabro_util::printerr!(
|
||||
printer,
|
||||
"{}{location}: {} ({})",
|
||||
styles.yellow.apply_to("warning"),
|
||||
d.message,
|
||||
styles.dim.apply_to(&d.rule),
|
||||
),
|
||||
Severity::Warning => fabro_util::printerr!(
|
||||
printer,
|
||||
"{}: {source_prefix}{}{location} ({})",
|
||||
styles.yellow.apply_to("warning"),
|
||||
d.message,
|
||||
styles.dim.apply_to(&d.rule),
|
||||
),
|
||||
Severity::Info => fabro_util::printerr!(
|
||||
printer,
|
||||
"{}",
|
||||
styles
|
||||
.dim
|
||||
.apply_to(format!("info{location}: {} ({})", d.message, d.rule)),
|
||||
styles.dim.apply_to(if source_prefix.is_empty() {
|
||||
format!("info{location}: {} ({})", d.message, d.rule)
|
||||
} else {
|
||||
format!("info: {source_prefix}{}{location} ({})", d.message, d.rule)
|
||||
}),
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn source_prefix(diagnostic: &Diagnostic) -> String {
|
||||
match (
|
||||
diagnostic.source_path.as_deref(),
|
||||
diagnostic.line,
|
||||
diagnostic.column,
|
||||
) {
|
||||
(Some(path), Some(line), Some(column)) => {
|
||||
format!("{}:{line}:{column}: ", display_diagnostic_path(path))
|
||||
}
|
||||
(Some(path), Some(line), None) => {
|
||||
format!("{}:{line}: ", display_diagnostic_path(path))
|
||||
}
|
||||
(Some(path), None, _) => format!("{}: ", display_diagnostic_path(path)),
|
||||
_ => String::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn display_diagnostic_path(path: &str) -> String {
|
||||
let path = Path::new(path);
|
||||
if let Ok(canonical) = path.canonicalize() {
|
||||
return relative_path(&canonical);
|
||||
}
|
||||
if path.is_absolute() {
|
||||
relative_path(path)
|
||||
} else {
|
||||
path.display().to_string()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn relative_path(path: &Path) -> String {
|
||||
if let Ok(cwd) = std::env::current_dir() {
|
||||
if let Ok(rel) = path.strip_prefix(&cwd) {
|
||||
|
|
|
|||
|
|
@ -73,8 +73,8 @@ fn preflight_rejects_unbound_template_inputs() {
|
|||
Graph: [FIXTURES]/templated_unbound.fabro
|
||||
Goal: Demo
|
||||
|
||||
error: undefined template variable `inputs.app_dir` at line 1 (template_undefined_variable)
|
||||
error [node: work]: undefined template variable `inputs.app_dir` in node `work` (template_undefined_variable)
|
||||
error: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable)
|
||||
error: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable)
|
||||
× Validation failed
|
||||
");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -693,6 +693,14 @@ fn run_rejects_unbound_template_inputs_before_creating_remote_run() {
|
|||
stderr.contains("inputs.app_dir"),
|
||||
"stderr should name the unbound variable: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
stderr.contains("templated_unbound.fabro"),
|
||||
"stderr should name the workflow source: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
!stderr.contains("<string>"),
|
||||
"stderr should not expose MiniJinja's generic source name: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -162,8 +162,8 @@ fn bare_fabro_with_unbound_inputs_validates_structurally_with_warning() {
|
|||
----- stderr -----
|
||||
Workflow: TemplatedUnbound (3 nodes, 2 edges)
|
||||
Graph: [FIXTURES]/templated_unbound.fabro
|
||||
warning: undefined template variable `inputs.app_dir` at line 1 (template_undefined_variable)
|
||||
warning [node: work]: undefined template variable `inputs.app_dir` in node `work` (template_undefined_variable)
|
||||
warning: [FIXTURES]/templated_unbound.fabro:2:26: undefined template variable `inputs.app_dir` in graph attribute `goal` (template_undefined_variable)
|
||||
warning: [FIXTURES]/templated_unbound.fabro:7:44: undefined template variable `inputs.app_dir` in node `work` attribute `prompt` [node: work] (template_undefined_variable)
|
||||
Validation: OK
|
||||
");
|
||||
}
|
||||
|
|
@ -185,7 +185,7 @@ fn bare_fabro_with_unbound_inputs_in_imported_prompt_validates_structurally_with
|
|||
----- stderr -----
|
||||
Workflow: TemplatedUnboundImported (3 nodes, 2 edges)
|
||||
Graph: [FIXTURES]/templated_unbound_imported/workflow.fabro
|
||||
warning [node: work]: undefined template variable `inputs.app_dir` in node `work` (template_undefined_variable)
|
||||
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)
|
||||
Validation: OK
|
||||
");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1263,19 +1263,40 @@ fn diagnostics_to_api(
|
|||
diagnostics
|
||||
.iter()
|
||||
.map(|diagnostic| types::WorkflowDiagnostic {
|
||||
edge: diagnostic
|
||||
column: diagnostic
|
||||
.column
|
||||
.and_then(|value| i32::try_from(value).ok()),
|
||||
edge: diagnostic
|
||||
.edge
|
||||
.as_ref()
|
||||
.map(|edge: &(String, String)| [edge.0.clone(), edge.1.clone()]),
|
||||
fix: diagnostic.fix.clone(),
|
||||
message: diagnostic.message.clone(),
|
||||
node_id: diagnostic.node_id.clone(),
|
||||
rule: diagnostic.rule.clone(),
|
||||
severity: match diagnostic.severity {
|
||||
fix: diagnostic.fix.clone(),
|
||||
line: diagnostic.line.and_then(|value| i32::try_from(value).ok()),
|
||||
message: diagnostic.message.clone(),
|
||||
node_id: diagnostic.node_id.clone(),
|
||||
related: diagnostic
|
||||
.related
|
||||
.iter()
|
||||
.map(|related| types::RelatedWorkflowDiagnostic {
|
||||
column: related.column.and_then(|value| i32::try_from(value).ok()),
|
||||
line: related.line.and_then(|value| i32::try_from(value).ok()),
|
||||
message: related.message.clone(),
|
||||
source_path: related.source_path.clone(),
|
||||
})
|
||||
.collect(),
|
||||
rule: diagnostic.rule.clone(),
|
||||
severity: match diagnostic.severity {
|
||||
Severity::Error => types::WorkflowDiagnosticSeverity::Error,
|
||||
Severity::Warning => types::WorkflowDiagnosticSeverity::Warning,
|
||||
Severity::Info => types::WorkflowDiagnosticSeverity::Info,
|
||||
},
|
||||
source_path: diagnostic.source_path.clone(),
|
||||
span_len: diagnostic
|
||||
.span_len
|
||||
.and_then(|value| i64::try_from(value).ok()),
|
||||
span_start: diagnostic
|
||||
.span_start
|
||||
.and_then(|value| i64::try_from(value).ok()),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3050,6 +3050,68 @@ reasoning = false
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_endpoint_returns_template_source_coordinates() {
|
||||
let app = test_app_with();
|
||||
let dot = r#"digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_imported_prompt [label="moo" prompt="@test.md"]
|
||||
start -> test_imported_prompt -> exit
|
||||
}"#;
|
||||
let manifest = serde_json::json!({
|
||||
"version": 1,
|
||||
"cwd": "/tmp",
|
||||
"target": {
|
||||
"identifier": "workflow.fabro",
|
||||
"path": "workflow.fabro",
|
||||
},
|
||||
"workflows": {
|
||||
"workflow.fabro": {
|
||||
"source": dot,
|
||||
"files": {
|
||||
"test.md": {
|
||||
"content": "{{ inputs.foo }}",
|
||||
"ref": {
|
||||
"type": "file_inline",
|
||||
"original": "test.md",
|
||||
"from": "workflow.fabro",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
let response = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(api("/validate"))
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(serde_json::to_string(&manifest).unwrap()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let body = response_json!(response, StatusCode::OK).await;
|
||||
let diagnostics = body["workflow"]["diagnostics"].as_array().unwrap();
|
||||
let diagnostic = diagnostics
|
||||
.iter()
|
||||
.find(|diagnostic| diagnostic["rule"] == "template_undefined_variable")
|
||||
.expect("expected template diagnostic");
|
||||
|
||||
assert_eq!(diagnostic["source_path"], "test.md");
|
||||
assert_eq!(diagnostic["line"], 1);
|
||||
assert_eq!(diagnostic["column"], 4);
|
||||
assert!(
|
||||
diagnostic["node_id"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.contains("test_imported_prompt")
|
||||
);
|
||||
}
|
||||
|
||||
async fn create_run_for_target(app: &Router, target_path: &str, dot_source: &str) -> String {
|
||||
let req = Request::builder()
|
||||
.method("POST")
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ workspace = true
|
|||
[dependencies]
|
||||
anyhow.workspace = true
|
||||
fabro-util = { path = "../fabro-util" }
|
||||
miette.workspace = true
|
||||
minijinja = { workspace = true, features = ["debug"] }
|
||||
serde.workspace = true
|
||||
thiserror.workspace = true
|
||||
|
|
|
|||
|
|
@ -3,9 +3,9 @@ use std::fmt;
|
|||
use std::sync::Arc;
|
||||
|
||||
use fabro_util::env::Env;
|
||||
use miette::{LabeledSpan, NamedSource, SourceCode, SourceSpan};
|
||||
use minijinja::value::{Object, Value};
|
||||
use minijinja::{AutoEscape, Environment, ErrorKind, UndefinedBehavior};
|
||||
use thiserror::Error;
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct TemplateContext {
|
||||
|
|
@ -115,33 +115,66 @@ where
|
|||
/// MiniJinja knows about (offending expression, line) plus the original
|
||||
/// `minijinja::Error` as `#[source]`, so the cause chain is preserved across
|
||||
/// boundaries that walk `Error::source()` (anyhow, miette, `collect_chain`).
|
||||
#[derive(Debug, Error)]
|
||||
#[derive(Debug)]
|
||||
pub enum TemplateError {
|
||||
#[error("template syntax error{location}", location = fmt_location(*line))]
|
||||
Syntax {
|
||||
line: Option<u32>,
|
||||
#[source]
|
||||
source: minijinja::Error,
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
#[error(
|
||||
"undefined template variable{expr}{location}",
|
||||
expr = fmt_expr(expression.as_deref()),
|
||||
location = fmt_location(*line),
|
||||
)]
|
||||
UndefinedVariable {
|
||||
expression: Option<String>,
|
||||
line: Option<u32>,
|
||||
#[source]
|
||||
source: minijinja::Error,
|
||||
expression: Option<String>,
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
#[error("template render error{location}", location = fmt_location(*line))]
|
||||
Render {
|
||||
line: Option<u32>,
|
||||
#[source]
|
||||
source: minijinja::Error,
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
}
|
||||
|
||||
impl fmt::Display for TemplateError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::Syntax { line, .. } => {
|
||||
write!(f, "template syntax error{}", fmt_location(*line))
|
||||
}
|
||||
Self::UndefinedVariable {
|
||||
expression, line, ..
|
||||
} => write!(
|
||||
f,
|
||||
"undefined template variable{}{}",
|
||||
fmt_expr(expression.as_deref()),
|
||||
fmt_location(*line)
|
||||
),
|
||||
Self::Render { line, .. } => {
|
||||
write!(f, "template render error{}", fmt_location(*line))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for TemplateError {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
match self {
|
||||
Self::Syntax { source, .. }
|
||||
| Self::UndefinedVariable { source, .. }
|
||||
| Self::Render { source, .. } => Some(source.as_ref()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn fmt_expr(expression: Option<&str>) -> String {
|
||||
expression.map(|e| format!(" `{e}`")).unwrap_or_default()
|
||||
}
|
||||
|
|
@ -161,27 +194,148 @@ fn extract_expression(error: &minijinja::Error) -> Option<String> {
|
|||
impl From<minijinja::Error> for TemplateError {
|
||||
fn from(error: minijinja::Error) -> Self {
|
||||
let line = error.line().and_then(|n| u32::try_from(n).ok());
|
||||
let source_name = error.name().map(str::to_owned);
|
||||
let source_text = error.template_source().map(str::to_owned);
|
||||
let span = error.range().and_then(|range| {
|
||||
let start = range.start;
|
||||
let len = range.end.checked_sub(range.start)?;
|
||||
Some((start, len).into())
|
||||
});
|
||||
let source_code = source_name
|
||||
.as_ref()
|
||||
.zip(source_text.as_ref())
|
||||
.map(|(name, source)| Box::new(NamedSource::new(name.clone(), source.clone())));
|
||||
match error.kind() {
|
||||
ErrorKind::SyntaxError => Self::Syntax {
|
||||
line,
|
||||
source: error,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
},
|
||||
ErrorKind::UndefinedError => {
|
||||
let expression = extract_expression(&error);
|
||||
Self::UndefinedVariable {
|
||||
expression,
|
||||
line,
|
||||
source: error,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
}
|
||||
}
|
||||
_ => Self::Render {
|
||||
line,
|
||||
source: error,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl TemplateError {
|
||||
#[must_use]
|
||||
pub fn expression(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::UndefinedVariable { expression, .. } => expression.as_deref(),
|
||||
Self::Syntax { .. } | Self::Render { .. } => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn line(&self) -> Option<u32> {
|
||||
match self {
|
||||
Self::Syntax { line, .. }
|
||||
| Self::UndefinedVariable { line, .. }
|
||||
| Self::Render { line, .. } => *line,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn source_name(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::Syntax { source_name, .. }
|
||||
| Self::UndefinedVariable { source_name, .. }
|
||||
| Self::Render { source_name, .. } => source_name.as_deref(),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn source_text(&self) -> Option<&str> {
|
||||
match self {
|
||||
Self::Syntax { source_text, .. }
|
||||
| Self::UndefinedVariable { source_text, .. }
|
||||
| Self::Render { source_text, .. } => source_text.as_deref(),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn span(&self) -> Option<SourceSpan> {
|
||||
match self {
|
||||
Self::Syntax { span, .. }
|
||||
| Self::UndefinedVariable { span, .. }
|
||||
| Self::Render { span, .. } => *span,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn column(&self) -> Option<u32> {
|
||||
let source_text = self.source_text()?;
|
||||
let offset = self.span()?.offset();
|
||||
if offset > source_text.len() || !source_text.is_char_boundary(offset) {
|
||||
return None;
|
||||
}
|
||||
let line_start = source_text[..offset]
|
||||
.rfind('\n')
|
||||
.map_or(0, |index| index + 1);
|
||||
u32::try_from(source_text[line_start..offset].chars().count() + 1).ok()
|
||||
}
|
||||
|
||||
fn source_code_ref(&self) -> Option<&NamedSource<String>> {
|
||||
match self {
|
||||
Self::Syntax { source_code, .. }
|
||||
| Self::UndefinedVariable { source_code, .. }
|
||||
| Self::Render { source_code, .. } => source_code.as_deref(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl miette::Diagnostic for TemplateError {
|
||||
fn code<'a>(&'a self) -> Option<Box<dyn fmt::Display + 'a>> {
|
||||
let code = match self {
|
||||
Self::Syntax { .. } => "fabro::template::syntax",
|
||||
Self::UndefinedVariable { .. } => "fabro::template::undefined_variable",
|
||||
Self::Render { .. } => "fabro::template::render",
|
||||
};
|
||||
Some(Box::new(code))
|
||||
}
|
||||
|
||||
fn source_code(&self) -> Option<&dyn SourceCode> {
|
||||
self.source_code_ref()
|
||||
.map(|source| source as &dyn SourceCode)
|
||||
}
|
||||
|
||||
fn labels(&self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + '_>> {
|
||||
let span = self.span()?;
|
||||
let label = match self {
|
||||
Self::UndefinedVariable { expression, .. } => expression.as_ref().map_or_else(
|
||||
|| "undefined variable".to_string(),
|
||||
|expr| format!("`{expr}`"),
|
||||
),
|
||||
Self::Syntax { .. } => "syntax error".to_string(),
|
||||
Self::Render { .. } => "render error".to_string(),
|
||||
};
|
||||
Some(Box::new(
|
||||
vec![LabeledSpan::new_primary_with_span(Some(label), span)].into_iter(),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `true` when the string contains MiniJinja delimiter syntax.
|
||||
#[must_use]
|
||||
pub fn contains_template_syntax(template: &str) -> bool {
|
||||
|
|
@ -195,7 +349,15 @@ fn is_plain_text(template: &str) -> bool {
|
|||
}
|
||||
|
||||
pub fn render(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(template, ctx, UndefinedBehavior::Strict)
|
||||
render_with(None, template, ctx, UndefinedBehavior::Strict)
|
||||
}
|
||||
|
||||
pub fn render_named(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(Some(name.into()), template, ctx, UndefinedBehavior::Strict)
|
||||
}
|
||||
|
||||
/// Render with chainable undefined handling: undefined variables and attribute
|
||||
|
|
@ -203,10 +365,24 @@ pub fn render(template: &str, ctx: &TemplateContext) -> Result<String, TemplateE
|
|||
/// passes (e.g. manifest scanning, `fabro validate` on a bare `.fabro`) where
|
||||
/// the user has not yet bound inputs — strict checking happens elsewhere.
|
||||
pub fn render_lenient(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(template, ctx, UndefinedBehavior::Chainable)
|
||||
render_with(None, template, ctx, UndefinedBehavior::Chainable)
|
||||
}
|
||||
|
||||
pub fn render_lenient_named(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(
|
||||
Some(name.into()),
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Chainable,
|
||||
)
|
||||
}
|
||||
|
||||
fn render_with(
|
||||
name: Option<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
undefined: UndefinedBehavior,
|
||||
|
|
@ -218,8 +394,11 @@ fn render_with(
|
|||
env.set_undefined_behavior(undefined);
|
||||
env.set_auto_escape_callback(|_| AutoEscape::None);
|
||||
env.set_debug(true);
|
||||
env.render_str(template, ctx.clone().into_value())
|
||||
.map_err(TemplateError::from)
|
||||
match name {
|
||||
Some(name) => env.render_named_str(&name, template, ctx.clone().into_value()),
|
||||
None => env.render_str(template, ctx.clone().into_value()),
|
||||
}
|
||||
.map_err(TemplateError::from)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -322,6 +501,40 @@ mod tests {
|
|||
assert!(matches!(err, TemplateError::Syntax { .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_named_reports_source_name_expression_and_span() {
|
||||
let ctx = TemplateContext::new();
|
||||
let err = render_named("prompts/test.md", "Hello {{ inputs.foo }}", &ctx).unwrap_err();
|
||||
|
||||
let TemplateError::UndefinedVariable {
|
||||
expression,
|
||||
line,
|
||||
source_name,
|
||||
span,
|
||||
..
|
||||
} = err
|
||||
else {
|
||||
panic!("expected undefined variable error");
|
||||
};
|
||||
|
||||
assert_eq!(expression.as_deref(), Some("inputs.foo"));
|
||||
assert_eq!(line, Some(1));
|
||||
assert_eq!(source_name.as_deref(), Some("prompts/test.md"));
|
||||
assert!(span.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_lenient_named_preserves_source_name_for_syntax_errors() {
|
||||
let ctx = TemplateContext::new();
|
||||
let err = render_lenient_named("workflow.fabro", "{{ unterminated", &ctx).unwrap_err();
|
||||
|
||||
let TemplateError::Syntax { source_name, .. } = err else {
|
||||
panic!("expected syntax error");
|
||||
};
|
||||
|
||||
assert_eq!(source_name.as_deref(), Some("workflow.fabro"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_undefined_variables_in_strict_mode() {
|
||||
let ctx = TemplateContext::new();
|
||||
|
|
|
|||
|
|
@ -15,12 +15,46 @@ pub enum Severity {
|
|||
/// A validation diagnostic produced by a lint rule.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Diagnostic {
|
||||
pub rule: String,
|
||||
pub severity: Severity,
|
||||
pub message: String,
|
||||
pub node_id: Option<String>,
|
||||
pub edge: Option<(String, String)>,
|
||||
pub fix: Option<String>,
|
||||
pub rule: String,
|
||||
pub severity: Severity,
|
||||
pub message: String,
|
||||
pub node_id: Option<String>,
|
||||
pub edge: Option<(String, String)>,
|
||||
pub fix: Option<String>,
|
||||
pub source_path: Option<String>,
|
||||
pub line: Option<u32>,
|
||||
pub column: Option<u32>,
|
||||
pub span_start: Option<usize>,
|
||||
pub span_len: Option<usize>,
|
||||
#[serde(default)]
|
||||
pub related: Vec<RelatedDiagnostic>,
|
||||
}
|
||||
|
||||
impl Default for Diagnostic {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
rule: String::new(),
|
||||
severity: Severity::Info,
|
||||
message: String::new(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
source_path: None,
|
||||
line: None,
|
||||
column: None,
|
||||
span_start: None,
|
||||
span_len: None,
|
||||
related: Vec::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct RelatedDiagnostic {
|
||||
pub message: String,
|
||||
pub source_path: Option<String>,
|
||||
pub line: Option<u32>,
|
||||
pub column: Option<u32>,
|
||||
}
|
||||
|
||||
/// A lint rule that validates a graph.
|
||||
|
|
@ -254,12 +288,14 @@ reasoning = false
|
|||
}
|
||||
fn apply(&self, _graph: &Graph) -> Vec<Diagnostic> {
|
||||
vec![Diagnostic {
|
||||
rule: "always_warn".to_string(),
|
||||
rule: "always_warn".to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: "custom warning".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
message: "custom warning".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
|
||||
..Diagnostic::default()
|
||||
}]
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,7 +36,8 @@ impl LintRule for Rule {
|
|||
fix: Some(
|
||||
"Add at least one unconditional edge as a fallback".to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
|
|
|
|||
|
|
@ -22,29 +22,33 @@ impl LintRule for Rule {
|
|||
Some(Err(_)) => {
|
||||
let expected = LlmBackend::expected_values();
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"unsupported LLM backend \"{backend}\"; expected one of: {expected}"
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {expected}")),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {expected}")),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
Some(Ok(LlmBackend::Acp)) if acp_command_missing(node) => {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: "backend=\"acp\" requires acp_command because Fabro does \
|
||||
message: "backend=\"acp\" requires acp_command because Fabro does \
|
||||
not install ACP agents"
|
||||
.to_string(),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(
|
||||
"Set acp_command to a stdio ACP command available in the sandbox"
|
||||
.to_string(),
|
||||
),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
Some(Ok(_)) | None => {}
|
||||
|
|
|
|||
|
|
@ -25,19 +25,21 @@ impl LintRule for Rule {
|
|||
}
|
||||
if let Err(e) = parse_condition(condition) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Condition '{condition}' on edge {} -> {} failed parse: {e}",
|
||||
edge.from, edge.to
|
||||
),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(
|
||||
"Use key=value, key!=value, key>value, key contains value, \
|
||||
key matches pattern, or bare key syntax"
|
||||
.to_string(),
|
||||
),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,12 +23,14 @@ impl LintRule for Rule {
|
|||
return Vec::new();
|
||||
}
|
||||
vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Graph has invalid rankdir '{rankdir}'"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", VALID_DIRECTIONS.join(", "))),
|
||||
message: format!("Graph has invalid rankdir '{rankdir}'"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", VALID_DIRECTIONS.join(", "))),
|
||||
|
||||
..Diagnostic::default()
|
||||
}]
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,28 +18,32 @@ impl LintRule for Rule {
|
|||
for edge in &graph.edges {
|
||||
if !graph.nodes.contains_key(&edge.to) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Edge from '{}' targets non-existent node '{}'",
|
||||
edge.from, edge.to
|
||||
),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(format!("Define node '{}' or fix the edge target", edge.to)),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(format!("Define node '{}' or fix the edge target", edge.to)),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
if !graph.nodes.contains_key(&edge.from) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!("Edge source '{}' references non-existent node", edge.from),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(format!(
|
||||
message: format!("Edge source '{}' references non-existent node", edge.from),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(format!(
|
||||
"Define node '{}' or fix the edge source",
|
||||
edge.from
|
||||
)),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,16 +25,18 @@ impl LintRule for Rule {
|
|||
let outgoing = graph.outgoing_edges(&node.id);
|
||||
if !outgoing.is_empty() {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Exit node '{}' has {} outgoing edge(s) but must have none",
|
||||
node.id,
|
||||
outgoing.len()
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Remove outgoing edges from the exit node".to_string()),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Remove outgoing edges from the exit node".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -32,15 +32,17 @@ impl LintRule for Rule {
|
|||
if let Some(fidelity) = node.fidelity() {
|
||||
if fidelity.parse::<Fidelity>().is_err() {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Node '{}' has invalid fidelity mode '{fidelity}'",
|
||||
node.id
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(Self::fix_message()),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(Self::fix_message()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -49,15 +51,17 @@ impl LintRule for Rule {
|
|||
if let Some(fidelity) = edge.fidelity() {
|
||||
if fidelity.parse::<Fidelity>().is_err() {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Edge {} -> {} has invalid fidelity mode '{fidelity}'",
|
||||
edge.from, edge.to
|
||||
),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(Self::fix_message()),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(Self::fix_message()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -65,12 +69,14 @@ impl LintRule for Rule {
|
|||
if let Some(fidelity) = graph.default_fidelity() {
|
||||
if fidelity.parse::<Fidelity>().is_err() {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Graph has invalid default_fidelity '{fidelity}'"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(Self::fix_message()),
|
||||
message: format!("Graph has invalid default_fidelity '{fidelity}'"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(Self::fix_message()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,7 +35,8 @@ impl LintRule for Rule {
|
|||
fix: Some(
|
||||
"Remove extra freeform=true edges so at most one remains".to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,7 +34,8 @@ impl LintRule for Rule {
|
|||
fix: Some(
|
||||
"Add retry_target or fallback_retry_target attribute".to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,26 +19,30 @@ impl LintRule for Rule {
|
|||
for node in graph.nodes.values() {
|
||||
if let Some(AttrValue::String(message)) = node.attrs.get("import_error") {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: message.clone(),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Fix the imported workflow or import path".to_string()),
|
||||
message: message.clone(),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Fix the imported workflow or import path".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
|
||||
if node.attrs.contains_key("import") {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: "unresolved import (no base directory available)".to_string(),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(
|
||||
message: "unresolved import (no base directory available)".to_string(),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(
|
||||
"Load the workflow from a file so imports can resolve relative to it"
|
||||
.to_string(),
|
||||
),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ pub(super) fn check_model_known(
|
|||
node_id,
|
||||
edge: None,
|
||||
fix: Some("Use a model ID from `fabro model list`".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -48,5 +50,7 @@ pub(super) fn check_provider_known(
|
|||
node_id,
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {valid_str}")),
|
||||
|
||||
..Diagnostic::default()
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -48,7 +48,8 @@ impl LintRule for Rule {
|
|||
"Add an unconditional edge as a safety net for unmatched outcomes"
|
||||
.to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
|
|
|
|||
|
|
@ -25,15 +25,14 @@ impl LintRule for Rule {
|
|||
.is_some_and(|l| !l.is_empty());
|
||||
if !has_prompt && !has_label {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
"LLM node '{}' has no prompt or label attribute",
|
||||
node.id
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Add a prompt or label attribute".to_string()),
|
||||
message: format!("LLM node '{}' has no prompt or label attribute", node.id),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Add a prompt or label attribute".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,7 +36,8 @@ impl LintRule for Rule {
|
|||
fix: Some(
|
||||
"Remove the condition attributes from outgoing edges, or remove selection=\"random\" from the node".to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
|
|
|
|||
|
|
@ -44,14 +44,16 @@ impl LintRule for Rule {
|
|||
unreachable
|
||||
.into_iter()
|
||||
.map(|node_id| Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Node '{node_id}' is not reachable from the start node"),
|
||||
node_id: Some(node_id.to_string()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
message: format!("Node '{node_id}' is not reachable from the start node"),
|
||||
node_id: Some(node_id.to_string()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
"Add an edge path from the start node to '{node_id}'"
|
||||
)),
|
||||
|
||||
..Diagnostic::default()
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,19 +23,21 @@ impl LintRule for Rule {
|
|||
.values()
|
||||
.filter(|node| DOT_RESERVED_KEYWORDS.contains(&node.id.to_lowercase().as_str()))
|
||||
.map(|node| Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Node ID '{}' is a DOT reserved keyword and may cause parsing failures",
|
||||
node.id
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
"Rename '{}' to '{}_step' or another non-reserved ID",
|
||||
node.id,
|
||||
node.id.to_lowercase()
|
||||
)),
|
||||
|
||||
..Diagnostic::default()
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,32 +19,36 @@ impl LintRule for Rule {
|
|||
if let Some(target) = node.retry_target() {
|
||||
if !graph.nodes.contains_key(target) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Node '{}' has retry_target '{}' that does not exist",
|
||||
node.id, target
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Define node '{target}' or fix retry_target")),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Define node '{target}' or fix retry_target")),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
if let Some(target) = node.fallback_retry_target() {
|
||||
if !graph.nodes.contains_key(target) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Node '{}' has fallback_retry_target '{}' that does not exist",
|
||||
node.id, target
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
"Define node '{target}' or fix fallback_retry_target"
|
||||
)),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -52,28 +56,32 @@ impl LintRule for Rule {
|
|||
if let Some(target) = graph.retry_target() {
|
||||
if !graph.nodes.contains_key(target) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Graph has retry_target '{target}' that does not exist"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!("Define node '{target}' or fix graph retry_target")),
|
||||
message: format!("Graph has retry_target '{target}' that does not exist"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!("Define node '{target}' or fix graph retry_target")),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
if let Some(target) = graph.fallback_retry_target() {
|
||||
if !graph.nodes.contains_key(target) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Graph has fallback_retry_target '{target}' that does not exist"
|
||||
),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
"Define node '{target}' or fix graph fallback_retry_target"
|
||||
)),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,7 +60,8 @@ impl LintRule for Rule {
|
|||
"Use a relative path; the engine sets the working directory to the worktree automatically"
|
||||
.to_string(),
|
||||
),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
diagnostics
|
||||
|
|
|
|||
|
|
@ -21,12 +21,14 @@ impl LintRule for Rule {
|
|||
if let Some(sel) = node.attrs.get("selection").and_then(AttrValue::as_str) {
|
||||
if !VALID_SELECTIONS.contains(&sel) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Node '{}' has invalid selection mode '{sel}'", node.id),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", VALID_SELECTIONS.join(", "))),
|
||||
message: format!("Node '{}' has invalid selection mode '{sel}'", node.id),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", VALID_SELECTIONS.join(", "))),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,16 +20,18 @@ impl LintRule for Rule {
|
|||
let incoming = graph.incoming_edges(&start.id);
|
||||
if !incoming.is_empty() {
|
||||
return vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Start node '{}' has {} incoming edge(s) but must have none",
|
||||
start.id,
|
||||
incoming.len()
|
||||
),
|
||||
node_id: Some(start.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Remove incoming edges to the start node".to_string()),
|
||||
node_id: Some(start.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Remove incoming edges to the start node".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}];
|
||||
}
|
||||
Vec::new()
|
||||
|
|
|
|||
|
|
@ -21,26 +21,30 @@ impl LintRule for Rule {
|
|||
.count();
|
||||
if start_count == 0 {
|
||||
return vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message:
|
||||
"Pipeline must have exactly one start node (shape=Mdiamond or id start/Start)"
|
||||
.to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Add a node with shape=Mdiamond or id 'start'".to_string()),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Add a node with shape=Mdiamond or id 'start'".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}];
|
||||
}
|
||||
if start_count > 1 {
|
||||
return vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Pipeline has {start_count} start nodes but must have exactly one"
|
||||
),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Remove extra start nodes".to_string()),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Remove extra start nodes".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}];
|
||||
}
|
||||
Vec::new()
|
||||
|
|
|
|||
|
|
@ -22,12 +22,14 @@ impl LintRule for Rule {
|
|||
match parse_stylesheet(stylesheet) {
|
||||
Ok(_) => Vec::new(),
|
||||
Err(e) => vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!("Model stylesheet parse error: {e}"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Fix the model_stylesheet syntax".to_string()),
|
||||
message: format!("Model stylesheet parse error: {e}"),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Fix the model_stylesheet syntax".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}],
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,26 +27,30 @@ impl LintRule for Rule {
|
|||
.count();
|
||||
if terminal_count == 0 {
|
||||
return vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message:
|
||||
"Pipeline must have exactly one terminal node (shape=Msquare or id exit/end)"
|
||||
.to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Add a node with shape=Msquare or id 'exit'/'end'".to_string()),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Add a node with shape=Msquare or id 'exit'/'end'".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}];
|
||||
}
|
||||
if terminal_count > 1 {
|
||||
return vec![Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Pipeline must have exactly one terminal node, found {terminal_count}"
|
||||
),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Remove extra terminal nodes so exactly one remains".to_string()),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Remove extra terminal nodes so exactly one remains".to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
}];
|
||||
}
|
||||
Vec::new()
|
||||
|
|
|
|||
|
|
@ -25,15 +25,17 @@ impl LintRule for Rule {
|
|||
if node.thread_id().is_some() && node.fidelity() != Some("full") && !graph_default_full
|
||||
{
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Node '{}' has thread_id but fidelity is not 'full'",
|
||||
node.id
|
||||
),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -45,15 +47,17 @@ impl LintRule for Rule {
|
|||
graph.nodes.get(&edge.to).and_then(|n| n.fidelity()) == Some("full");
|
||||
if !edge_full && !target_full && !graph_default_full {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
message: format!(
|
||||
"Edge {} -> {} has thread_id but fidelity is not 'full'",
|
||||
edge.from, edge.to
|
||||
),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
node_id: None,
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -61,12 +65,14 @@ impl LintRule for Rule {
|
|||
|
||||
if graph.default_thread().is_some() && !graph_default_full {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: "Graph has default_thread but default_fidelity is not 'full'".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
message: "Graph has default_thread but default_fidelity is not 'full'".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some(Self::FIX.to_string()),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -19,12 +19,14 @@ impl LintRule for Rule {
|
|||
if let Some(node_type) = node.node_type() {
|
||||
if !is_known_handler_type(node_type) {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("Node '{}' has unrecognized type '{node_type}'", node.id),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", KNOWN_HANDLER_TYPES.join(", "))),
|
||||
message: format!("Node '{}' has unrecognized type '{node_type}'", node.id),
|
||||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some(format!("Use one of: {}", KNOWN_HANDLER_TYPES.join(", "))),
|
||||
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -29,7 +29,8 @@ impl LintRule for Rule {
|
|||
node_id: Some(node.id.clone()),
|
||||
edge: None,
|
||||
fix: Some("Check that the path is relative to the workflow file's directory and the file exists".to_string()),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -43,7 +44,8 @@ impl LintRule for Rule {
|
|||
node_id: None,
|
||||
edge: None,
|
||||
fix: Some("Check that the path is relative to the workflow file's directory and the file exists".to_string()),
|
||||
});
|
||||
|
||||
..Diagnostic::default()});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ md5.workspace = true
|
|||
hex.workspace = true
|
||||
sha2 = { workspace = true }
|
||||
mime_guess.workspace = true
|
||||
miette.workspace = true
|
||||
shlex = "1"
|
||||
git2.workspace = true
|
||||
tokio-util.workspace = true
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
use std::fmt;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_graphviz::Error as GraphvizError;
|
||||
use fabro_llm::{Error as LlmError, ProviderErrorKind};
|
||||
use fabro_template::TemplateError;
|
||||
pub use fabro_types::failure_signature::FailureSignature;
|
||||
pub use fabro_types::outcome::FailureCategory;
|
||||
use fabro_types::{ExecOutputTail, FailureReason, RunFailure};
|
||||
|
|
@ -99,6 +103,55 @@ const STRUCTURAL_HINTS: &[&str] = &[
|
|||
"scope violation",
|
||||
];
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct SharedTemplateError(Arc<TemplateError>);
|
||||
|
||||
impl SharedTemplateError {
|
||||
#[must_use]
|
||||
pub fn new(error: TemplateError) -> Self {
|
||||
Self(Arc::new(error))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn inner(&self) -> &TemplateError {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for SharedTemplateError {
|
||||
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
fmt::Display::fmt(&self.0, formatter)
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for SharedTemplateError {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
self.0.source()
|
||||
}
|
||||
}
|
||||
|
||||
impl miette::Diagnostic for SharedTemplateError {
|
||||
fn code<'a>(&'a self) -> Option<Box<dyn fmt::Display + 'a>> {
|
||||
miette::Diagnostic::code(self.inner())
|
||||
}
|
||||
|
||||
fn help<'a>(&'a self) -> Option<Box<dyn fmt::Display + 'a>> {
|
||||
miette::Diagnostic::help(self.inner())
|
||||
}
|
||||
|
||||
fn source_code(&self) -> Option<&dyn miette::SourceCode> {
|
||||
miette::Diagnostic::source_code(self.inner())
|
||||
}
|
||||
|
||||
fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
|
||||
miette::Diagnostic::labels(self.inner())
|
||||
}
|
||||
|
||||
fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> {
|
||||
miette::Diagnostic::diagnostic_source(self.inner())
|
||||
}
|
||||
}
|
||||
|
||||
/// Classify a failure reason string using heuristics.
|
||||
///
|
||||
/// This is the fallback when structured error information is not available
|
||||
|
|
@ -205,6 +258,13 @@ pub enum Error {
|
|||
#[error("Validation failed")]
|
||||
ValidationFailed { diagnostics: Vec<Diagnostic> },
|
||||
|
||||
#[error("{message}")]
|
||||
Template {
|
||||
message: String,
|
||||
#[source]
|
||||
source: SharedTemplateError,
|
||||
},
|
||||
|
||||
#[error("Engine error: {message}")]
|
||||
Engine {
|
||||
message: String,
|
||||
|
|
@ -262,6 +322,13 @@ impl Error {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn template(message: impl Into<String>, source: TemplateError) -> Self {
|
||||
Self::Template {
|
||||
message: message.into(),
|
||||
source: SharedTemplateError::new(source),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn handler_with_exec_output_tail(
|
||||
message: impl Into<String>,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
|
|
@ -345,6 +412,7 @@ impl Error {
|
|||
Self::Engine { source, .. } | Self::Handler { source, .. } => source
|
||||
.as_ref()
|
||||
.map_or_else(Vec::new, |source| collect_chain(source)),
|
||||
Self::Template { source, .. } => collect_chain(source),
|
||||
Self::Llm(err) => collect_causes(err),
|
||||
_ => Vec::new(),
|
||||
}
|
||||
|
|
@ -370,6 +438,7 @@ impl Error {
|
|||
Self::Parse(_)
|
||||
| Self::Validation(_)
|
||||
| Self::ValidationFailed { .. }
|
||||
| Self::Template { .. }
|
||||
| Self::Stylesheet(_)
|
||||
| Self::Checkpoint(_)
|
||||
| Self::Precondition(_)
|
||||
|
|
@ -389,6 +458,7 @@ impl Error {
|
|||
Self::Parse(_)
|
||||
| Self::Validation(_)
|
||||
| Self::ValidationFailed { .. }
|
||||
| Self::Template { .. }
|
||||
| Self::Stylesheet(_)
|
||||
| Self::Checkpoint(_)
|
||||
| Self::Unsupported(_) => FailureCategory::Deterministic,
|
||||
|
|
@ -448,6 +518,43 @@ impl Error {
|
|||
}
|
||||
}
|
||||
|
||||
impl miette::Diagnostic for Error {
|
||||
fn code<'a>(&'a self) -> Option<Box<dyn fmt::Display + 'a>> {
|
||||
match self {
|
||||
Self::Template { source, .. } => miette::Diagnostic::code(source),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn help<'a>(&'a self) -> Option<Box<dyn fmt::Display + 'a>> {
|
||||
match self {
|
||||
Self::Template { source, .. } => miette::Diagnostic::help(source),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn source_code(&self) -> Option<&dyn miette::SourceCode> {
|
||||
match self {
|
||||
Self::Template { source, .. } => miette::Diagnostic::source_code(source),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn labels(&self) -> Option<Box<dyn Iterator<Item = miette::LabeledSpan> + '_>> {
|
||||
match self {
|
||||
Self::Template { source, .. } => miette::Diagnostic::labels(source),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> {
|
||||
match self {
|
||||
Self::Template { source, .. } => Some(source),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn run_failure_from_error(error: &Error, reason: FailureReason) -> RunFailure {
|
||||
RunFailure {
|
||||
|
|
@ -490,7 +597,8 @@ impl From<GraphvizError> for Error {
|
|||
|
||||
impl From<fabro_template::TemplateError> for Error {
|
||||
fn from(err: fabro_template::TemplateError) -> Self {
|
||||
Self::Validation(collect_chain(&err).join(": "))
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
Self::template(format!("template expansion failed: {rendered}"), err)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -570,17 +678,43 @@ mod tests {
|
|||
fn validation_failed_display() {
|
||||
let err = Error::ValidationFailed {
|
||||
diagnostics: vec![Diagnostic {
|
||||
rule: "test".to_string(),
|
||||
rule: "test".to_string(),
|
||||
severity: fabro_validate::Severity::Error,
|
||||
message: "missing start node".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
message: "missing start node".to_string(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
|
||||
..Diagnostic::default()
|
||||
}],
|
||||
};
|
||||
assert_eq!(err.to_string(), "Validation failed");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn template_error_variant_preserves_source_chain() {
|
||||
let template_err = fabro_template::render_named(
|
||||
"workflow.fabro",
|
||||
"{{ inputs.missing }}",
|
||||
&fabro_template::TemplateContext::new(),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
let err = Error::template("template expansion failed", template_err);
|
||||
let chain = collect_chain(&err);
|
||||
|
||||
assert!(
|
||||
chain
|
||||
.iter()
|
||||
.any(|part| part.contains("template expansion failed"))
|
||||
);
|
||||
assert!(
|
||||
chain
|
||||
.iter()
|
||||
.any(|part| part.contains("undefined template variable"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn engine_error_display() {
|
||||
let err = Error::engine("no outgoing edge");
|
||||
|
|
@ -1807,12 +1941,14 @@ mod tests {
|
|||
Error::Validation("bad".into()),
|
||||
Error::ValidationFailed {
|
||||
diagnostics: vec![Diagnostic {
|
||||
rule: "test".into(),
|
||||
rule: "test".into(),
|
||||
severity: fabro_validate::Severity::Error,
|
||||
message: "bad".into(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
message: "bad".into(),
|
||||
node_id: None,
|
||||
edge: None,
|
||||
fix: None,
|
||||
|
||||
..Diagnostic::default()
|
||||
}],
|
||||
},
|
||||
Error::engine("engine err"),
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ use crate::pipeline::{self, Persisted, TransformOptions, Validated};
|
|||
use crate::records::RunSpec;
|
||||
use crate::run_lookup::default_scratch_base;
|
||||
use crate::run_materialization::materialize_run;
|
||||
use crate::transforms::Transform;
|
||||
use crate::transforms::{RenderMode, Transform};
|
||||
use crate::workflow_bundle::{RunDefinition, WorkflowBundle};
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
|
|
@ -66,6 +66,7 @@ struct PersistCreateOptions {
|
|||
run_id: Option<RunId>,
|
||||
run_dir: Option<PathBuf>,
|
||||
workflow_slug: Option<String>,
|
||||
source_name: Option<String>,
|
||||
labels: HashMap<String, String>,
|
||||
source_directory: Option<String>,
|
||||
git: Option<GitContext>,
|
||||
|
|
@ -132,6 +133,10 @@ pub async fn create(
|
|||
};
|
||||
|
||||
let raw_source = resolved.raw_source.clone();
|
||||
let source_name = resolved
|
||||
.dot_path
|
||||
.as_ref()
|
||||
.map(|path| path.display().to_string());
|
||||
let persisted = spawn_blocking(move || {
|
||||
create_from_source(
|
||||
&raw_source,
|
||||
|
|
@ -140,6 +145,7 @@ pub async fn create(
|
|||
run_id: Some(run_id),
|
||||
run_dir: Some(persisted_run_dir),
|
||||
workflow_slug: workflow_slug.or(resolved_workflow_slug),
|
||||
source_name,
|
||||
labels,
|
||||
source_directory,
|
||||
git,
|
||||
|
|
@ -288,11 +294,13 @@ fn create_from_source(
|
|||
) -> Result<Persisted, Error> {
|
||||
let mut validated = preprocess_and_validate(
|
||||
dot_source,
|
||||
options.source_name.clone(),
|
||||
current_dir,
|
||||
file_resolver,
|
||||
Vec::new(),
|
||||
Some(&options.settings),
|
||||
goal_override,
|
||||
RenderMode::Structural,
|
||||
&options.catalog,
|
||||
)?;
|
||||
|
||||
|
|
@ -308,11 +316,13 @@ fn create_from_source(
|
|||
|
||||
pub(super) fn preprocess_and_validate(
|
||||
dot_source: &str,
|
||||
source_name: Option<String>,
|
||||
current_dir: Option<PathBuf>,
|
||||
file_resolver: Option<Arc<dyn FileResolver>>,
|
||||
custom_transforms: Vec<Box<dyn Transform>>,
|
||||
settings: Option<&WorkflowSettings>,
|
||||
goal_override: Option<&str>,
|
||||
render_mode: RenderMode,
|
||||
catalog: &Arc<Catalog>,
|
||||
) -> Result<Validated, Error> {
|
||||
let inputs = run_inputs(settings);
|
||||
|
|
@ -323,6 +333,8 @@ pub(super) fn preprocess_and_validate(
|
|||
current_dir,
|
||||
file_resolver,
|
||||
inputs,
|
||||
source_name,
|
||||
render_mode,
|
||||
custom_transforms,
|
||||
catalog: Arc::clone(catalog),
|
||||
})?;
|
||||
|
|
@ -353,6 +365,7 @@ fn persist_validated(
|
|||
run_id,
|
||||
run_dir,
|
||||
workflow_slug,
|
||||
source_name: _,
|
||||
labels,
|
||||
source_directory,
|
||||
git,
|
||||
|
|
@ -415,6 +428,7 @@ mod tests {
|
|||
use fabro_types::settings::InterpString;
|
||||
use fabro_types::settings::run::RunMode;
|
||||
use fabro_types::{WorkflowSettings, fixtures};
|
||||
use fabro_util::error::collect_chain;
|
||||
use fabro_validate::Severity;
|
||||
use object_store::local::LocalFileSystem;
|
||||
use object_store::memory::InMemory;
|
||||
|
|
@ -527,6 +541,73 @@ mod tests {
|
|||
assert_eq!(diagnostic.severity, Severity::Error);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_template_error_for_inline_prompt_names_workflow_file_and_node() {
|
||||
let dot = r#"digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_inline_prompt [label="moo" prompt="{{ inputs.foo }}"]
|
||||
start -> test_inline_prompt -> exit
|
||||
}"#;
|
||||
|
||||
let result = preprocess_and_validate(
|
||||
dot,
|
||||
Some("workflow.fabro".to_string()),
|
||||
Some(PathBuf::from(".")),
|
||||
None,
|
||||
Vec::new(),
|
||||
Some(&WorkflowSettings::default()),
|
||||
None,
|
||||
RenderMode::Strict,
|
||||
&test_catalog(),
|
||||
);
|
||||
let Err(err) = result else {
|
||||
panic!("expected strict mode to hard-fail on unbound inline prompt");
|
||||
};
|
||||
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
assert!(rendered.contains("workflow.fabro"), "{rendered}");
|
||||
assert!(rendered.contains("test_inline_prompt"), "{rendered}");
|
||||
assert!(rendered.contains("prompt"), "{rendered}");
|
||||
assert!(!rendered.contains("<string>"), "{rendered}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_prompt_template_error_names_prompt_file_and_node() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let prompt_path = dir.path().join("test.md");
|
||||
std::fs::write(&prompt_path, "{{ inputs.foo }}").unwrap();
|
||||
let dot = r#"digraph ValidatePlan {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
test_imported_prompt [label="moo" prompt="@test.md"]
|
||||
start -> test_imported_prompt -> exit
|
||||
}"#;
|
||||
|
||||
let result = preprocess_and_validate(
|
||||
dot,
|
||||
Some("workflow.fabro".to_string()),
|
||||
Some(dir.path().to_path_buf()),
|
||||
Some(Arc::new(crate::file_resolver::FilesystemFileResolver::new(
|
||||
None,
|
||||
))),
|
||||
Vec::new(),
|
||||
Some(&WorkflowSettings::default()),
|
||||
None,
|
||||
RenderMode::Strict,
|
||||
&test_catalog(),
|
||||
);
|
||||
let Err(err) = result else {
|
||||
panic!("expected strict mode to hard-fail on unbound imported prompt");
|
||||
};
|
||||
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
assert!(rendered.contains("test.md"), "{rendered}");
|
||||
assert!(rendered.contains("test_imported_prompt"), "{rendered}");
|
||||
assert!(rendered.contains("prompt"), "{rendered}");
|
||||
assert!(!rendered.contains("<string>"), "{rendered}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_applies_variable_expansion() {
|
||||
let dot = r#"digraph Test {
|
||||
|
|
|
|||
|
|
@ -23,3 +23,4 @@ pub use timeline::{ForkTarget, RunTimeline, TimelineEntry, build_timeline, timel
|
|||
pub use validate::{ValidateInput, validate};
|
||||
|
||||
pub use crate::pipeline::{DevcontainerSpec, LlmSpec, SandboxEnvSpec};
|
||||
pub use crate::transforms::RenderMode;
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use fabro_types::WorkflowSettings;
|
|||
use super::create::preprocess_and_validate;
|
||||
use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow};
|
||||
use crate::error::Error;
|
||||
use crate::operations::RenderMode;
|
||||
use crate::pipeline::Validated;
|
||||
use crate::transforms::Transform;
|
||||
|
||||
|
|
@ -32,11 +33,16 @@ pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
|
|||
|
||||
preprocess_and_validate(
|
||||
&resolved.raw_source,
|
||||
resolved
|
||||
.dot_path
|
||||
.as_ref()
|
||||
.map(|path| path.display().to_string()),
|
||||
resolved.current_dir,
|
||||
resolved.file_resolver,
|
||||
input.custom_transforms,
|
||||
Some(&resolved.settings),
|
||||
resolved.goal_override.as_deref(),
|
||||
RenderMode::Structural,
|
||||
&input.catalog,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,12 +13,25 @@ use crate::transforms::{
|
|||
/// (e.g. goal override) before validation.
|
||||
pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result<Transformed, Error> {
|
||||
let Parsed { graph, source } = parsed;
|
||||
let mut diagnostics = Vec::new();
|
||||
|
||||
// Built-in transforms (PreambleTransform moved to engine execution time)
|
||||
let graph = if let (Some(current_dir), Some(file_resolver)) =
|
||||
(&options.current_dir, &options.file_resolver)
|
||||
{
|
||||
ImportTransform::new(current_dir.clone(), Arc::clone(file_resolver)).apply(graph)?
|
||||
let (graph, transform_diagnostics) = ImportTransform::new(
|
||||
current_dir.clone(),
|
||||
Arc::clone(file_resolver),
|
||||
options.inputs.clone(),
|
||||
)
|
||||
.with_template_options(
|
||||
options.source_name.clone(),
|
||||
Some(source.clone()),
|
||||
options.render_mode,
|
||||
)
|
||||
.apply_with_diagnostics(graph)?;
|
||||
diagnostics.extend(transform_diagnostics);
|
||||
graph
|
||||
} else {
|
||||
graph
|
||||
};
|
||||
|
|
@ -26,13 +39,29 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result<Transform
|
|||
let graph = if let (Some(current_dir), Some(file_resolver)) =
|
||||
(&options.current_dir, &options.file_resolver)
|
||||
{
|
||||
FileInliningTransform::new(current_dir.clone(), Arc::clone(file_resolver)).apply(graph)?
|
||||
let (graph, transform_diagnostics) =
|
||||
FileInliningTransform::new(current_dir.clone(), Arc::clone(file_resolver))
|
||||
.with_template_options(
|
||||
options.inputs.clone(),
|
||||
options.source_name.clone(),
|
||||
Some(source.clone()),
|
||||
options.render_mode,
|
||||
)
|
||||
.apply_with_diagnostics(graph)?;
|
||||
diagnostics.extend(transform_diagnostics);
|
||||
graph
|
||||
} else {
|
||||
graph
|
||||
};
|
||||
|
||||
let (graph, diagnostics) =
|
||||
TemplateTransform::new(options.inputs.clone()).apply_with_diagnostics(graph)?;
|
||||
let (graph, transform_diagnostics) = TemplateTransform {
|
||||
inputs: options.inputs.clone(),
|
||||
source_name: options.source_name.clone(),
|
||||
source_text: Some(source.clone()),
|
||||
render_mode: options.render_mode,
|
||||
}
|
||||
.apply_with_diagnostics(graph)?;
|
||||
diagnostics.extend(transform_diagnostics);
|
||||
let graph = StylesheetApplicationTransform.apply(graph)?;
|
||||
let graph = ModelResolutionTransform::new(Arc::clone(&options.catalog)).apply(graph)?;
|
||||
|
||||
|
|
@ -79,6 +108,8 @@ mod tests {
|
|||
current_dir: None,
|
||||
file_resolver: None,
|
||||
inputs: HashMap::new(),
|
||||
source_name: None,
|
||||
render_mode: crate::operations::RenderMode::Strict,
|
||||
custom_transforms: vec![],
|
||||
catalog: test_catalog(),
|
||||
}
|
||||
|
|
@ -139,6 +170,8 @@ mod tests {
|
|||
current_dir: Some(dir.path().to_path_buf()),
|
||||
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
|
||||
inputs: HashMap::new(),
|
||||
source_name: None,
|
||||
render_mode: crate::operations::RenderMode::Strict,
|
||||
custom_transforms: vec![],
|
||||
catalog: test_catalog(),
|
||||
})
|
||||
|
|
@ -187,6 +220,8 @@ mod tests {
|
|||
"task".to_string(),
|
||||
toml::Value::String("Launch".to_string()),
|
||||
)]),
|
||||
source_name: None,
|
||||
render_mode: crate::operations::RenderMode::Strict,
|
||||
custom_transforms: vec![],
|
||||
catalog: test_catalog(),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ use crate::run_options::{GitCheckpointOptions, LifecycleOptions, RunOptions};
|
|||
use crate::runtime_store::RunStoreHandle;
|
||||
use crate::services::{EngineServices, RunServices};
|
||||
use crate::steering_hub::SteeringHub;
|
||||
use crate::transforms::Transform;
|
||||
use crate::transforms::{RenderMode, Transform};
|
||||
use crate::workflow_bundle::WorkflowBundle;
|
||||
|
||||
/// Output of the PARSE phase.
|
||||
|
|
@ -51,41 +51,6 @@ pub struct Transformed {
|
|||
/// Lint rule name attached to diagnostics for undefined template variables.
|
||||
pub const TEMPLATE_UNDEFINED_VARIABLE_RULE: &str = "template_undefined_variable";
|
||||
|
||||
/// Build a warning diagnostic describing an undefined template variable.
|
||||
/// Shared between the DOT-source render pass (`operations::create`) and the
|
||||
/// per-attribute render pass (`transforms::variable_expansion`).
|
||||
pub(crate) fn template_undefined_variable_diagnostic(
|
||||
expression: Option<&str>,
|
||||
line: Option<u32>,
|
||||
node_id: Option<&str>,
|
||||
) -> Diagnostic {
|
||||
let location = match (node_id, line) {
|
||||
(Some(id), _) => format!(" in node `{id}`"),
|
||||
(None, Some(l)) => format!(" at line {l}"),
|
||||
(None, None) => String::new(),
|
||||
};
|
||||
let (name, message) = match expression {
|
||||
Some(expr) => (
|
||||
expr,
|
||||
format!("undefined template variable `{expr}`{location}"),
|
||||
),
|
||||
None => (
|
||||
"<unknown>",
|
||||
format!("undefined template variable{location}"),
|
||||
),
|
||||
};
|
||||
Diagnostic {
|
||||
rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(),
|
||||
severity: Severity::Warning,
|
||||
message,
|
||||
node_id: node_id.map(str::to_owned),
|
||||
edge: None,
|
||||
fix: Some(format!(
|
||||
"bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=<value>`"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Output of the VALIDATE phase. Always produced (even with errors).
|
||||
/// Caller inspects diagnostics and decides whether to proceed.
|
||||
/// Graph is read-only — use accessors, not direct field access.
|
||||
|
|
@ -365,6 +330,8 @@ pub struct TransformOptions {
|
|||
pub current_dir: Option<PathBuf>,
|
||||
pub file_resolver: Option<Arc<dyn FileResolver>>,
|
||||
pub inputs: HashMap<String, toml::Value>,
|
||||
pub source_name: Option<String>,
|
||||
pub render_mode: RenderMode,
|
||||
pub custom_transforms: Vec<Box<dyn Transform>>,
|
||||
pub catalog: Arc<fabro_model::Catalog>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,11 +15,14 @@ pub fn validate(
|
|||
let Transformed {
|
||||
graph,
|
||||
source,
|
||||
diagnostics: mut transform_diagnostics,
|
||||
mut diagnostics,
|
||||
} = transformed;
|
||||
let lint_diagnostics = fabro_validate::validate_with_catalog(&graph, catalog, extra_rules);
|
||||
transform_diagnostics.extend(lint_diagnostics);
|
||||
Validated::new(graph, source, transform_diagnostics)
|
||||
diagnostics.extend(fabro_validate::validate_with_catalog(
|
||||
&graph,
|
||||
catalog,
|
||||
extra_rules,
|
||||
));
|
||||
Validated::new(graph, source, diagnostics)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -42,6 +45,8 @@ mod tests {
|
|||
current_dir: None,
|
||||
file_resolver: None,
|
||||
inputs: std::collections::HashMap::new(),
|
||||
source_name: None,
|
||||
render_mode: crate::operations::RenderMode::Strict,
|
||||
custom_transforms: vec![],
|
||||
catalog: std::sync::Arc::clone(&catalog),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,12 +1,18 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_graphviz::graph::{AttrValue, Graph};
|
||||
use fabro_template::TemplateContext;
|
||||
use fabro_validate::Diagnostic;
|
||||
|
||||
use super::Transform;
|
||||
use crate::error::Error;
|
||||
use crate::file_resolver::FileResolver;
|
||||
use crate::file_resolver::{FileResolver, ResolvedFile};
|
||||
use crate::static_reference::{ReferenceKind, validate_static_reference};
|
||||
use crate::transforms::variable_expansion::{
|
||||
RenderMode, TemplateRenderTarget, TemplateTransform, render_template_for_target,
|
||||
};
|
||||
|
||||
/// Resolve a potential `@path` file reference.
|
||||
///
|
||||
|
|
@ -30,8 +36,13 @@ pub fn resolve_file_ref(
|
|||
|
||||
/// Inlines `@file` references in node prompts and the graph-level goal.
|
||||
pub struct FileInliningTransform {
|
||||
current_dir: PathBuf,
|
||||
resolver: Arc<dyn FileResolver>,
|
||||
current_dir: PathBuf,
|
||||
resolver: Arc<dyn FileResolver>,
|
||||
inputs: HashMap<String, toml::Value>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
goal_override: Option<String>,
|
||||
render_mode: RenderMode,
|
||||
}
|
||||
|
||||
impl FileInliningTransform {
|
||||
|
|
@ -40,39 +51,154 @@ impl FileInliningTransform {
|
|||
Self {
|
||||
current_dir,
|
||||
resolver,
|
||||
inputs: HashMap::new(),
|
||||
source_name: None,
|
||||
source_text: None,
|
||||
goal_override: None,
|
||||
render_mode: RenderMode::Strict,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_template_options(
|
||||
mut self,
|
||||
inputs: HashMap<String, toml::Value>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
render_mode: RenderMode,
|
||||
) -> Self {
|
||||
self.inputs = inputs;
|
||||
self.source_name = source_name;
|
||||
self.source_text = source_text;
|
||||
self.render_mode = render_mode;
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_goal_override(mut self, goal: Option<String>) -> Self {
|
||||
self.goal_override = goal;
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn apply_with_diagnostics(
|
||||
&self,
|
||||
graph: Graph,
|
||||
) -> Result<(Graph, Vec<Diagnostic>), Error> {
|
||||
let mut graph = graph;
|
||||
let mut diagnostics = Vec::new();
|
||||
self.inline_graph_goal(&mut graph, &mut diagnostics)?;
|
||||
|
||||
let resolved_goal = match &self.goal_override {
|
||||
Some(goal) => goal.clone(),
|
||||
None => TemplateTransform {
|
||||
inputs: self.inputs.clone(),
|
||||
source_name: self.source_name.clone(),
|
||||
source_text: self.source_text.clone(),
|
||||
render_mode: self.render_mode,
|
||||
}
|
||||
.resolved_goal(&graph, &mut diagnostics)?,
|
||||
};
|
||||
let ctx = TemplateContext::new()
|
||||
.with_goal(resolved_goal)
|
||||
.with_inputs(self.inputs.clone());
|
||||
|
||||
for (node_id, node) in &mut graph.nodes {
|
||||
let Some(AttrValue::String(prompt)) = node.attrs.get("prompt") else {
|
||||
continue;
|
||||
};
|
||||
let target = TemplateRenderTarget::node_attr(
|
||||
self.source_name.clone(),
|
||||
node_id.clone(),
|
||||
"prompt",
|
||||
)
|
||||
.with_source_text(self.source_text.as_deref(), prompt);
|
||||
let rendered = render_template_for_target(
|
||||
prompt,
|
||||
&ctx,
|
||||
self.render_mode,
|
||||
&target,
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
let value = self
|
||||
.render_resolved_file_ref(&rendered, &ctx, target, &mut diagnostics)?
|
||||
.unwrap_or(rendered);
|
||||
node.attrs
|
||||
.insert("prompt".to_string(), AttrValue::String(value));
|
||||
}
|
||||
|
||||
Ok((graph, diagnostics))
|
||||
}
|
||||
|
||||
fn inline_graph_goal(
|
||||
&self,
|
||||
graph: &mut Graph,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<(), Error> {
|
||||
let Some(AttrValue::String(goal)) = graph.attrs.get("goal") else {
|
||||
return Ok(());
|
||||
};
|
||||
let ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), goal);
|
||||
let rendered =
|
||||
render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)?;
|
||||
let value = self
|
||||
.render_resolved_file_ref(&rendered, &ctx, target, diagnostics)?
|
||||
.unwrap_or(rendered);
|
||||
graph
|
||||
.attrs
|
||||
.insert("goal".to_string(), AttrValue::String(value));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn render_resolved_file_ref(
|
||||
&self,
|
||||
value: &str,
|
||||
ctx: &TemplateContext,
|
||||
owner_target: TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<Option<String>, Error> {
|
||||
let Some(path_str) = value.strip_prefix('@') else {
|
||||
return Ok(None);
|
||||
};
|
||||
validate_static_reference(path_str, ReferenceKind::FileInline)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
let Some(resolved) = self.resolver.resolve(&self.current_dir, path_str) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let target = owner_target
|
||||
.with_source_name(resolved.path.display().to_string())
|
||||
.with_source_text(Some(&resolved.content), &resolved.content);
|
||||
Ok(Some(render_file_contents(
|
||||
&resolved,
|
||||
ctx,
|
||||
self.render_mode,
|
||||
&target,
|
||||
diagnostics,
|
||||
)?))
|
||||
}
|
||||
}
|
||||
|
||||
impl Transform for FileInliningTransform {
|
||||
fn apply(&self, graph: Graph) -> Result<Graph, Error> {
|
||||
let mut graph = graph;
|
||||
|
||||
// Inline @file refs in node prompts
|
||||
for node in graph.nodes.values_mut() {
|
||||
if let Some(AttrValue::String(prompt)) = node.attrs.get("prompt") {
|
||||
let resolved = resolve_file_ref(prompt, &self.current_dir, self.resolver.as_ref())?;
|
||||
if resolved != *prompt {
|
||||
node.attrs
|
||||
.insert("prompt".to_string(), AttrValue::String(resolved));
|
||||
}
|
||||
}
|
||||
let (graph, diagnostics) = self.apply_with_diagnostics(graph)?;
|
||||
if !diagnostics.is_empty() {
|
||||
return Err(Error::ValidationFailed { diagnostics });
|
||||
}
|
||||
|
||||
// Inline @file refs in graph-level goal
|
||||
if let Some(AttrValue::String(goal)) = graph.attrs.get("goal") {
|
||||
let resolved = resolve_file_ref(goal, &self.current_dir, self.resolver.as_ref())?;
|
||||
if resolved != *goal {
|
||||
graph
|
||||
.attrs
|
||||
.insert("goal".to_string(), AttrValue::String(resolved));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(graph)
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn render_file_contents(
|
||||
resolved: &ResolvedFile,
|
||||
ctx: &TemplateContext,
|
||||
render_mode: RenderMode,
|
||||
target: &TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<String, Error> {
|
||||
render_template_for_target(&resolved.content, ctx, render_mode, target, diagnostics)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![expect(
|
||||
|
|
|
|||
|
|
@ -4,15 +4,24 @@ use std::sync::Arc;
|
|||
|
||||
use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node};
|
||||
use fabro_graphviz::parser;
|
||||
use fabro_template::TemplateContext;
|
||||
use fabro_validate::Diagnostic;
|
||||
|
||||
use super::{FileInliningTransform, Transform};
|
||||
use crate::error::Error;
|
||||
use crate::file_resolver::{FileResolver, ResolvedFile};
|
||||
use crate::static_reference::{ReferenceKind, validate_static_reference};
|
||||
use crate::transforms::variable_expansion::{
|
||||
RenderMode, TemplateRenderTarget, TemplateTransform, render_template_for_target,
|
||||
};
|
||||
|
||||
pub struct ImportTransform {
|
||||
current_dir: PathBuf,
|
||||
resolver: Arc<dyn FileResolver>,
|
||||
inputs: HashMap<String, toml::Value>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
render_mode: RenderMode,
|
||||
}
|
||||
|
||||
struct PlaceholderOptions {
|
||||
|
|
@ -27,6 +36,7 @@ struct PreparedImport {
|
|||
exit_id: String,
|
||||
entry_id: String,
|
||||
exit_predecessor_id: String,
|
||||
diagnostics: Vec<Diagnostic>,
|
||||
}
|
||||
|
||||
enum ImportPrepareError {
|
||||
|
|
@ -42,13 +52,34 @@ impl From<Error> for ImportPrepareError {
|
|||
|
||||
impl ImportTransform {
|
||||
#[must_use]
|
||||
pub fn new(current_dir: PathBuf, resolver: Arc<dyn FileResolver>) -> Self {
|
||||
pub fn new(
|
||||
current_dir: PathBuf,
|
||||
resolver: Arc<dyn FileResolver>,
|
||||
inputs: HashMap<String, toml::Value>,
|
||||
) -> Self {
|
||||
Self {
|
||||
current_dir,
|
||||
resolver,
|
||||
inputs,
|
||||
source_name: None,
|
||||
source_text: None,
|
||||
render_mode: RenderMode::Structural,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_template_options(
|
||||
mut self,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
render_mode: RenderMode,
|
||||
) -> Self {
|
||||
self.source_name = source_name;
|
||||
self.source_text = source_text;
|
||||
self.render_mode = render_mode;
|
||||
self
|
||||
}
|
||||
|
||||
fn collect_import_nodes(graph: &Graph) -> Vec<(String, String)> {
|
||||
graph
|
||||
.nodes
|
||||
|
|
@ -67,11 +98,12 @@ impl ImportTransform {
|
|||
graph: &mut Graph,
|
||||
placeholder_id: &str,
|
||||
import_path: &str,
|
||||
parent_goal: &str,
|
||||
current_base_dir: &Path,
|
||||
import_stack: &mut Vec<PathBuf>,
|
||||
) -> Result<(), Error> {
|
||||
) -> Result<Vec<Diagnostic>, Error> {
|
||||
if !graph.nodes.contains_key(placeholder_id) {
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
if graph
|
||||
|
|
@ -84,20 +116,20 @@ impl ImportTransform {
|
|||
placeholder_id,
|
||||
&format!("import placeholder '{placeholder_id}' cannot have a self-loop"),
|
||||
);
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let placeholder = match Self::placeholder_config(graph, placeholder_id) {
|
||||
Ok(placeholder) => placeholder,
|
||||
Err(message) => {
|
||||
Self::poison_placeholder(graph, placeholder_id, &message);
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
};
|
||||
|
||||
if let Err(error) = validate_static_reference(import_path, ReferenceKind::Import) {
|
||||
Self::poison_placeholder(graph, placeholder_id, &error.to_string());
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let Some(resolved_file) = self.resolver.resolve(current_base_dir, import_path) else {
|
||||
|
|
@ -106,7 +138,7 @@ impl ImportTransform {
|
|||
placeholder_id,
|
||||
&format!("file not found: {import_path}"),
|
||||
);
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
|
||||
if import_stack.contains(&resolved_file.path) {
|
||||
|
|
@ -121,17 +153,18 @@ impl ImportTransform {
|
|||
placeholder_id,
|
||||
&format!("circular import detected: {cycle}"),
|
||||
);
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let prepared = match self.prepare_import(&resolved_file, import_stack) {
|
||||
let prepared = match self.prepare_import(&resolved_file, parent_goal, import_stack) {
|
||||
Ok(prepared) => prepared,
|
||||
Err(ImportPrepareError::Hard(error)) => return Err(error),
|
||||
Err(ImportPrepareError::Soft(message)) => {
|
||||
Self::poison_placeholder(graph, placeholder_id, &message);
|
||||
return Ok(());
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
};
|
||||
let diagnostics = prepared.diagnostics.clone();
|
||||
|
||||
if let Err(message) = Self::splice_import(
|
||||
graph,
|
||||
|
|
@ -143,15 +176,20 @@ impl ImportTransform {
|
|||
Self::poison_placeholder(graph, placeholder_id, &message);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
Ok(diagnostics)
|
||||
}
|
||||
|
||||
fn prepare_import(
|
||||
&self,
|
||||
resolved_file: &ResolvedFile,
|
||||
parent_goal: &str,
|
||||
import_stack: &mut Vec<PathBuf>,
|
||||
) -> Result<PreparedImport, ImportPrepareError> {
|
||||
Self::with_import_stack(import_stack, resolved_file.path.clone(), |import_stack| {
|
||||
let mut diagnostics = Vec::new();
|
||||
let source_name = resolved_file.path.display().to_string();
|
||||
let source_text = resolved_file.content.clone();
|
||||
|
||||
let mut graph = parser::parse(&resolved_file.content).map_err(|error| {
|
||||
ImportPrepareError::Soft(format!(
|
||||
"failed to parse {}: {error}",
|
||||
|
|
@ -163,9 +201,34 @@ impl ImportTransform {
|
|||
.path
|
||||
.parent()
|
||||
.map_or_else(|| PathBuf::from("."), Path::to_path_buf);
|
||||
graph = FileInliningTransform::new(import_base_dir.clone(), Arc::clone(&self.resolver))
|
||||
.apply(graph)
|
||||
.map_err(ImportPrepareError::Hard)?;
|
||||
let (inlined_graph, file_diagnostics) =
|
||||
FileInliningTransform::new(import_base_dir.clone(), Arc::clone(&self.resolver))
|
||||
.with_template_options(
|
||||
self.inputs.clone(),
|
||||
Some(source_name.clone()),
|
||||
Some(source_text.clone()),
|
||||
self.render_mode,
|
||||
)
|
||||
.with_goal_override(Some(parent_goal.to_string()))
|
||||
.apply_with_diagnostics(graph)
|
||||
.map_err(ImportPrepareError::Hard)?;
|
||||
graph = inlined_graph;
|
||||
diagnostics.extend(file_diagnostics);
|
||||
|
||||
graph.attrs.insert(
|
||||
"goal".to_string(),
|
||||
AttrValue::String(parent_goal.to_string()),
|
||||
);
|
||||
let (templated_graph, template_diagnostics) = TemplateTransform {
|
||||
inputs: self.inputs.clone(),
|
||||
source_name: Some(source_name),
|
||||
source_text: Some(source_text),
|
||||
render_mode: self.render_mode,
|
||||
}
|
||||
.apply_with_diagnostics(graph)
|
||||
.map_err(ImportPrepareError::Hard)?;
|
||||
graph = templated_graph;
|
||||
diagnostics.extend(template_diagnostics);
|
||||
|
||||
if let Some(message) = Self::unresolved_imported_prompt_error(&graph) {
|
||||
return Err(ImportPrepareError::Soft(message));
|
||||
|
|
@ -173,16 +236,21 @@ impl ImportTransform {
|
|||
|
||||
let nested_imports = Self::collect_import_nodes(&graph);
|
||||
for (placeholder_id, import_path) in nested_imports {
|
||||
self.expand_import(
|
||||
let nested_diagnostics = self.expand_import(
|
||||
&mut graph,
|
||||
&placeholder_id,
|
||||
&import_path,
|
||||
parent_goal,
|
||||
&import_base_dir,
|
||||
import_stack,
|
||||
)?;
|
||||
diagnostics.extend(nested_diagnostics);
|
||||
}
|
||||
|
||||
Self::validate_imported_graph(graph).map_err(ImportPrepareError::Soft)
|
||||
let mut prepared =
|
||||
Self::validate_imported_graph(graph).map_err(ImportPrepareError::Soft)?;
|
||||
prepared.diagnostics = diagnostics;
|
||||
Ok(prepared)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -221,6 +289,7 @@ impl ImportTransform {
|
|||
exit_id,
|
||||
entry_id,
|
||||
exit_predecessor_id,
|
||||
diagnostics: _,
|
||||
} = prepared;
|
||||
|
||||
for node_id in imported_graph.nodes.keys() {
|
||||
|
|
@ -469,6 +538,7 @@ impl ImportTransform {
|
|||
exit_id,
|
||||
entry_id,
|
||||
exit_predecessor_id,
|
||||
diagnostics: Vec::new(),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -583,21 +653,65 @@ impl PreparedImport {
|
|||
|
||||
impl Transform for ImportTransform {
|
||||
fn apply(&self, graph: Graph) -> Result<Graph, Error> {
|
||||
let (graph, diagnostics) = self.apply_with_diagnostics(graph)?;
|
||||
if !diagnostics.is_empty() {
|
||||
return Err(Error::ValidationFailed { diagnostics });
|
||||
}
|
||||
Ok(graph)
|
||||
}
|
||||
}
|
||||
|
||||
impl ImportTransform {
|
||||
pub(crate) fn apply_with_diagnostics(
|
||||
&self,
|
||||
graph: Graph,
|
||||
) -> Result<(Graph, Vec<Diagnostic>), Error> {
|
||||
let mut graph = graph;
|
||||
let imports = Self::collect_import_nodes(&graph);
|
||||
let mut import_stack = Vec::new();
|
||||
let mut diagnostics = Vec::new();
|
||||
let path_ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
let mut ignored_goal_diagnostics = Vec::new();
|
||||
let goal_target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), graph.goal());
|
||||
let parent_goal = render_template_for_target(
|
||||
graph.goal(),
|
||||
&path_ctx,
|
||||
self.render_mode,
|
||||
&goal_target,
|
||||
&mut ignored_goal_diagnostics,
|
||||
)?;
|
||||
|
||||
for (placeholder_id, import_path) in imports {
|
||||
self.expand_import(
|
||||
if let Err(error) = validate_static_reference(&import_path, ReferenceKind::Import) {
|
||||
Self::poison_placeholder(&mut graph, &placeholder_id, &error.to_string());
|
||||
continue;
|
||||
}
|
||||
let target = TemplateRenderTarget::node_attr(
|
||||
self.source_name.clone(),
|
||||
placeholder_id.clone(),
|
||||
"import",
|
||||
)
|
||||
.with_source_text(self.source_text.as_deref(), &import_path);
|
||||
let rendered_import_path = render_template_for_target(
|
||||
&import_path,
|
||||
&path_ctx,
|
||||
self.render_mode,
|
||||
&target,
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
let import_diagnostics = self.expand_import(
|
||||
&mut graph,
|
||||
&placeholder_id,
|
||||
&import_path,
|
||||
&rendered_import_path,
|
||||
&parent_goal,
|
||||
&self.current_dir,
|
||||
&mut import_stack,
|
||||
)?;
|
||||
diagnostics.extend(import_diagnostics);
|
||||
}
|
||||
|
||||
Ok(graph)
|
||||
Ok((graph, diagnostics))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -609,6 +723,7 @@ mod tests {
|
|||
|
||||
use fabro_graphviz::graph::AttrValue;
|
||||
use fabro_graphviz::parser;
|
||||
use fabro_util::error::collect_chain;
|
||||
|
||||
use super::*;
|
||||
use crate::file_resolver::FilesystemFileResolver;
|
||||
|
|
@ -631,11 +746,89 @@ mod tests {
|
|||
Arc::new(FilesystemFileResolver::new(
|
||||
fallback_dir.map(Path::to_path_buf),
|
||||
)),
|
||||
HashMap::new(),
|
||||
)
|
||||
.apply(graph)
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn templated_import_path_poison_placeholder_before_rendering() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let graph = parse_graph(
|
||||
r#"digraph Test {
|
||||
start [shape=Mdiamond]
|
||||
validate [import="{{ inputs.path }}"]
|
||||
exit [shape=Msquare]
|
||||
start -> validate -> exit
|
||||
}"#,
|
||||
);
|
||||
|
||||
let graph = ImportTransform::new(
|
||||
dir.path().to_path_buf(),
|
||||
Arc::new(FilesystemFileResolver::new(None)),
|
||||
HashMap::new(),
|
||||
)
|
||||
.with_template_options(
|
||||
Some("workflow.fabro".to_string()),
|
||||
Some("workflow source {{ inputs.path }}".to_string()),
|
||||
RenderMode::Strict,
|
||||
)
|
||||
.apply(graph)
|
||||
.unwrap();
|
||||
|
||||
let error = graph.nodes["validate"]
|
||||
.attrs
|
||||
.get("import_error")
|
||||
.and_then(AttrValue::as_str)
|
||||
.expect("templated import path should poison placeholder");
|
||||
assert!(
|
||||
error.contains("templates are not supported in import references"),
|
||||
"unexpected error: {error}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_workflow_template_error_names_imported_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_file(
|
||||
&dir.path().join("child.fabro"),
|
||||
r#"digraph Child {
|
||||
graph [goal="{{ inputs.foo }}"]
|
||||
start [shape=Mdiamond]
|
||||
work [prompt="Do it"]
|
||||
exit [shape=Msquare]
|
||||
start -> work -> exit
|
||||
}"#,
|
||||
);
|
||||
let graph = parse_graph(
|
||||
r#"digraph Test {
|
||||
start [shape=Mdiamond]
|
||||
validate [import="./child.fabro"]
|
||||
exit [shape=Msquare]
|
||||
start -> validate -> exit
|
||||
}"#,
|
||||
);
|
||||
|
||||
let err = ImportTransform::new(
|
||||
dir.path().to_path_buf(),
|
||||
Arc::new(FilesystemFileResolver::new(None)),
|
||||
HashMap::new(),
|
||||
)
|
||||
.with_template_options(
|
||||
Some("workflow.fabro".to_string()),
|
||||
Some("workflow source".to_string()),
|
||||
RenderMode::Strict,
|
||||
)
|
||||
.apply(graph)
|
||||
.unwrap_err();
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
|
||||
assert!(rendered.contains("child.fabro"), "{rendered}");
|
||||
assert!(rendered.contains("inputs.foo"), "{rendered}");
|
||||
assert!(!rendered.contains("<string>"), "{rendered}");
|
||||
}
|
||||
|
||||
fn basic_import_source() -> &'static str {
|
||||
r#"digraph validate {
|
||||
start [shape=Mdiamond]
|
||||
|
|
@ -709,7 +902,7 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn import_preserves_prompt_templates_for_template_transform() {
|
||||
fn import_reports_structural_diagnostic_for_imported_prompt_templates() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_file(
|
||||
&dir.path().join("validate.fabro"),
|
||||
|
|
@ -721,23 +914,47 @@ mod tests {
|
|||
}"#,
|
||||
);
|
||||
|
||||
let graph = apply_import(
|
||||
let graph = parse_graph(
|
||||
r#"digraph Deploy {
|
||||
start [shape=Mdiamond]
|
||||
validate [import="./validate.fabro"]
|
||||
exit [shape=Msquare]
|
||||
start -> validate -> exit
|
||||
}"#,
|
||||
dir.path(),
|
||||
None,
|
||||
);
|
||||
|
||||
let (graph, diagnostics) = ImportTransform::new(
|
||||
dir.path().to_path_buf(),
|
||||
Arc::new(FilesystemFileResolver::new(None)),
|
||||
HashMap::new(),
|
||||
)
|
||||
.apply_with_diagnostics(graph)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
graph.nodes["validate.lint"]
|
||||
.attrs
|
||||
.get("prompt")
|
||||
.and_then(AttrValue::as_str),
|
||||
Some("Run {{ inputs.task }}")
|
||||
Some("Run ")
|
||||
);
|
||||
let diagnostic = diagnostics
|
||||
.iter()
|
||||
.find(|diagnostic| diagnostic.rule == "template_undefined_variable")
|
||||
.expect("expected imported prompt template diagnostic");
|
||||
assert_eq!(diagnostic.node_id.as_deref(), Some("lint"));
|
||||
assert!(
|
||||
diagnostic
|
||||
.source_path
|
||||
.as_deref()
|
||||
.is_some_and(|path| path.ends_with("validate.fabro")),
|
||||
"{diagnostic:?}"
|
||||
);
|
||||
assert!(
|
||||
diagnostic
|
||||
.message
|
||||
.contains("node `lint` attribute `prompt`"),
|
||||
"{diagnostic:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -1410,6 +1627,7 @@ mod tests {
|
|||
let graph = ImportTransform::new(
|
||||
dir.path().to_path_buf(),
|
||||
Arc::new(FilesystemFileResolver::new(None)),
|
||||
HashMap::new(),
|
||||
)
|
||||
.apply(graph)
|
||||
.unwrap();
|
||||
|
|
|
|||
|
|
@ -21,4 +21,4 @@ pub use import::ImportTransform;
|
|||
pub use model_resolution::ModelResolutionTransform;
|
||||
pub use preamble::PreambleTransform;
|
||||
pub use stylesheet_application::StylesheetApplicationTransform;
|
||||
pub use variable_expansion::TemplateTransform;
|
||||
pub use variable_expansion::{RenderMode, TemplateTransform};
|
||||
|
|
|
|||
|
|
@ -1,73 +1,233 @@
|
|||
use std::collections::HashMap;
|
||||
use std::fmt::Write as _;
|
||||
|
||||
use fabro_graphviz::graph::{AttrValue, Graph};
|
||||
use fabro_template::{TemplateContext, TemplateError, render as render_template, render_lenient};
|
||||
use fabro_validate::Diagnostic;
|
||||
use fabro_template::{TemplateContext, TemplateError, render_lenient_named, render_named};
|
||||
use fabro_util::error::collect_chain;
|
||||
use fabro_validate::{Diagnostic, Severity};
|
||||
|
||||
use super::Transform;
|
||||
use crate::error::Error;
|
||||
use crate::pipeline::types::template_undefined_variable_diagnostic;
|
||||
use crate::pipeline::types::TEMPLATE_UNDEFINED_VARIABLE_RULE;
|
||||
use crate::static_reference::{
|
||||
AttributeScope, ReferenceKind, reference_kind_for_attribute, validate_static_reference,
|
||||
};
|
||||
|
||||
/// How the template-expansion pass should treat undefined input variables.
|
||||
///
|
||||
/// Validate is structural — it should not fail just because the user has not
|
||||
/// bound `{{ inputs.* }}` yet. Run-start is strict — missing inputs are real
|
||||
/// errors. Splitting the two lets validate work on a bare `.fabro` while
|
||||
/// run-start preserves its current hard-fail behavior.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub enum RenderMode {
|
||||
/// Undefined inputs are hard errors. Used by run-create.
|
||||
Strict,
|
||||
/// Undefined inputs render as empty and become warning diagnostics on the
|
||||
/// returned `Validated`, so structural lints still run. Used by
|
||||
/// `fabro validate`.
|
||||
Structural,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct TemplateRenderTarget {
|
||||
pub source_name: Option<String>,
|
||||
pub source_text: Option<String>,
|
||||
pub source_offset: Option<usize>,
|
||||
pub node_id: Option<String>,
|
||||
pub edge: Option<(String, String)>,
|
||||
pub owner: String,
|
||||
}
|
||||
|
||||
impl TemplateRenderTarget {
|
||||
#[must_use]
|
||||
pub(crate) fn graph_attr(source_name: Option<String>, attr_name: impl Into<String>) -> Self {
|
||||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: None,
|
||||
edge: None,
|
||||
owner: format!("graph attribute `{attr_name}`"),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn node_attr(
|
||||
source_name: Option<String>,
|
||||
node_id: impl Into<String>,
|
||||
attr_name: impl Into<String>,
|
||||
) -> Self {
|
||||
let node_id = node_id.into();
|
||||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: Some(node_id.clone()),
|
||||
edge: None,
|
||||
owner: format!("node `{node_id}` attribute `{attr_name}`"),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn edge_attr(
|
||||
source_name: Option<String>,
|
||||
from: impl Into<String>,
|
||||
to: impl Into<String>,
|
||||
attr_name: impl Into<String>,
|
||||
) -> Self {
|
||||
let from = from.into();
|
||||
let to = to.into();
|
||||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: None,
|
||||
edge: Some((from.clone(), to.clone())),
|
||||
owner: format!("edge `{from} -> {to}` attribute `{attr_name}`"),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn with_source_name(mut self, source_name: impl Into<String>) -> Self {
|
||||
self.source_name = Some(source_name.into());
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn with_source_text(mut self, source_text: Option<&str>, value: &str) -> Self {
|
||||
self.source_text = source_text.map(ToOwned::to_owned);
|
||||
self.source_offset = source_text.and_then(|source_text| source_text.find(value));
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
fn template_source_name(&self) -> String {
|
||||
self.source_name
|
||||
.clone()
|
||||
.unwrap_or_else(|| "workflow".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn render_template_for_target(
|
||||
text: &str,
|
||||
ctx: &TemplateContext,
|
||||
render_mode: RenderMode,
|
||||
target: &TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<String, Error> {
|
||||
let source_name = target.template_source_name();
|
||||
match render_mode {
|
||||
RenderMode::Strict => render_named(source_name, text, ctx)
|
||||
.map_err(|err| template_error_for_target(target, err)),
|
||||
RenderMode::Structural => match render_named(source_name.clone(), text, ctx) {
|
||||
Ok(rendered) => Ok(rendered),
|
||||
Err(err @ TemplateError::UndefinedVariable { .. }) => {
|
||||
diagnostics.push(template_diagnostic(&err, target));
|
||||
render_lenient_named(source_name, text, ctx)
|
||||
.map_err(|err| template_error_for_target(target, err))
|
||||
}
|
||||
Err(err) => Err(template_error_for_target(target, err)),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn template_error_for_target(target: &TemplateRenderTarget, err: TemplateError) -> Error {
|
||||
let rendered = collect_chain(&err).join(": ");
|
||||
Error::template(
|
||||
format!("template expansion failed in {}: {rendered}", target.owner),
|
||||
err,
|
||||
)
|
||||
}
|
||||
|
||||
fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) -> Diagnostic {
|
||||
let expression = error.expression();
|
||||
let name = expression.unwrap_or("<unknown>");
|
||||
let mut message = match expression {
|
||||
Some(expr) => format!("undefined template variable `{expr}`"),
|
||||
None => "undefined template variable".to_string(),
|
||||
};
|
||||
let _ = write!(message, " in {}", target.owner);
|
||||
|
||||
let source_location = target
|
||||
.source_text
|
||||
.as_deref()
|
||||
.zip(target.source_offset)
|
||||
.zip(error.span())
|
||||
.and_then(|((source_text, source_offset), span)| {
|
||||
let absolute_offset = source_offset.checked_add(span.offset())?;
|
||||
let (line, column) = source_position(source_text, absolute_offset)?;
|
||||
Some((line, column, absolute_offset, span.len()))
|
||||
});
|
||||
|
||||
Diagnostic {
|
||||
rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(),
|
||||
severity: Severity::Warning,
|
||||
message,
|
||||
node_id: target.node_id.clone(),
|
||||
edge: target.edge.clone(),
|
||||
fix: Some(format!(
|
||||
"bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=<value>`"
|
||||
)),
|
||||
source_path: error
|
||||
.source_name()
|
||||
.map(ToOwned::to_owned)
|
||||
.or_else(|| target.source_name.clone()),
|
||||
line: source_location
|
||||
.map(|(line, _, _, _)| line)
|
||||
.or_else(|| error.line()),
|
||||
column: source_location
|
||||
.map(|(_, column, _, _)| column)
|
||||
.or_else(|| error.column()),
|
||||
span_start: source_location
|
||||
.map(|(_, _, span_start, _)| span_start)
|
||||
.or_else(|| error.span().map(|span| span.offset())),
|
||||
span_len: source_location
|
||||
.map(|(_, _, _, span_len)| span_len)
|
||||
.or_else(|| error.span().map(|span| span.len())),
|
||||
related: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn source_position(source_text: &str, offset: usize) -> Option<(u32, u32)> {
|
||||
if offset > source_text.len() || !source_text.is_char_boundary(offset) {
|
||||
return None;
|
||||
}
|
||||
let line = source_text[..offset]
|
||||
.bytes()
|
||||
.filter(|byte| *byte == b'\n')
|
||||
.count()
|
||||
+ 1;
|
||||
let line_start = source_text[..offset]
|
||||
.rfind('\n')
|
||||
.map_or(0, |index| index + 1);
|
||||
let column = source_text[line_start..offset].chars().count() + 1;
|
||||
Some((u32::try_from(line).ok()?, u32::try_from(column).ok()?))
|
||||
}
|
||||
|
||||
/// Expands `{{ goal }}` / `{{ inputs.* }}` across all string attributes.
|
||||
pub struct TemplateTransform {
|
||||
pub inputs: HashMap<String, toml::Value>,
|
||||
pub inputs: HashMap<String, toml::Value>,
|
||||
pub source_name: Option<String>,
|
||||
pub source_text: Option<String>,
|
||||
pub render_mode: RenderMode,
|
||||
}
|
||||
|
||||
impl TemplateTransform {
|
||||
#[must_use]
|
||||
pub fn new(inputs: HashMap<String, toml::Value>) -> Self {
|
||||
Self { inputs }
|
||||
Self {
|
||||
inputs,
|
||||
source_name: None,
|
||||
source_text: None,
|
||||
render_mode: RenderMode::Structural,
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the transform, returning the rendered graph together with any
|
||||
/// diagnostics collected during lenient undefined-variable rendering.
|
||||
pub fn apply_with_diagnostics(
|
||||
&self,
|
||||
mut graph: Graph,
|
||||
) -> Result<(Graph, Vec<Diagnostic>), Error> {
|
||||
let mut diagnostics = Vec::new();
|
||||
|
||||
let resolved_goal = self.resolve_goal(&graph, &mut diagnostics)?;
|
||||
graph
|
||||
.attrs
|
||||
.insert("goal".to_string(), AttrValue::String(resolved_goal.clone()));
|
||||
let ctx = TemplateContext::new()
|
||||
.with_goal(resolved_goal)
|
||||
.with_inputs(self.inputs.clone());
|
||||
|
||||
Self::render_attrs(
|
||||
&mut graph.attrs,
|
||||
&ctx,
|
||||
AttributeScope::Graph,
|
||||
None,
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
for (node_id, node) in &mut graph.nodes {
|
||||
Self::render_attrs(
|
||||
&mut node.attrs,
|
||||
&ctx,
|
||||
AttributeScope::Node,
|
||||
Some(node_id),
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
}
|
||||
for edge in &mut graph.edges {
|
||||
Self::render_attrs(
|
||||
&mut edge.attrs,
|
||||
&ctx,
|
||||
AttributeScope::Edge,
|
||||
None,
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
}
|
||||
|
||||
Ok((graph, diagnostics))
|
||||
}
|
||||
|
||||
fn resolve_goal(
|
||||
pub(crate) fn resolved_goal(
|
||||
&self,
|
||||
graph: &Graph,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
|
|
@ -79,55 +239,108 @@ impl TemplateTransform {
|
|||
return Ok(goal.to_string());
|
||||
}
|
||||
let ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
Self::render_text(goal, &ctx, None, diagnostics)
|
||||
let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), goal);
|
||||
render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)
|
||||
}
|
||||
|
||||
fn render_attrs(
|
||||
attrs: &mut HashMap<String, AttrValue>,
|
||||
ctx: &TemplateContext,
|
||||
source_name: Option<&String>,
|
||||
source_text: Option<&str>,
|
||||
render_mode: RenderMode,
|
||||
scope: AttributeScope,
|
||||
node_id: Option<&str>,
|
||||
owner_for_attr: impl Fn(&str) -> TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<(), Error> {
|
||||
for (key, value) in attrs {
|
||||
for (attr_name, value) in attrs {
|
||||
if let AttrValue::String(text) = value {
|
||||
if matches!(scope, AttributeScope::Graph) && key == "goal" {
|
||||
if matches!(scope, AttributeScope::Graph) && attr_name == "goal" {
|
||||
continue;
|
||||
}
|
||||
if key == "stack.child_dot_source" {
|
||||
if attr_name == "stack.child_dot_source" {
|
||||
continue;
|
||||
}
|
||||
if let Some(kind) = reference_kind_for_attribute(scope, key, text) {
|
||||
if let Some(kind) = reference_kind_for_attribute(scope, attr_name, text) {
|
||||
validate_static_reference(text, kind)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
continue;
|
||||
}
|
||||
*text = Self::render_text(text, ctx, node_id, diagnostics)?;
|
||||
let target = owner_for_attr(attr_name)
|
||||
.with_source_name(source_name.cloned().unwrap_or_else(|| "workflow".into()))
|
||||
.with_source_text(source_text, text);
|
||||
*text = render_template_for_target(text, ctx, render_mode, &target, diagnostics)?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn render_text(
|
||||
text: &str,
|
||||
ctx: &TemplateContext,
|
||||
node_id: Option<&str>,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<String, Error> {
|
||||
match render_template(text, ctx) {
|
||||
Ok(rendered) => Ok(rendered),
|
||||
Err(TemplateError::UndefinedVariable {
|
||||
expression, line, ..
|
||||
}) => {
|
||||
diagnostics.push(template_undefined_variable_diagnostic(
|
||||
expression.as_deref(),
|
||||
line,
|
||||
node_id,
|
||||
));
|
||||
Ok(render_lenient(text, ctx)?)
|
||||
}
|
||||
Err(error) => Err(error.into()),
|
||||
pub(crate) fn apply_with_diagnostics(
|
||||
&self,
|
||||
graph: Graph,
|
||||
) -> Result<(Graph, Vec<Diagnostic>), Error> {
|
||||
let mut diagnostics = Vec::new();
|
||||
let mut graph = graph;
|
||||
let resolved_goal = self.resolved_goal(&graph, &mut diagnostics)?;
|
||||
graph
|
||||
.attrs
|
||||
.insert("goal".to_string(), AttrValue::String(resolved_goal.clone()));
|
||||
let ctx = TemplateContext::new()
|
||||
.with_goal(resolved_goal)
|
||||
.with_inputs(self.inputs.clone());
|
||||
|
||||
Self::render_attrs(
|
||||
&mut graph.attrs,
|
||||
&ctx,
|
||||
self.source_name.as_ref(),
|
||||
self.source_text.as_deref(),
|
||||
self.render_mode,
|
||||
AttributeScope::Graph,
|
||||
|attr_name| TemplateRenderTarget::graph_attr(self.source_name.clone(), attr_name),
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
for (node_id, node) in &mut graph.nodes {
|
||||
Self::render_attrs(
|
||||
&mut node.attrs,
|
||||
&ctx,
|
||||
self.source_name.as_ref(),
|
||||
self.source_text.as_deref(),
|
||||
self.render_mode,
|
||||
AttributeScope::Node,
|
||||
|attr_name| {
|
||||
TemplateRenderTarget::node_attr(
|
||||
self.source_name.clone(),
|
||||
node_id.clone(),
|
||||
attr_name,
|
||||
)
|
||||
},
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
}
|
||||
for edge in &mut graph.edges {
|
||||
let from = edge.from.clone();
|
||||
let to = edge.to.clone();
|
||||
Self::render_attrs(
|
||||
&mut edge.attrs,
|
||||
&ctx,
|
||||
self.source_name.as_ref(),
|
||||
self.source_text.as_deref(),
|
||||
self.render_mode,
|
||||
AttributeScope::Edge,
|
||||
|attr_name| {
|
||||
TemplateRenderTarget::edge_attr(
|
||||
self.source_name.clone(),
|
||||
from.clone(),
|
||||
to.clone(),
|
||||
attr_name,
|
||||
)
|
||||
},
|
||||
&mut diagnostics,
|
||||
)?;
|
||||
}
|
||||
|
||||
Ok((graph, diagnostics))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -391,4 +604,35 @@ mod tests {
|
|||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn template_transform_reports_structural_diagnostics_with_owner_context() {
|
||||
let mut graph = Graph::new("test");
|
||||
let mut node = Node::new("plan");
|
||||
node.attrs.insert(
|
||||
"prompt".to_string(),
|
||||
AttrValue::String("{{ inputs.missing }}".to_string()),
|
||||
);
|
||||
graph.nodes.insert("plan".to_string(), node);
|
||||
|
||||
let transform = TemplateTransform {
|
||||
inputs: HashMap::new(),
|
||||
source_name: Some("workflow.fabro".to_string()),
|
||||
source_text: None,
|
||||
render_mode: RenderMode::Structural,
|
||||
};
|
||||
let (_, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(diagnostics.len(), 1);
|
||||
assert_eq!(diagnostics[0].node_id.as_deref(), Some("plan"));
|
||||
assert_eq!(
|
||||
diagnostics[0].source_path.as_deref(),
|
||||
Some("workflow.fabro")
|
||||
);
|
||||
assert!(
|
||||
diagnostics[0]
|
||||
.message
|
||||
.contains("node `plan` attribute `prompt`")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4307,6 +4307,8 @@ async fn import_e2e_through_engine() {
|
|||
fabro_workflow::file_resolver::FilesystemFileResolver::new(None),
|
||||
)),
|
||||
inputs: std::collections::HashMap::new(),
|
||||
source_name: None,
|
||||
render_mode: fabro_workflow::operations::RenderMode::Strict,
|
||||
custom_transforms: vec![],
|
||||
catalog: std::sync::Arc::clone(&catalog),
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue