diff --git a/Cargo.lock b/Cargo.lock index a1e0fb336..543c7fa38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1499,6 +1499,7 @@ name = "fabro-graphviz" version = "0.174.0" dependencies = [ "nom", + "regex", "serde", "thiserror 2.0.18", ] @@ -1698,6 +1699,16 @@ dependencies = [ "uuid", ] +[[package]] +name = "fabro-validate" +version = "0.174.0" +dependencies = [ + "fabro-graphviz", + "fabro-llm", + "serde", + "thiserror 2.0.18", +] + [[package]] name = "fabro-workflows" version = "0.174.0" @@ -1725,6 +1736,7 @@ dependencies = [ "fabro-retro", "fabro-ssh", "fabro-util", + "fabro-validate", "futures", "git2", "hex", diff --git a/lib/crates/fabro-graphviz/Cargo.toml b/lib/crates/fabro-graphviz/Cargo.toml index 0c069fce2..1f8351f6e 100644 --- a/lib/crates/fabro-graphviz/Cargo.toml +++ b/lib/crates/fabro-graphviz/Cargo.toml @@ -10,5 +10,6 @@ doctest = false [dependencies] nom = "7" +regex = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } diff --git a/lib/crates/fabro-graphviz/src/condition.rs b/lib/crates/fabro-graphviz/src/condition.rs new file mode 100644 index 000000000..f9265f7ce --- /dev/null +++ b/lib/crates/fabro-graphviz/src/condition.rs @@ -0,0 +1,460 @@ +/// Condition expression parser for edge guards (spec Section 10). +/// +/// Grammar: +/// ```text +/// Expr ::= OrExpr +/// OrExpr ::= AndExpr ('||' AndExpr)* +/// AndExpr ::= UnaryExpr ('&&' UnaryExpr)* +/// UnaryExpr ::= '!' UnaryExpr | Clause +/// Clause ::= Key Op Literal | Key (bare key = truthy) +/// Op ::= '=' | '!=' | '>' | '<' | '>=' | '<=' +/// | 'contains' | 'matches' +/// ``` +use crate::error::GraphvizError; + +// --------------------------------------------------------------------------- +// AST +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq)] +pub enum ConditionExpr { + Clause(Clause), + Not(Box), + And(Vec), + Or(Vec), +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Clause { + pub key: String, + pub op: Op, + pub value: String, +} + +#[derive(Debug, Clone, PartialEq)] +pub enum Op { + Eq, + NotEq, + Gt, + Lt, + Gte, + Lte, + Contains, + Matches, + Truthy, +} + +// --------------------------------------------------------------------------- +// Tokenizer +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq)] +enum Token { + Word(String), + OpEq, // = + OpNotEq, // != + OpGt, // > + OpLt, // < + OpGte, // >= + OpLte, // <= + And, // && + Or, // || + Not, // ! + Contains, // contains + Matches, // matches +} + +fn tokenize(input: &str) -> Result, GraphvizError> { + let input = input.trim(); + if input.is_empty() { + return Ok(Vec::new()); + } + + let chars: Vec = input.chars().collect(); + let len = chars.len(); + let mut i = 0; + let mut tokens = Vec::new(); + + while i < len { + // Skip whitespace + if chars[i].is_whitespace() { + i += 1; + continue; + } + + // Two-char operators (longest match first) + if i + 1 < len { + let two = format!("{}{}", chars[i], chars[i + 1]); + match two.as_str() { + "&&" => { + tokens.push(Token::And); + i += 2; + continue; + } + "||" => { + tokens.push(Token::Or); + i += 2; + continue; + } + "!=" => { + tokens.push(Token::OpNotEq); + i += 2; + continue; + } + ">=" => { + tokens.push(Token::OpGte); + i += 2; + continue; + } + "<=" => { + tokens.push(Token::OpLte); + i += 2; + continue; + } + _ => {} + } + } + + // Single-char operators + match chars[i] { + '=' => { + tokens.push(Token::OpEq); + i += 1; + continue; + } + '>' => { + tokens.push(Token::OpGt); + i += 1; + continue; + } + '<' => { + tokens.push(Token::OpLt); + i += 1; + continue; + } + '!' => { + tokens.push(Token::Not); + i += 1; + continue; + } + _ => {} + } + + // Word: everything up to whitespace or operator char + let start = i; + while i < len && !chars[i].is_whitespace() && !is_op_char(chars[i]) { + i += 1; + } + if i == start { + return Err(GraphvizError::Parse(format!( + "unexpected character '{}' in condition expression", + chars[i] + ))); + } + let word: String = chars[start..i].iter().collect(); + + // Recognize keyword operators only when they appear between words + // (not as the first or last token, and not adjacent to another operator) + match word.as_str() { + "contains" if is_word_operator_context(&tokens) => { + tokens.push(Token::Contains); + } + "matches" if is_word_operator_context(&tokens) => { + tokens.push(Token::Matches); + } + _ => { + tokens.push(Token::Word(word)); + } + } + } + + Ok(tokens) +} + +fn is_op_char(c: char) -> bool { + matches!(c, '=' | '!' | '>' | '<' | '&' | '|') +} + +/// Word operators (`contains`, `matches`) are recognized when preceded by a Word token. +fn is_word_operator_context(tokens: &[Token]) -> bool { + matches!(tokens.last(), Some(Token::Word(_))) +} + +// --------------------------------------------------------------------------- +// Parser (recursive descent) +// --------------------------------------------------------------------------- + +struct Parser { + tokens: Vec, + pos: usize, +} + +impl Parser { + fn new(tokens: Vec) -> Self { + Self { tokens, pos: 0 } + } + + fn peek(&self) -> Option<&Token> { + self.tokens.get(self.pos) + } + + fn advance(&mut self) -> Option { + let tok = self.tokens.get(self.pos).cloned(); + if tok.is_some() { + self.pos += 1; + } + tok + } + + fn parse_expr(&mut self) -> Result { + self.parse_or() + } + + fn parse_or(&mut self) -> Result { + let mut children = vec![self.parse_and()?]; + while self.peek() == Some(&Token::Or) { + self.advance(); + children.push(self.parse_and()?); + } + if children.len() == 1 { + Ok(children.pop().expect("just checked length")) + } else { + Ok(ConditionExpr::Or(children)) + } + } + + fn parse_and(&mut self) -> Result { + let mut children = vec![self.parse_unary()?]; + while self.peek() == Some(&Token::And) { + self.advance(); + children.push(self.parse_unary()?); + } + if children.len() == 1 { + Ok(children.pop().expect("just checked length")) + } else { + Ok(ConditionExpr::And(children)) + } + } + + fn parse_unary(&mut self) -> Result { + if self.peek() == Some(&Token::Not) { + self.advance(); + let inner = self.parse_unary()?; + return Ok(ConditionExpr::Not(Box::new(inner))); + } + self.parse_clause() + } + + fn parse_clause(&mut self) -> Result { + let key = match self.advance() { + Some(Token::Word(w)) => w, + Some(other) => { + return Err(GraphvizError::Parse(format!( + "expected key, got {other:?} in condition expression" + ))); + } + None => { + return Err(GraphvizError::Parse( + "unexpected end of condition expression".to_string(), + )); + } + }; + + // Check for operator + let op = match self.peek() { + Some(Token::OpEq) => Some(Op::Eq), + Some(Token::OpNotEq) => Some(Op::NotEq), + Some(Token::OpGt) => Some(Op::Gt), + Some(Token::OpLt) => Some(Op::Lt), + Some(Token::OpGte) => Some(Op::Gte), + Some(Token::OpLte) => Some(Op::Lte), + Some(Token::Contains) => Some(Op::Contains), + Some(Token::Matches) => Some(Op::Matches), + _ => None, + }; + + let Some(op) = op else { + // Bare key -> truthy + return Ok(ConditionExpr::Clause(Clause { + key, + op: Op::Truthy, + value: String::new(), + })); + }; + + self.advance(); // consume the operator + + // Value: must be a Word + let value = match self.advance() { + Some(Token::Word(w)) => w, + Some(other) => { + return Err(GraphvizError::Parse(format!( + "expected value after operator, got {other:?}" + ))); + } + None => { + // Allow empty value for `=` and `!=` (backward compat: `missing_key=`) + if op == Op::Eq || op == Op::NotEq { + String::new() + } else { + return Err(GraphvizError::Parse( + "expected value after operator".to_string(), + )); + } + } + }; + + // Validate regex at parse time + if op == Op::Matches { + regex::Regex::new(&value).map_err(|e| { + GraphvizError::Parse(format!("invalid regex pattern '{value}': {e}")) + })?; + } + + Ok(ConditionExpr::Clause(Clause { key, op, value })) + } +} + +fn parse_expression(expr: &str) -> Result { + let tokens = tokenize(expr)?; + if tokens.is_empty() { + return Ok(ConditionExpr::And(Vec::new())); + } + let mut parser = Parser::new(tokens); + let result = parser.parse_expr()?; + if parser.pos < parser.tokens.len() { + return Err(GraphvizError::Parse(format!( + "unexpected token {:?} in condition expression", + parser.tokens[parser.pos] + ))); + } + Ok(result) +} + +/// Parse and validate a condition expression. +/// +/// # Errors +/// +/// Returns an error if the expression contains invalid syntax. +pub fn parse_condition(expr: &str) -> Result<(), GraphvizError> { + parse_expression(expr)?; + Ok(()) +} + +/// Parse a condition expression and return the AST. +/// +/// # Errors +/// +/// Returns an error if the expression contains invalid syntax. +pub fn parse_condition_expr(expr: &str) -> Result { + parse_expression(expr) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_condition_validates() { + assert!(parse_condition("outcome=success").is_ok()); + assert!(parse_condition("outcome=success && context.x=y").is_ok()); + assert!(parse_condition("").is_ok()); + } + + #[test] + fn parse_condition_accepts_bare_key() { + assert!(parse_condition("some_flag").is_ok()); + } + + #[test] + fn parse_eq_into_clause() { + let expr = parse_expression("outcome=success").unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "outcome".to_string(), + op: Op::Eq, + value: "success".to_string(), + }) + ); + } + + #[test] + fn parse_and_into_and_node() { + let expr = parse_expression("a=1 && b=2").unwrap(); + assert_eq!( + expr, + ConditionExpr::And(vec![ + ConditionExpr::Clause(Clause { + key: "a".to_string(), + op: Op::Eq, + value: "1".to_string(), + }), + ConditionExpr::Clause(Clause { + key: "b".to_string(), + op: Op::Eq, + value: "2".to_string(), + }), + ]) + ); + } + + #[test] + fn parse_bare_key_into_truthy() { + let expr = parse_expression("some_flag").unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "some_flag".to_string(), + op: Op::Truthy, + value: String::new(), + }) + ); + } + + #[test] + fn parse_not_eq_into_clause() { + let expr = parse_expression("outcome!=fail").unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "outcome".to_string(), + op: Op::NotEq, + value: "fail".to_string(), + }) + ); + } + + #[test] + fn parse_numeric_comparisons() { + assert!(parse_condition("x > 5").is_ok()); + assert!(parse_condition("x >= 5").is_ok()); + assert!(parse_condition("x < 5").is_ok()); + assert!(parse_condition("x <= 5").is_ok()); + } + + #[test] + fn parse_contains() { + assert!(parse_condition("x contains y").is_ok()); + } + + #[test] + fn parse_matches() { + assert!(parse_condition("x matches ^ok$").is_ok()); + } + + #[test] + fn matches_invalid_regex_fails_parse() { + assert!(parse_condition("x matches [bad").is_err()); + } + + #[test] + fn parse_or() { + assert!(parse_condition("a=1 || b=2").is_ok()); + } + + #[test] + fn parse_not() { + assert!(parse_condition("!x=y").is_ok()); + } +} diff --git a/lib/crates/fabro-graphviz/src/error.rs b/lib/crates/fabro-graphviz/src/error.rs index fb9ac8b3c..8ea9ea90d 100644 --- a/lib/crates/fabro-graphviz/src/error.rs +++ b/lib/crates/fabro-graphviz/src/error.rs @@ -4,4 +4,7 @@ use thiserror::Error; pub enum GraphvizError { #[error("Parse error: {0}")] Parse(String), + + #[error("Stylesheet error: {0}")] + Stylesheet(String), } diff --git a/lib/crates/fabro-graphviz/src/fidelity.rs b/lib/crates/fabro-graphviz/src/fidelity.rs new file mode 100644 index 000000000..c658495c8 --- /dev/null +++ b/lib/crates/fabro-graphviz/src/fidelity.rs @@ -0,0 +1,104 @@ +use std::fmt; +use std::str::FromStr; + +/// Fidelity mode controlling how much prior context is provided to LLM sessions. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum Fidelity { + /// Complete context, no summarization — sessions share a thread. + Full, + /// Minimal: only graph goal and run ID. + Truncate, + /// Structured nested-bullet summary (default). + #[default] + Compact, + /// Brief textual summary (~600 token target). + SummaryLow, + /// Moderate textual summary (~1500 token target). + SummaryMedium, + /// Detailed per-stage Markdown report. + SummaryHigh, +} + +impl Fidelity { + /// Degrade full fidelity to summary:high (used on checkpoint resume). + #[must_use] + pub fn degraded(self) -> Self { + match self { + Self::Full => Self::SummaryHigh, + other => other, + } + } +} + +impl fmt::Display for Fidelity { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + Self::Full => "full", + Self::Truncate => "truncate", + Self::Compact => "compact", + Self::SummaryLow => "summary:low", + Self::SummaryMedium => "summary:medium", + Self::SummaryHigh => "summary:high", + }; + write!(f, "{s}") + } +} + +impl FromStr for Fidelity { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "full" => Ok(Self::Full), + "truncate" => Ok(Self::Truncate), + "compact" => Ok(Self::Compact), + "summary:low" => Ok(Self::SummaryLow), + "summary:medium" => Ok(Self::SummaryMedium), + "summary:high" => Ok(Self::SummaryHigh), + other => Err(format!("unknown fidelity mode: {other}")), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fidelity_display_roundtrips() { + let modes = [ + Fidelity::Full, + Fidelity::Truncate, + Fidelity::Compact, + Fidelity::SummaryLow, + Fidelity::SummaryMedium, + Fidelity::SummaryHigh, + ]; + for mode in modes { + let s = mode.to_string(); + let parsed: Fidelity = s.parse().unwrap(); + assert_eq!(parsed, mode); + } + } + + #[test] + fn fidelity_default_is_compact() { + assert_eq!(Fidelity::default(), Fidelity::Compact); + } + + #[test] + fn fidelity_degraded_full_becomes_summary_high() { + assert_eq!(Fidelity::Full.degraded(), Fidelity::SummaryHigh); + } + + #[test] + fn fidelity_degraded_non_full_unchanged() { + assert_eq!(Fidelity::Compact.degraded(), Fidelity::Compact); + assert_eq!(Fidelity::SummaryHigh.degraded(), Fidelity::SummaryHigh); + } + + #[test] + fn fidelity_unknown_mode_errors() { + assert!("bogus".parse::().is_err()); + } +} diff --git a/lib/crates/fabro-graphviz/src/lib.rs b/lib/crates/fabro-graphviz/src/lib.rs index 96b17c8c8..bb646a9ae 100644 --- a/lib/crates/fabro-graphviz/src/lib.rs +++ b/lib/crates/fabro-graphviz/src/lib.rs @@ -1,3 +1,8 @@ +pub mod condition; pub mod error; +pub mod fidelity; pub mod graph; pub mod parser; +pub mod stylesheet; + +pub use fidelity::Fidelity; diff --git a/lib/crates/fabro-graphviz/src/stylesheet.rs b/lib/crates/fabro-graphviz/src/stylesheet.rs new file mode 100644 index 000000000..421c84959 --- /dev/null +++ b/lib/crates/fabro-graphviz/src/stylesheet.rs @@ -0,0 +1,250 @@ +use crate::error::GraphvizError; + +/// A parsed stylesheet selector. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Selector { + /// `*` -- matches all nodes, specificity 0. + Universal, + /// Bare word -- matches nodes by shape name, specificity 1. + Shape(String), + /// `.classname` -- matches nodes with that class, specificity 2. + Class(String), + /// `#nodeid` -- matches a specific node, specificity 3. + Id(String), +} + +impl Selector { + #[must_use] + pub const fn specificity(&self) -> u8 { + match self { + Self::Universal => 0, + Self::Shape(_) => 1, + Self::Class(_) => 2, + Self::Id(_) => 3, + } + } +} + +/// A single CSS-like declaration: `property: value`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Declaration { + pub property: String, + pub value: String, +} + +/// A stylesheet rule: selector + declarations. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Rule { + pub selector: Selector, + pub declarations: Vec, +} + +/// A parsed stylesheet containing multiple rules. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Stylesheet { + pub rules: Vec, +} + +/// Parse a stylesheet string into a `Stylesheet`. +/// +/// # Errors +/// +/// Returns an error if the input contains invalid stylesheet syntax. +pub fn parse_stylesheet(input: &str) -> Result { + let input = input.trim(); + if input.is_empty() { + return Ok(Stylesheet { rules: Vec::new() }); + } + + let mut rules = Vec::new(); + let mut remaining = input; + + while !remaining.trim().is_empty() { + remaining = remaining.trim(); + + let selector = parse_selector(&mut remaining)?; + if !remaining.starts_with('{') { + return Err(GraphvizError::Stylesheet(format!( + "expected '{{' after selector, got: {:?}", + &remaining[..remaining.len().min(20)] + ))); + } + remaining = remaining[1..].trim(); + + let declarations = parse_declarations(&mut remaining)?; + remaining = remaining[1..].trim(); // skip '}' + + rules.push(Rule { + selector, + declarations, + }); + } + + Ok(Stylesheet { rules }) +} + +fn parse_selector(remaining: &mut &str) -> Result { + if remaining.starts_with('*') { + *remaining = remaining[1..].trim(); + Ok(Selector::Universal) + } else if remaining.starts_with('#') { + *remaining = remaining[1..].trim(); + let end = remaining + .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') + .unwrap_or(remaining.len()); + if end == 0 { + return Err(GraphvizError::Stylesheet( + "expected identifier after '#'".into(), + )); + } + let id = remaining[..end].to_string(); + *remaining = remaining[end..].trim(); + Ok(Selector::Id(id)) + } else if remaining.starts_with('.') { + *remaining = remaining[1..].trim(); + let end = remaining + .find(|c: char| !c.is_ascii_lowercase() && !c.is_ascii_digit() && c != '-') + .unwrap_or(remaining.len()); + if end == 0 { + return Err(GraphvizError::Stylesheet( + "expected class name after '.'".into(), + )); + } + let class = remaining[..end].to_string(); + *remaining = remaining[end..].trim(); + Ok(Selector::Class(class)) + } else { + // Bare word: shape selector + let end = remaining + .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') + .unwrap_or(remaining.len()); + if end == 0 { + return Err(GraphvizError::Stylesheet(format!( + "expected selector ('*', '#id', '.class', or shape name), got: {:?}", + &remaining[..remaining.len().min(20)] + ))); + } + let shape = remaining[..end].to_string(); + *remaining = remaining[end..].trim(); + Ok(Selector::Shape(shape)) + } +} + +fn parse_declarations(remaining: &mut &str) -> Result, GraphvizError> { + let mut declarations = Vec::new(); + while !remaining.starts_with('}') { + if remaining.is_empty() { + return Err(GraphvizError::Stylesheet( + "unexpected end of stylesheet, expected '}'".into(), + )); + } + if remaining.starts_with(';') { + *remaining = remaining[1..].trim(); + continue; + } + + let prop_end = remaining + .find(|c: char| c == ':' || c.is_whitespace()) + .unwrap_or(remaining.len()); + let property = remaining[..prop_end].to_string(); + *remaining = remaining[prop_end..].trim(); + + if !remaining.starts_with(':') { + return Err(GraphvizError::Stylesheet(format!( + "expected ':' after property name '{property}'" + ))); + } + *remaining = remaining[1..].trim(); + + let val_end = remaining.find([';', '}']).unwrap_or(remaining.len()); + let value = remaining[..val_end].trim().to_string(); + *remaining = remaining[val_end..].trim(); + + if value.is_empty() { + return Err(GraphvizError::Stylesheet(format!( + "empty value for property '{property}'" + ))); + } + + declarations.push(Declaration { property, value }); + + if remaining.starts_with(';') { + *remaining = remaining[1..].trim(); + } + } + Ok(declarations) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_empty_stylesheet() { + let ss = parse_stylesheet("").unwrap(); + assert!(ss.rules.is_empty()); + } + + #[test] + fn parse_universal_rule() { + let ss = parse_stylesheet("* { model: claude-sonnet-4-5; provider: anthropic; }").unwrap(); + assert_eq!(ss.rules.len(), 1); + assert_eq!(ss.rules[0].selector, Selector::Universal); + assert_eq!(ss.rules[0].declarations.len(), 2); + assert_eq!(ss.rules[0].declarations[0].property, "model"); + assert_eq!(ss.rules[0].declarations[0].value, "claude-sonnet-4-5"); + } + + #[test] + fn parse_class_rule() { + let ss = parse_stylesheet(".code { model: claude-opus-4-6; }").unwrap(); + assert_eq!(ss.rules[0].selector, Selector::Class("code".into())); + } + + #[test] + fn parse_id_rule() { + let ss = parse_stylesheet("#critical_review { model: gpt-5.2; reasoning_effort: high; }") + .unwrap(); + assert_eq!(ss.rules[0].selector, Selector::Id("critical_review".into())); + assert_eq!(ss.rules[0].declarations.len(), 2); + } + + #[test] + fn parse_multiple_rules() { + let input = r" + * { model: claude-sonnet-4-5; provider: anthropic; } + .code { model: claude-opus-4-6; provider: anthropic; } + #critical_review { model: gpt-5.2; provider: openai; reasoning_effort: high; } + "; + let ss = parse_stylesheet(input).unwrap(); + assert_eq!(ss.rules.len(), 3); + } + + #[test] + fn parse_error_missing_brace() { + let result = parse_stylesheet("* model: test; }"); + assert!(result.is_err()); + } + + #[test] + fn parse_error_missing_selector() { + let result = parse_stylesheet("{ model: test; }"); + assert!(result.is_err()); + } + + #[test] + fn parse_shape_selector() { + let ss = parse_stylesheet("box { model: opus; }").unwrap(); + assert_eq!(ss.rules.len(), 1); + assert_eq!(ss.rules[0].selector, Selector::Shape("box".into())); + assert_eq!(ss.rules[0].declarations[0].value, "opus"); + } + + #[test] + fn selector_specificity_values() { + assert_eq!(Selector::Universal.specificity(), 0); + assert_eq!(Selector::Shape("box".into()).specificity(), 1); + assert_eq!(Selector::Class("x".into()).specificity(), 2); + assert_eq!(Selector::Id("x".into()).specificity(), 3); + } +} diff --git a/lib/crates/fabro-validate/Cargo.toml b/lib/crates/fabro-validate/Cargo.toml new file mode 100644 index 000000000..e9841c3a1 --- /dev/null +++ b/lib/crates/fabro-validate/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "fabro-validate" +edition.workspace = true +version.workspace = true +license.workspace = true +description = "Validation and lint rules for Fabro workflow graphs" + +[lib] +doctest = false + +[dependencies] +fabro-graphviz = { path = "../fabro-graphviz" } +fabro-llm = { path = "../fabro-llm" } +serde = { workspace = true } +thiserror = { workspace = true } diff --git a/lib/crates/fabro-workflows/src/validation/mod.rs b/lib/crates/fabro-validate/src/lib.rs similarity index 90% rename from lib/crates/fabro-workflows/src/validation/mod.rs rename to lib/crates/fabro-validate/src/lib.rs index b3766cab8..202c244f4 100644 --- a/lib/crates/fabro-workflows/src/validation/mod.rs +++ b/lib/crates/fabro-validate/src/lib.rs @@ -2,7 +2,6 @@ pub mod rules; use serde::{Deserialize, Serialize}; -use crate::error::FabroError; use fabro_graphviz::graph::Graph; /// Severity level for validation diagnostics. @@ -30,6 +29,11 @@ pub trait LintRule { fn apply(&self, graph: &Graph) -> Vec; } +/// Validation error returned when error-severity diagnostics are present. +#[derive(Debug, thiserror::Error)] +#[error("Validation error: {0}")] +pub struct ValidationError(pub String); + /// Run all built-in lint rules (and any extra rules) against the graph. #[must_use] pub fn validate(graph: &Graph, extra_rules: &[&dyn LintRule]) -> Vec { @@ -44,11 +48,11 @@ pub fn validate(graph: &Graph, extra_rules: &[&dyn LintRule]) -> Vec diagnostics } -/// If any Error-severity diagnostics are present, return `FabroError::Validation`. +/// If any Error-severity diagnostics are present, return `ValidationError`. /// /// # Errors -/// Returns `FabroError::Validation` with joined error messages. -pub fn raise_on_errors(diagnostics: &[Diagnostic]) -> Result<(), FabroError> { +/// Returns `ValidationError` with joined error messages. +pub fn raise_on_errors(diagnostics: &[Diagnostic]) -> Result<(), ValidationError> { let mut errors = diagnostics .iter() .filter(|d| d.severity == Severity::Error) @@ -58,7 +62,7 @@ pub fn raise_on_errors(diagnostics: &[Diagnostic]) -> Result<(), FabroError> { .map(|d| d.message.as_str()) .collect::>() .join("; "); - return Err(FabroError::Validation(message)); + return Err(ValidationError(message)); } Ok(()) } @@ -67,11 +71,11 @@ pub fn raise_on_errors(diagnostics: &[Diagnostic]) -> Result<(), FabroError> { /// diagnostics are found. /// /// # Errors -/// Returns `FabroError::Validation` if any Error-severity diagnostics are found. +/// Returns `ValidationError` if any Error-severity diagnostics are found. pub fn validate_or_raise( graph: &Graph, extra_rules: &[&dyn LintRule], -) -> Result, FabroError> { +) -> Result, ValidationError> { let diagnostics = validate(graph, extra_rules); raise_on_errors(&diagnostics)?; Ok(diagnostics) diff --git a/lib/crates/fabro-workflows/src/validation/rules.rs b/lib/crates/fabro-validate/src/rules.rs similarity index 99% rename from lib/crates/fabro-workflows/src/validation/rules.rs rename to lib/crates/fabro-validate/src/rules.rs index 92737d950..1bae5f571 100644 --- a/lib/crates/fabro-workflows/src/validation/rules.rs +++ b/lib/crates/fabro-validate/src/rules.rs @@ -1,10 +1,10 @@ use std::collections::{HashSet, VecDeque}; use std::str::FromStr; -use crate::condition::parse_condition; +use fabro_graphviz::condition::parse_condition; use fabro_graphviz::graph::{is_llm_handler_type, AttrValue, Graph}; -use super::{Diagnostic, LintRule, Severity}; +use crate::{Diagnostic, LintRule, Severity}; /// Returns all built-in lint rules. #[must_use] @@ -348,7 +348,7 @@ impl LintRule for StylesheetSyntaxRule { if stylesheet.is_empty() { return Vec::new(); } - match crate::stylesheet::parse_stylesheet(stylesheet) { + match fabro_graphviz::stylesheet::parse_stylesheet(stylesheet) { Ok(_) => Vec::new(), Err(e) => vec![Diagnostic { rule: self.name().to_string(), @@ -414,7 +414,7 @@ struct FidelityValidRule; impl FidelityValidRule { fn fix_message() -> String { - use crate::context::keys::Fidelity; + use fabro_graphviz::Fidelity; let modes: Vec<_> = [ Fidelity::Full, Fidelity::Truncate, @@ -436,7 +436,7 @@ impl LintRule for FidelityValidRule { } fn apply(&self, graph: &Graph) -> Vec { - use crate::context::keys::Fidelity; + use fabro_graphviz::Fidelity; let mut diagnostics = Vec::new(); for node in graph.nodes.values() { @@ -902,12 +902,12 @@ impl LintRule for ScriptAbsoluteCdRule { struct StylesheetModelKnownRule; impl StylesheetModelKnownRule { - fn selector_label(selector: &crate::stylesheet::Selector) -> String { + fn selector_label(selector: &fabro_graphviz::stylesheet::Selector) -> String { match selector { - crate::stylesheet::Selector::Universal => "*".to_string(), - crate::stylesheet::Selector::Shape(s) => s.clone(), - crate::stylesheet::Selector::Class(c) => format!(".{c}"), - crate::stylesheet::Selector::Id(id) => format!("#{id}"), + fabro_graphviz::stylesheet::Selector::Universal => "*".to_string(), + fabro_graphviz::stylesheet::Selector::Shape(s) => s.clone(), + fabro_graphviz::stylesheet::Selector::Class(c) => format!(".{c}"), + fabro_graphviz::stylesheet::Selector::Id(id) => format!("#{id}"), } } } @@ -922,7 +922,7 @@ impl LintRule for StylesheetModelKnownRule { if stylesheet_str.is_empty() { return Vec::new(); } - let stylesheet = match crate::stylesheet::parse_stylesheet(stylesheet_str) { + let stylesheet = match fabro_graphviz::stylesheet::parse_stylesheet(stylesheet_str) { Ok(ss) => ss, Err(_) => return Vec::new(), // syntax errors caught by stylesheet_syntax rule }; diff --git a/lib/crates/fabro-workflows/Cargo.toml b/lib/crates/fabro-workflows/Cargo.toml index daa671395..7aefac2f1 100644 --- a/lib/crates/fabro-workflows/Cargo.toml +++ b/lib/crates/fabro-workflows/Cargo.toml @@ -22,6 +22,7 @@ anyhow.workspace = true dotenvy.workspace = true fabro-agent = { path = "../fabro-agent" } fabro-graphviz = { path = "../fabro-graphviz" } +fabro-validate = { path = "../fabro-validate" } fabro-devcontainer = { path = "../fabro-devcontainer" } fabro-exe = { path = "../fabro-exe", optional = true } fabro-ssh = { path = "../fabro-ssh" } diff --git a/lib/crates/fabro-workflows/src/cli/graph.rs b/lib/crates/fabro-workflows/src/cli/graph.rs index c6e43153f..ff003dfe8 100644 --- a/lib/crates/fabro-workflows/src/cli/graph.rs +++ b/lib/crates/fabro-workflows/src/cli/graph.rs @@ -10,8 +10,8 @@ use clap::{Args, ValueEnum}; use fabro_util::terminal::Styles; use tracing::debug; -use crate::validation::Severity; use crate::workflow::prepare_from_file; +use fabro_validate::Severity; use super::{print_diagnostics, read_workflow_file, relative_path}; diff --git a/lib/crates/fabro-workflows/src/cli/mod.rs b/lib/crates/fabro-workflows/src/cli/mod.rs index 3f3a3027f..1fc23f311 100644 --- a/lib/crates/fabro-workflows/src/cli/mod.rs +++ b/lib/crates/fabro-workflows/src/cli/mod.rs @@ -29,7 +29,7 @@ use std::path::PathBuf; use std::str::FromStr; use crate::outcome::StageUsage; -use crate::validation::{Diagnostic, Severity}; +use fabro_validate::{Diagnostic, Severity}; /// Sandbox provider for agent tool operations. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, ValueEnum)] diff --git a/lib/crates/fabro-workflows/src/cli/run.rs b/lib/crates/fabro-workflows/src/cli/run.rs index 4cdbedf9b..f9035774e 100644 --- a/lib/crates/fabro-workflows/src/cli/run.rs +++ b/lib/crates/fabro-workflows/src/cli/run.rs @@ -19,8 +19,8 @@ use crate::interviewer::auto_approve::AutoApproveInterviewer; use crate::interviewer::console::ConsoleInterviewer; use crate::interviewer::Interviewer; use crate::outcome::StageStatus; -use crate::validation::Severity; use crate::workflow::WorkflowBuilder; +use fabro_validate::Severity; use fabro_llm::provider::Provider; @@ -1599,10 +1599,7 @@ async fn run_from_branch( ); super::print_diagnostics(&diagnostics, styles); - if diagnostics - .iter() - .any(|d| d.severity == crate::validation::Severity::Error) - { + if diagnostics.iter().any(|d| d.severity == Severity::Error) { anyhow::bail!("Validation failed"); } diff --git a/lib/crates/fabro-workflows/src/cli/validate.rs b/lib/crates/fabro-workflows/src/cli/validate.rs index 7490ae344..c502d76a5 100644 --- a/lib/crates/fabro-workflows/src/cli/validate.rs +++ b/lib/crates/fabro-workflows/src/cli/validate.rs @@ -1,8 +1,8 @@ use anyhow::bail; use fabro_util::terminal::Styles; -use crate::validation::Severity; use crate::workflow::prepare_from_file; +use fabro_validate::Severity; use super::{print_diagnostics, relative_path, ValidateArgs}; diff --git a/lib/crates/fabro-workflows/src/cli/workflow.rs b/lib/crates/fabro-workflows/src/cli/workflow.rs index 12fe7ba3d..e4cbf34ee 100644 --- a/lib/crates/fabro-workflows/src/cli/workflow.rs +++ b/lib/crates/fabro-workflows/src/cli/workflow.rs @@ -367,10 +367,10 @@ mod tests { fs::read_to_string(tmp.path().join("fabro/workflows/test-wf/workflow.fabro")).unwrap(); let graph = fabro_graphviz::parser::parse(&content).expect("generated .fabro should parse"); - let diagnostics = crate::validation::validate(&graph, &[]); + let diagnostics = fabro_validate::validate(&graph, &[]); let errors: Vec<_> = diagnostics .iter() - .filter(|d| d.severity == crate::validation::Severity::Error) + .filter(|d| d.severity == fabro_validate::Severity::Error) .collect(); assert!(errors.is_empty(), "validation errors: {errors:?}"); } diff --git a/lib/crates/fabro-workflows/src/condition.rs b/lib/crates/fabro-workflows/src/condition.rs index 1f709a207..9df445867 100644 --- a/lib/crates/fabro-workflows/src/condition.rs +++ b/lib/crates/fabro-workflows/src/condition.rs @@ -1,348 +1,14 @@ /// Condition expression evaluator for edge guards (spec Section 10). /// -/// Grammar: -/// ```text -/// Expr ::= OrExpr -/// OrExpr ::= AndExpr ('||' AndExpr)* -/// AndExpr ::= UnaryExpr ('&&' UnaryExpr)* -/// UnaryExpr ::= '!' UnaryExpr | Clause -/// Clause ::= Key Op Literal | Key (bare key = truthy) -/// Op ::= '=' | '!=' | '>' | '<' | '>=' | '<=' -/// | 'contains' | 'matches' -/// ``` +/// The parser lives in `fabro_graphviz::condition`; this module re-exports +/// `parse_condition` and provides runtime evaluation against `Outcome`/`Context`. +pub use fabro_graphviz::condition::parse_condition; +use fabro_graphviz::condition::{Clause, ConditionExpr, Op}; + use crate::context::keys; use crate::context::Context; -use crate::error::FabroError; use crate::outcome::Outcome; -// --------------------------------------------------------------------------- -// AST -// --------------------------------------------------------------------------- - -#[derive(Debug, Clone, PartialEq)] -enum ConditionExpr { - Clause(Clause), - Not(Box), - And(Vec), - Or(Vec), -} - -#[derive(Debug, Clone, PartialEq)] -struct Clause { - key: String, - op: Op, - value: String, -} - -#[derive(Debug, Clone, PartialEq)] -enum Op { - Eq, - NotEq, - Gt, - Lt, - Gte, - Lte, - Contains, - Matches, - Truthy, -} - -// --------------------------------------------------------------------------- -// Tokenizer -// --------------------------------------------------------------------------- - -#[derive(Debug, Clone, PartialEq)] -enum Token { - Word(String), - OpEq, // = - OpNotEq, // != - OpGt, // > - OpLt, // < - OpGte, // >= - OpLte, // <= - And, // && - Or, // || - Not, // ! - Contains, // contains - Matches, // matches -} - -fn tokenize(input: &str) -> Result, FabroError> { - let input = input.trim(); - if input.is_empty() { - return Ok(Vec::new()); - } - - let chars: Vec = input.chars().collect(); - let len = chars.len(); - let mut i = 0; - let mut tokens = Vec::new(); - - while i < len { - // Skip whitespace - if chars[i].is_whitespace() { - i += 1; - continue; - } - - // Two-char operators (longest match first) - if i + 1 < len { - let two = format!("{}{}", chars[i], chars[i + 1]); - match two.as_str() { - "&&" => { - tokens.push(Token::And); - i += 2; - continue; - } - "||" => { - tokens.push(Token::Or); - i += 2; - continue; - } - "!=" => { - tokens.push(Token::OpNotEq); - i += 2; - continue; - } - ">=" => { - tokens.push(Token::OpGte); - i += 2; - continue; - } - "<=" => { - tokens.push(Token::OpLte); - i += 2; - continue; - } - _ => {} - } - } - - // Single-char operators - match chars[i] { - '=' => { - tokens.push(Token::OpEq); - i += 1; - continue; - } - '>' => { - tokens.push(Token::OpGt); - i += 1; - continue; - } - '<' => { - tokens.push(Token::OpLt); - i += 1; - continue; - } - '!' => { - tokens.push(Token::Not); - i += 1; - continue; - } - _ => {} - } - - // Word: everything up to whitespace or operator char - let start = i; - while i < len && !chars[i].is_whitespace() && !is_op_char(chars[i]) { - i += 1; - } - if i == start { - return Err(FabroError::Parse(format!( - "unexpected character '{}' in condition expression", - chars[i] - ))); - } - let word: String = chars[start..i].iter().collect(); - - // Recognize keyword operators only when they appear between words - // (not as the first or last token, and not adjacent to another operator) - match word.as_str() { - "contains" if is_word_operator_context(&tokens) => { - tokens.push(Token::Contains); - } - "matches" if is_word_operator_context(&tokens) => { - tokens.push(Token::Matches); - } - _ => { - tokens.push(Token::Word(word)); - } - } - } - - Ok(tokens) -} - -fn is_op_char(c: char) -> bool { - matches!(c, '=' | '!' | '>' | '<' | '&' | '|') -} - -/// Word operators (`contains`, `matches`) are recognized when preceded by a Word token. -fn is_word_operator_context(tokens: &[Token]) -> bool { - matches!(tokens.last(), Some(Token::Word(_))) -} - -// --------------------------------------------------------------------------- -// Parser (recursive descent) -// --------------------------------------------------------------------------- - -struct Parser { - tokens: Vec, - pos: usize, -} - -impl Parser { - fn new(tokens: Vec) -> Self { - Self { tokens, pos: 0 } - } - - fn peek(&self) -> Option<&Token> { - self.tokens.get(self.pos) - } - - fn advance(&mut self) -> Option { - let tok = self.tokens.get(self.pos).cloned(); - if tok.is_some() { - self.pos += 1; - } - tok - } - - fn parse_expr(&mut self) -> Result { - self.parse_or() - } - - fn parse_or(&mut self) -> Result { - let mut children = vec![self.parse_and()?]; - while self.peek() == Some(&Token::Or) { - self.advance(); - children.push(self.parse_and()?); - } - if children.len() == 1 { - Ok(children.pop().expect("just checked length")) - } else { - Ok(ConditionExpr::Or(children)) - } - } - - fn parse_and(&mut self) -> Result { - let mut children = vec![self.parse_unary()?]; - while self.peek() == Some(&Token::And) { - self.advance(); - children.push(self.parse_unary()?); - } - if children.len() == 1 { - Ok(children.pop().expect("just checked length")) - } else { - Ok(ConditionExpr::And(children)) - } - } - - fn parse_unary(&mut self) -> Result { - if self.peek() == Some(&Token::Not) { - self.advance(); - let inner = self.parse_unary()?; - return Ok(ConditionExpr::Not(Box::new(inner))); - } - self.parse_clause() - } - - fn parse_clause(&mut self) -> Result { - let key = match self.advance() { - Some(Token::Word(w)) => w, - Some(other) => { - return Err(FabroError::Parse(format!( - "expected key, got {other:?} in condition expression" - ))); - } - None => { - return Err(FabroError::Parse( - "unexpected end of condition expression".to_string(), - )); - } - }; - - // Check for operator - let op = match self.peek() { - Some(Token::OpEq) => Some(Op::Eq), - Some(Token::OpNotEq) => Some(Op::NotEq), - Some(Token::OpGt) => Some(Op::Gt), - Some(Token::OpLt) => Some(Op::Lt), - Some(Token::OpGte) => Some(Op::Gte), - Some(Token::OpLte) => Some(Op::Lte), - Some(Token::Contains) => Some(Op::Contains), - Some(Token::Matches) => Some(Op::Matches), - _ => None, - }; - - let Some(op) = op else { - // Bare key → truthy - return Ok(ConditionExpr::Clause(Clause { - key, - op: Op::Truthy, - value: String::new(), - })); - }; - - self.advance(); // consume the operator - - // Value: must be a Word - let value = match self.advance() { - Some(Token::Word(w)) => w, - Some(other) => { - return Err(FabroError::Parse(format!( - "expected value after operator, got {other:?}" - ))); - } - None => { - // Allow empty value for `=` and `!=` (backward compat: `missing_key=`) - if op == Op::Eq || op == Op::NotEq { - String::new() - } else { - return Err(FabroError::Parse( - "expected value after operator".to_string(), - )); - } - } - }; - - // Validate regex at parse time - if op == Op::Matches { - regex::Regex::new(&value) - .map_err(|e| FabroError::Parse(format!("invalid regex pattern '{value}': {e}")))?; - } - - Ok(ConditionExpr::Clause(Clause { key, op, value })) - } -} - -fn parse_expression(expr: &str) -> Result { - let tokens = tokenize(expr)?; - if tokens.is_empty() { - return Ok(ConditionExpr::And(Vec::new())); - } - let mut parser = Parser::new(tokens); - let result = parser.parse_expr()?; - if parser.pos < parser.tokens.len() { - return Err(FabroError::Parse(format!( - "unexpected token {:?} in condition expression", - parser.tokens[parser.pos] - ))); - } - Ok(result) -} - -/// Parse and validate a condition expression. -/// -/// # Errors -/// -/// Returns an error if the expression contains invalid syntax. -pub fn parse_condition(expr: &str) -> Result<(), FabroError> { - parse_expression(expr)?; - Ok(()) -} - // --------------------------------------------------------------------------- // Evaluator // --------------------------------------------------------------------------- @@ -478,7 +144,8 @@ fn eval_clause(clause: &Clause, outcome: &Outcome, context: &Context) -> bool { /// Empty conditions always return true. #[must_use] pub fn evaluate_condition(expr: &str, outcome: &Outcome, context: &Context) -> bool { - let Ok(parsed) = parse_expression(expr) else { + use fabro_graphviz::condition::parse_condition_expr; + let Ok(parsed) = parse_condition_expr(expr) else { return false; }; eval_expr(&parsed, outcome, context) @@ -702,69 +369,6 @@ mod tests { )); } - // ----------------------------------------------------------------------- - // Phase 0: AST structure tests - // ----------------------------------------------------------------------- - - #[test] - fn parse_eq_into_clause() { - let expr = parse_expression("outcome=success").unwrap(); - assert_eq!( - expr, - ConditionExpr::Clause(Clause { - key: "outcome".to_string(), - op: Op::Eq, - value: "success".to_string(), - }) - ); - } - - #[test] - fn parse_and_into_and_node() { - let expr = parse_expression("a=1 && b=2").unwrap(); - assert_eq!( - expr, - ConditionExpr::And(vec![ - ConditionExpr::Clause(Clause { - key: "a".to_string(), - op: Op::Eq, - value: "1".to_string(), - }), - ConditionExpr::Clause(Clause { - key: "b".to_string(), - op: Op::Eq, - value: "2".to_string(), - }), - ]) - ); - } - - #[test] - fn parse_bare_key_into_truthy() { - let expr = parse_expression("some_flag").unwrap(); - assert_eq!( - expr, - ConditionExpr::Clause(Clause { - key: "some_flag".to_string(), - op: Op::Truthy, - value: String::new(), - }) - ); - } - - #[test] - fn parse_not_eq_into_clause() { - let expr = parse_expression("outcome!=fail").unwrap(); - assert_eq!( - expr, - ConditionExpr::Clause(Clause { - key: "outcome".to_string(), - op: Op::NotEq, - value: "fail".to_string(), - }) - ); - } - // ----------------------------------------------------------------------- // Phase 1: Numeric comparisons // ----------------------------------------------------------------------- @@ -839,14 +443,6 @@ mod tests { )); } - #[test] - fn parse_numeric_comparisons() { - assert!(parse_condition("x > 5").is_ok()); - assert!(parse_condition("x >= 5").is_ok()); - assert!(parse_condition("x < 5").is_ok()); - assert!(parse_condition("x <= 5").is_ok()); - } - // ----------------------------------------------------------------------- // Phase 2: contains operator // ----------------------------------------------------------------------- @@ -898,11 +494,6 @@ mod tests { )); } - #[test] - fn parse_contains() { - assert!(parse_condition("x contains y").is_ok()); - } - // ----------------------------------------------------------------------- // Phase 3: matches operator (regex) // ----------------------------------------------------------------------- @@ -925,16 +516,6 @@ mod tests { )); } - #[test] - fn matches_invalid_regex_fails_parse() { - assert!(parse_condition("x matches [bad").is_err()); - } - - #[test] - fn parse_matches() { - assert!(parse_condition("x matches ^ok$").is_ok()); - } - // ----------------------------------------------------------------------- // Phase 4: OR (||) // ----------------------------------------------------------------------- @@ -980,11 +561,6 @@ mod tests { assert!(!evaluate_condition("a=1 || b=2 && c=3", &outcome, &context)); } - #[test] - fn parse_or() { - assert!(parse_condition("a=1 || b=2").is_ok()); - } - // ----------------------------------------------------------------------- // Phase 5: NOT (!) // ----------------------------------------------------------------------- @@ -1015,9 +591,4 @@ mod tests { &context )); } - - #[test] - fn parse_not() { - assert!(parse_condition("!x=y").is_ok()); - } } diff --git a/lib/crates/fabro-workflows/src/context/keys.rs b/lib/crates/fabro-workflows/src/context/keys.rs index 744c18af5..391521d87 100644 --- a/lib/crates/fabro-workflows/src/context/keys.rs +++ b/lib/crates/fabro-workflows/src/context/keys.rs @@ -1,9 +1,7 @@ -/// Static context key constants and helper functions for dynamic keys. -/// -/// All context keys used across the engine, handlers, and preamble are -/// defined here to prevent typos and improve discoverability. -use std::fmt; -use std::str::FromStr; +// Static context key constants and helper functions for dynamic keys. +// +// All context keys used across the engine, handlers, and preamble are +// defined here to prevent typos and improve discoverability. // --- Top-level keys --- pub const CURRENT_NODE: &str = "current_node"; @@ -84,64 +82,7 @@ pub fn is_engine_internal_key(key: &str) -> bool { || key.starts_with(CURRENT_PREFIX) } -/// Fidelity mode controlling how much prior context is provided to LLM sessions. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] -pub enum Fidelity { - /// Complete context, no summarization — sessions share a thread. - Full, - /// Minimal: only graph goal and run ID. - Truncate, - /// Structured nested-bullet summary (default). - #[default] - Compact, - /// Brief textual summary (~600 token target). - SummaryLow, - /// Moderate textual summary (~1500 token target). - SummaryMedium, - /// Detailed per-stage Markdown report. - SummaryHigh, -} - -impl Fidelity { - /// Degrade full fidelity to summary:high (used on checkpoint resume). - #[must_use] - pub fn degraded(self) -> Self { - match self { - Self::Full => Self::SummaryHigh, - other => other, - } - } -} - -impl fmt::Display for Fidelity { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = match self { - Self::Full => "full", - Self::Truncate => "truncate", - Self::Compact => "compact", - Self::SummaryLow => "summary:low", - Self::SummaryMedium => "summary:medium", - Self::SummaryHigh => "summary:high", - }; - write!(f, "{s}") - } -} - -impl FromStr for Fidelity { - type Err = String; - - fn from_str(s: &str) -> Result { - match s { - "full" => Ok(Self::Full), - "truncate" => Ok(Self::Truncate), - "compact" => Ok(Self::Compact), - "summary:low" => Ok(Self::SummaryLow), - "summary:medium" => Ok(Self::SummaryMedium), - "summary:high" => Ok(Self::SummaryHigh), - other => Err(format!("unknown fidelity mode: {other}")), - } - } -} +pub use fabro_graphviz::Fidelity; #[cfg(test)] mod tests { @@ -167,44 +108,6 @@ mod tests { assert_eq!(retry_count_key("plan"), "internal.retry_count.plan"); } - #[test] - fn fidelity_display_roundtrips() { - let modes = [ - Fidelity::Full, - Fidelity::Truncate, - Fidelity::Compact, - Fidelity::SummaryLow, - Fidelity::SummaryMedium, - Fidelity::SummaryHigh, - ]; - for mode in modes { - let s = mode.to_string(); - let parsed: Fidelity = s.parse().unwrap(); - assert_eq!(parsed, mode); - } - } - - #[test] - fn fidelity_default_is_compact() { - assert_eq!(Fidelity::default(), Fidelity::Compact); - } - - #[test] - fn fidelity_degraded_full_becomes_summary_high() { - assert_eq!(Fidelity::Full.degraded(), Fidelity::SummaryHigh); - } - - #[test] - fn fidelity_degraded_non_full_unchanged() { - assert_eq!(Fidelity::Compact.degraded(), Fidelity::Compact); - assert_eq!(Fidelity::SummaryHigh.degraded(), Fidelity::SummaryHigh); - } - - #[test] - fn fidelity_unknown_mode_errors() { - assert!("bogus".parse::().is_err()); - } - #[test] fn is_engine_internal_key_classifies_correctly() { // Keys that ARE engine-internal (should not propagate) diff --git a/lib/crates/fabro-workflows/src/error.rs b/lib/crates/fabro-workflows/src/error.rs index 948ea74e6..a9991623c 100644 --- a/lib/crates/fabro-workflows/src/error.rs +++ b/lib/crates/fabro-workflows/src/error.rs @@ -419,8 +419,10 @@ impl From for FabroError { impl From for FabroError { fn from(e: fabro_graphviz::error::GraphvizError) -> Self { - let fabro_graphviz::error::GraphvizError::Parse(msg) = e; - Self::Parse(msg) + match e { + fabro_graphviz::error::GraphvizError::Parse(msg) => Self::Parse(msg), + fabro_graphviz::error::GraphvizError::Stylesheet(msg) => Self::Stylesheet(msg), + } } } diff --git a/lib/crates/fabro-workflows/src/handler/manager_loop.rs b/lib/crates/fabro-workflows/src/handler/manager_loop.rs index 57b22ebb0..f8e6ba25d 100644 --- a/lib/crates/fabro-workflows/src/handler/manager_loop.rs +++ b/lib/crates/fabro-workflows/src/handler/manager_loop.rs @@ -12,7 +12,6 @@ use crate::context::Context; use crate::engine::{RunConfig, WorkflowRunEngine}; use crate::error::FabroError; use crate::outcome::{Outcome, StageStatus}; -use crate::validation; use crate::workflow::{prepare_from_file, prepare_from_source}; use fabro_graphviz::graph::{Graph, Node}; @@ -62,7 +61,7 @@ fn parse_child_graph(node: &Node) -> Result { .and_then(|v| v.as_str()) { let (graph, diagnostics) = prepare_from_file(std::path::Path::new(path))?; - validation::raise_on_errors(&diagnostics)?; + fabro_validate::raise_on_errors(&diagnostics).map_err(|e| FabroError::Validation(e.0))?; return Ok(graph); } Err(FabroError::handler("No child workflow source".to_string())) diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index 80e15052d..58bc2345a 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -116,5 +116,4 @@ pub mod run_status; pub mod sandbox_record; pub mod stylesheet; pub mod transform; -pub mod validation; pub mod workflow; diff --git a/lib/crates/fabro-workflows/src/stylesheet.rs b/lib/crates/fabro-workflows/src/stylesheet.rs index 8d3869372..31fddb2d9 100644 --- a/lib/crates/fabro-workflows/src/stylesheet.rs +++ b/lib/crates/fabro-workflows/src/stylesheet.rs @@ -1,180 +1,5 @@ -use crate::error::FabroError; use fabro_graphviz::graph::{AttrValue, Graph}; - -/// A parsed stylesheet selector. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Selector { - /// `*` -- matches all nodes, specificity 0. - Universal, - /// Bare word -- matches nodes by shape name, specificity 1. - Shape(String), - /// `.classname` -- matches nodes with that class, specificity 2. - Class(String), - /// `#nodeid` -- matches a specific node, specificity 3. - Id(String), -} - -impl Selector { - #[must_use] - pub const fn specificity(&self) -> u8 { - match self { - Self::Universal => 0, - Self::Shape(_) => 1, - Self::Class(_) => 2, - Self::Id(_) => 3, - } - } -} - -/// A single CSS-like declaration: `property: value`. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Declaration { - pub property: String, - pub value: String, -} - -/// A stylesheet rule: selector + declarations. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Rule { - pub selector: Selector, - pub declarations: Vec, -} - -/// A parsed stylesheet containing multiple rules. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Stylesheet { - pub rules: Vec, -} - -/// Parse a stylesheet string into a `Stylesheet`. -/// -/// # Errors -/// -/// Returns an error if the input contains invalid stylesheet syntax. -pub fn parse_stylesheet(input: &str) -> Result { - let input = input.trim(); - if input.is_empty() { - return Ok(Stylesheet { rules: Vec::new() }); - } - - let mut rules = Vec::new(); - let mut remaining = input; - - while !remaining.trim().is_empty() { - remaining = remaining.trim(); - - let selector = parse_selector(&mut remaining)?; - if !remaining.starts_with('{') { - return Err(FabroError::Stylesheet(format!( - "expected '{{' after selector, got: {:?}", - &remaining[..remaining.len().min(20)] - ))); - } - remaining = remaining[1..].trim(); - - let declarations = parse_declarations(&mut remaining)?; - remaining = remaining[1..].trim(); // skip '}' - - rules.push(Rule { - selector, - declarations, - }); - } - - Ok(Stylesheet { rules }) -} - -fn parse_selector(remaining: &mut &str) -> Result { - if remaining.starts_with('*') { - *remaining = remaining[1..].trim(); - Ok(Selector::Universal) - } else if remaining.starts_with('#') { - *remaining = remaining[1..].trim(); - let end = remaining - .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(FabroError::Stylesheet( - "expected identifier after '#'".into(), - )); - } - let id = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Id(id)) - } else if remaining.starts_with('.') { - *remaining = remaining[1..].trim(); - let end = remaining - .find(|c: char| !c.is_ascii_lowercase() && !c.is_ascii_digit() && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(FabroError::Stylesheet( - "expected class name after '.'".into(), - )); - } - let class = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Class(class)) - } else { - // Bare word: shape selector - let end = remaining - .find(|c: char| !c.is_ascii_alphanumeric() && c != '_' && c != '-') - .unwrap_or(remaining.len()); - if end == 0 { - return Err(FabroError::Stylesheet(format!( - "expected selector ('*', '#id', '.class', or shape name), got: {:?}", - &remaining[..remaining.len().min(20)] - ))); - } - let shape = remaining[..end].to_string(); - *remaining = remaining[end..].trim(); - Ok(Selector::Shape(shape)) - } -} - -fn parse_declarations(remaining: &mut &str) -> Result, FabroError> { - let mut declarations = Vec::new(); - while !remaining.starts_with('}') { - if remaining.is_empty() { - return Err(FabroError::Stylesheet( - "unexpected end of stylesheet, expected '}'".into(), - )); - } - if remaining.starts_with(';') { - *remaining = remaining[1..].trim(); - continue; - } - - let prop_end = remaining - .find(|c: char| c == ':' || c.is_whitespace()) - .unwrap_or(remaining.len()); - let property = remaining[..prop_end].to_string(); - *remaining = remaining[prop_end..].trim(); - - if !remaining.starts_with(':') { - return Err(FabroError::Stylesheet(format!( - "expected ':' after property name '{property}'" - ))); - } - *remaining = remaining[1..].trim(); - - let val_end = remaining.find([';', '}']).unwrap_or(remaining.len()); - let value = remaining[..val_end].trim().to_string(); - *remaining = remaining[val_end..].trim(); - - if value.is_empty() { - return Err(FabroError::Stylesheet(format!( - "empty value for property '{property}'" - ))); - } - - declarations.push(Declaration { property, value }); - - if remaining.starts_with(';') { - *remaining = remaining[1..].trim(); - } - } - Ok(declarations) -} +pub use fabro_graphviz::stylesheet::{parse_stylesheet, Declaration, Rule, Selector, Stylesheet}; /// Recognized stylesheet properties. const STYLESHEET_PROPERTIES: &[&str] = &["model", "provider", "reasoning_effort", "backend"]; @@ -237,59 +62,6 @@ mod tests { use super::*; use fabro_graphviz::graph::Node; - #[test] - fn parse_empty_stylesheet() { - let ss = parse_stylesheet("").unwrap(); - assert!(ss.rules.is_empty()); - } - - #[test] - fn parse_universal_rule() { - let ss = parse_stylesheet("* { model: claude-sonnet-4-5; provider: anthropic; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Universal); - assert_eq!(ss.rules[0].declarations.len(), 2); - assert_eq!(ss.rules[0].declarations[0].property, "model"); - assert_eq!(ss.rules[0].declarations[0].value, "claude-sonnet-4-5"); - } - - #[test] - fn parse_class_rule() { - let ss = parse_stylesheet(".code { model: claude-opus-4-6; }").unwrap(); - assert_eq!(ss.rules[0].selector, Selector::Class("code".into())); - } - - #[test] - fn parse_id_rule() { - let ss = parse_stylesheet("#critical_review { model: gpt-5.2; reasoning_effort: high; }") - .unwrap(); - assert_eq!(ss.rules[0].selector, Selector::Id("critical_review".into())); - assert_eq!(ss.rules[0].declarations.len(), 2); - } - - #[test] - fn parse_multiple_rules() { - let input = r" - * { model: claude-sonnet-4-5; provider: anthropic; } - .code { model: claude-opus-4-6; provider: anthropic; } - #critical_review { model: gpt-5.2; provider: openai; reasoning_effort: high; } - "; - let ss = parse_stylesheet(input).unwrap(); - assert_eq!(ss.rules.len(), 3); - } - - #[test] - fn parse_error_missing_brace() { - let result = parse_stylesheet("* model: test; }"); - assert!(result.is_err()); - } - - #[test] - fn parse_error_missing_selector() { - let result = parse_stylesheet("{ model: test; }"); - assert!(result.is_err()); - } - #[test] fn apply_universal_to_all_nodes() { let ss = parse_stylesheet("* { model: sonnet; }").unwrap(); @@ -367,14 +139,6 @@ mod tests { ); } - #[test] - fn selector_specificity_values() { - assert_eq!(Selector::Universal.specificity(), 0); - assert_eq!(Selector::Shape("box".into()).specificity(), 1); - assert_eq!(Selector::Class("x".into()).specificity(), 2); - assert_eq!(Selector::Id("x".into()).specificity(), 3); - } - #[test] fn spec_section_86_example() { let input = r" @@ -423,14 +187,6 @@ mod tests { ); } - #[test] - fn parse_shape_selector() { - let ss = parse_stylesheet("box { model: opus; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Shape("box".into())); - assert_eq!(ss.rules[0].declarations[0].value, "opus"); - } - #[test] fn apply_shape_selector_to_matching_nodes() { let ss = parse_stylesheet("box { model: opus; }").unwrap(); @@ -456,58 +212,6 @@ mod tests { assert_eq!(graph.nodes["b"].attrs.get("model"), None); } - #[test] - fn shape_overrides_universal_specificity() { - let ss = parse_stylesheet("* { model: sonnet; } box { model: opus; }").unwrap(); - let mut graph = Graph::new("test"); - graph.nodes.insert("a".into(), Node::new("a")); // default shape = box - apply_stylesheet(&ss, &mut graph); - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("opus".into())) - ); - } - - #[test] - fn class_overrides_shape_specificity() { - let ss = parse_stylesheet("box { model: opus; } .fast { model: flash; }").unwrap(); - let mut graph = Graph::new("test"); - let mut node = Node::new("a"); - node.classes.push("fast".into()); - graph.nodes.insert("a".into(), node); - apply_stylesheet(&ss, &mut graph); - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("flash".into())) - ); - } - - #[test] - fn class_overrides_universal_specificity() { - let ss = parse_stylesheet("* { model: sonnet; } .special { model: gpt; }").unwrap(); - let mut graph = Graph::new("test"); - - let mut node_a = Node::new("a"); - node_a.classes.push("special".into()); - graph.nodes.insert("a".into(), node_a); - - let node_b = Node::new("b"); - graph.nodes.insert("b".into(), node_b); - - apply_stylesheet(&ss, &mut graph); - - // .special (specificity 1) overrides * (specificity 0) - assert_eq!( - graph.nodes["a"].attrs.get("model"), - Some(&AttrValue::String("gpt".into())) - ); - // No class, gets universal - assert_eq!( - graph.nodes["b"].attrs.get("model"), - Some(&AttrValue::String("sonnet".into())) - ); - } - #[test] fn apply_backend_property_via_stylesheet() { let ss = parse_stylesheet("* { backend: cli; }").unwrap(); diff --git a/lib/crates/fabro-workflows/src/workflow.rs b/lib/crates/fabro-workflows/src/workflow.rs index f0b7334dd..508a5ba2d 100644 --- a/lib/crates/fabro-workflows/src/workflow.rs +++ b/lib/crates/fabro-workflows/src/workflow.rs @@ -5,8 +5,8 @@ use crate::transform::{ FileInliningTransform, ProviderInferenceTransform, StylesheetApplicationTransform, Transform, VariableExpansionTransform, }; -use crate::validation::{self, Diagnostic}; use fabro_graphviz::graph::Graph; +use fabro_validate::Diagnostic; /// Builder for configuring and executing a workflow preparation. /// Collects custom transforms that run after the built-in ones. @@ -74,7 +74,7 @@ impl WorkflowBuilder { transform.apply(&mut graph); } - let diagnostics = validation::validate(&graph, &[]); + let diagnostics = fabro_validate::validate(&graph, &[]); Ok((graph, diagnostics)) } } @@ -106,7 +106,7 @@ pub fn prepare_from_file(path: &Path) -> Result<(Graph, Vec), FabroE pub fn prepare_from_source(dot_source: &str) -> Result { let builder = WorkflowBuilder::new(); let (graph, diagnostics) = builder.prepare(dot_source)?; - validation::raise_on_errors(&diagnostics)?; + fabro_validate::raise_on_errors(&diagnostics).map_err(|e| FabroError::Validation(e.0))?; Ok(graph) } diff --git a/lib/crates/fabro-workflows/tests/integration.rs b/lib/crates/fabro-workflows/tests/integration.rs index a0fc90a9e..a54866ddb 100644 --- a/lib/crates/fabro-workflows/tests/integration.rs +++ b/lib/crates/fabro-workflows/tests/integration.rs @@ -6,6 +6,7 @@ use std::time::Duration; use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_graphviz::parser::parse; use fabro_llm::provider::Provider; +use fabro_validate::{validate, validate_or_raise, Severity}; use fabro_workflows::checkpoint::Checkpoint; use fabro_workflows::cli::backend::AgentApiBackend; use fabro_workflows::context::Context; @@ -31,7 +32,6 @@ use fabro_workflows::stylesheet::{apply_stylesheet, parse_stylesheet}; use fabro_workflows::transform::{ StylesheetApplicationTransform, Transform, VariableExpansionTransform, }; -use fabro_workflows::validation::{validate, validate_or_raise, Severity}; fn local_env() -> Arc { Arc::new(fabro_agent::LocalSandbox::new( @@ -69,7 +69,7 @@ fn parse_and_validate_simple_linear() { let diagnostics = validate_or_raise(&graph, &[]).expect("validation should pass"); let errors: Vec<_> = diagnostics .iter() - .filter(|d| d.severity == fabro_workflows::validation::Severity::Error) + .filter(|d| d.severity == fabro_validate::Severity::Error) .collect(); assert!(errors.is_empty(), "expected no validation errors"); } @@ -115,7 +115,7 @@ fn parse_and_validate_branching_with_conditions() { let diagnostics = validate_or_raise(&graph, &[]).expect("validation should pass"); let errors: Vec<_> = diagnostics .iter() - .filter(|d| d.severity == fabro_workflows::validation::Severity::Error) + .filter(|d| d.severity == fabro_validate::Severity::Error) .collect(); assert!(errors.is_empty(), "expected no validation errors"); } @@ -154,7 +154,7 @@ fn parse_and_validate_human_gate() { let diagnostics = validate_or_raise(&graph, &[]).expect("validation should pass"); let errors: Vec<_> = diagnostics .iter() - .filter(|d| d.severity == fabro_workflows::validation::Severity::Error) + .filter(|d| d.severity == fabro_validate::Severity::Error) .collect(); assert!(errors.is_empty(), "expected no validation errors"); }