Simplify edge-only node validation

lib/apps/fabro-cli/src/shared/utilities.rs

@@ -66,43 +66,28 @@ fn print_diagnostic(d: &Diagnostic, styles: &Styles, printer: Printer) {
         _ => String::new(),
     };
     let source_prefix = source_prefix(d);
+    let body = if source_prefix.is_empty() {
+        format!("{location}: {}", d.message)
+    } else {
+        format!(": {source_prefix}{}{location}", d.message)
+    };
     match d.severity {
-        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::Error => fabro_util::printerr!(
             printer,
-            "{}: {source_prefix}{}{location} ({})",
+            "{}{body} ({})",
             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} ({})",
+            "{}{body} ({})",
             styles.yellow.apply_to("warning"),
-            d.message,
             styles.dim.apply_to(&d.rule),
         ),
         Severity::Info => fabro_util::printerr!(
             printer,
             "{}",
-            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)
-            }),
+            styles.dim.apply_to(format!("info{body} ({})", d.rule)),
         ),
     }
 }

lib/apps/fabro-cli/tests/it/cmd/validate.rs

@@ -295,12 +295,10 @@ fn edge_only_node() {
     exit_code: 1
     ----- stdout -----
     ----- stderr -----
-    Workflow: EdgeOnlyNode (3 nodes, 2 edges)
+    Workflow: EdgeOnlyNode (2 nodes, 2 edges)
     Graph: [FIXTURES]/edge_only_node.fabro
     error [node: misspelled_node]: Node 'misspelled_node' is referenced by edge 'start -> misspelled_node' but has no node declaration (edge_target_exists)
       fix: Declare node 'misspelled_node' or correct the edge endpoint
-    warning [node: misspelled_node]: LLM node 'misspelled_node' has no prompt or label attribute (prompt_on_llm_nodes)
-      fix: Add a prompt or label attribute
       × Validation failed
     ");
 }

lib/components/fabro-graphviz/src/parser/semantic.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::time::Duration;
 
 use crate::error::Error;
@@ -57,49 +57,44 @@ fn derive_class_from_label(label: &str) -> String {
         .collect()
 }
 
-/// How a statement named a node. A node stays implicit only while every
-/// mention of it is an edge endpoint, so declaration order does not matter.
-#[derive(Clone, Copy, PartialEq, Eq)]
-enum Mention {
-    Declaration,
-    EdgeEndpoint,
+fn collect_declared_node_ids(statements: &[Statement], node_ids: &mut HashSet<String>) {
+    for statement in statements {
+        match statement {
+            Statement::Node(node) => {
+                node_ids.insert(node.id.clone());
+            }
+            Statement::Subgraph(subgraph) => {
+                collect_declared_node_ids(&subgraph.statements, node_ids);
+            }
+            _ => {}
+        }
+    }
 }
 
 struct SemanticState {
-    graph:         Graph,
-    node_defaults: HashMap<String, AttrValue>,
-    edge_defaults: HashMap<String, AttrValue>,
+    graph:             Graph,
+    declared_node_ids: HashSet<String>,
+    node_defaults:     HashMap<String, AttrValue>,
+    edge_defaults:     HashMap<String, AttrValue>,
 }
 
 impl SemanticState {
-    fn new(name: String) -> Self {
+    fn new(name: String, declared_node_ids: HashSet<String>) -> Self {
         Self {
-            graph:         Graph::new(name),
+            graph: Graph::new(name),
+            declared_node_ids,
             node_defaults: HashMap::new(),
             edge_defaults: HashMap::new(),
         }
     }
 
-    /// Insert the node if this is the first statement to mention it, and record
-    /// whether the workflow ever declares it.
-    fn ensure_node(&mut self, id: &str, mention: Mention) {
-        if !self.graph.nodes.contains_key(id) {
+    fn ensure_node(&mut self, id: &str) -> &mut Node {
+        let node_defaults = &self.node_defaults;
+        self.graph.nodes.entry(id.to_string()).or_insert_with(|| {
             let mut node = Node::new(id);
-            for (k, v) in &self.node_defaults {
-                node.attrs.insert(k.clone(), v.clone());
-            }
-            node.implicit = mention == Mention::EdgeEndpoint;
-            self.graph.nodes.insert(id.to_string(), node);
-            return;
-        }
-        if mention == Mention::Declaration {
-            let node = self
-                .graph
-                .nodes
-                .get_mut(id)
-                .expect("contains_key returned true, so get_mut cannot return None");
-            node.implicit = false;
-        }
+            node.attrs.clone_from(node_defaults);
+            node
+        })
     }
 
     fn add_class_to_node(node: &mut Node, cls: &str) {
@@ -110,12 +105,7 @@ impl SemanticState {
     }
 
     fn process_node(&mut self, node_stmt: &NodeStmt, subgraph_class: Option<&str>) {
-        self.ensure_node(&node_stmt.id, Mention::Declaration);
-        let node = self
-            .graph
-            .nodes
-            .get_mut(&node_stmt.id)
-            .expect("node was just inserted by ensure_node, so get_mut cannot return None");
+        let node = self.ensure_node(&node_stmt.id);
         if let Some(attrs) = &node_stmt.attrs {
             for (k, v) in attrs {
                 node.attrs.insert(k.clone(), convert_value(v));
@@ -131,11 +121,6 @@ impl SemanticState {
             .and_then(AttrValue::as_str)
             .map(String::from);
         if let Some(class_str) = class_str {
-            let node = self
-                .graph
-                .nodes
-                .get_mut(&node_stmt.id)
-                .expect("node was just inserted by ensure_node, so get_mut cannot return None");
             for cls in class_str.split(',') {
                 let cls = cls.trim().to_string();
                 if !cls.is_empty() && !node.classes.contains(&cls) {
@@ -147,12 +132,11 @@ impl SemanticState {
 
     fn process_edge(&mut self, edge_stmt: &EdgeStmt, subgraph_class: Option<&str>) {
         for id in &edge_stmt.nodes {
-            self.ensure_node(id, Mention::EdgeEndpoint);
+            if !self.declared_node_ids.contains(id) {
+                continue;
+            }
+            let node = self.ensure_node(id);
             if let Some(cls) = subgraph_class {
-                let node =
-                    self.graph.nodes.get_mut(id).expect(
-                        "node was just inserted by ensure_node, so get_mut cannot return None",
-                    );
                 Self::add_class_to_node(node, cls);
             }
         }
@@ -271,7 +255,9 @@ impl SemanticState {
 ///
 /// Returns an error if the AST cannot be converted to a valid graph.
 pub fn ast_to_graph(dot: &DotGraph) -> Result<Graph, Error> {
-    let mut state = SemanticState::new(dot.name.clone());
+    let mut declared_node_ids = HashSet::new();
+    collect_declared_node_ids(&dot.statements, &mut declared_node_ids);
+    let mut state = SemanticState::new(dot.name.clone(), declared_node_ids);
     let empty = HashMap::new();
     state.process_statements(&dot.statements, None, &empty, &empty);
     Ok(state.graph)
@@ -535,7 +521,7 @@ mod tests {
     }
 
     #[test]
-    fn ast_to_graph_implicit_nodes_from_edges() {
+    fn ast_to_graph_keeps_undeclared_edge_endpoints_out_of_nodes() {
         let dot = DotGraph {
             name:       "Implicit".into(),
             statements: vec![Statement::Edge(EdgeStmt {
@@ -545,14 +531,12 @@ mod tests {
         };
 
         let graph = ast_to_graph(&dot).unwrap();
-        assert!(graph.nodes.contains_key("a"));
-        assert!(graph.nodes.contains_key("b"));
-        assert!(graph.nodes["a"].implicit);
-        assert!(graph.nodes["b"].implicit);
+        assert!(graph.nodes.is_empty());
+        assert_eq!(graph.edges, vec![Edge::new("a", "b")]);
     }
 
     #[test]
-    fn ast_to_graph_marks_declared_nodes_explicit() {
+    fn ast_to_graph_includes_only_declared_edge_endpoints() {
         let dot = DotGraph {
             name:       "Declared".into(),
             statements: vec![
@@ -568,8 +552,8 @@ mod tests {
         };
 
         let graph = ast_to_graph(&dot).unwrap();
-        assert!(!graph.nodes["a"].implicit);
-        assert!(graph.nodes["b"].implicit);
+        assert!(graph.nodes.contains_key("a"));
+        assert!(!graph.nodes.contains_key("b"));
     }
 
     #[test]
@@ -577,10 +561,12 @@ mod tests {
         let dot = DotGraph {
             name:       "DeclaredLater".into(),
             statements: vec![
+                Statement::NodeDefaults(vec![("model".into(), AstValue::Str("first".into()))]),
                 Statement::Edge(EdgeStmt {
                     nodes: vec!["a".into(), "b".into()],
                     attrs: None,
                 }),
+                Statement::NodeDefaults(vec![("model".into(), AstValue::Str("second".into()))]),
                 Statement::Node(NodeStmt {
                     id:    "b".into(),
                     attrs: Some(vec![("prompt".into(), AstValue::Str("Do it".into()))]),
@@ -589,7 +575,15 @@ mod tests {
         };
 
         let graph = ast_to_graph(&dot).unwrap();
-        assert!(!graph.nodes["b"].implicit);
+        assert!(graph.nodes.contains_key("b"));
+        assert!(!graph.nodes.contains_key("a"));
+        assert_eq!(
+            graph.nodes["b"]
+                .attrs
+                .get("model")
+                .and_then(AttrValue::as_str),
+            Some("first")
+        );
     }
 
     #[test]
@@ -612,7 +606,7 @@ mod tests {
         };
 
         let graph = ast_to_graph(&dot).unwrap();
-        assert!(!graph.nodes["plan"].implicit);
-        assert!(graph.nodes["start"].implicit);
+        assert!(graph.nodes.contains_key("plan"));
+        assert!(!graph.nodes.contains_key("start"));
     }
 }

lib/components/fabro-validate/src/rules/edge_target_exists.rs

@@ -11,13 +11,6 @@ pub(super) fn rule() -> Box<dyn LintRule> {
 struct Rule;
 
 impl Rule {
-    /// An edge endpoint is only usable when the workflow declares it. A node
-    /// the parser synthesized from the edge itself carries no attributes, so it
-    /// would silently run as a default agent stage.
-    fn is_declared(graph: &Graph, node_id: &str) -> bool {
-        graph.nodes.get(node_id).is_some_and(|node| !node.implicit)
-    }
-
     fn diagnostic(&self, node_id: &str, from: &str, to: &str) -> Diagnostic {
         Diagnostic {
             rule: self.name().to_string(),
@@ -47,7 +40,7 @@ impl LintRule for Rule {
         let mut reported = HashSet::new();
         for edge in &graph.edges {
             for endpoint in [&edge.to, &edge.from] {
-                if !Self::is_declared(graph, endpoint) && reported.insert(endpoint) {
+                if !graph.nodes.contains_key(endpoint) && reported.insert(endpoint) {
                     diagnostics.push(self.diagnostic(endpoint, &edge.from, &edge.to));
                 }
             }
@@ -71,8 +64,8 @@ mod tests {
 
     fn undeclared_nodes(graph: &Graph) -> Vec<String> {
         Rule.apply(graph)
-            .iter()
-            .map(|d| d.node_id.clone().expect("diagnostic should name a node"))
+            .into_iter()
+            .map(|d| d.node_id.expect("diagnostic should name a node"))
             .collect()
     }
 

lib/components/fabro-workflow/src/transforms/import.rs

@@ -339,19 +339,18 @@ impl ImportTransform {
             .edges
             .retain(|edge| edge.from != placeholder_id && edge.to != placeholder_id);
 
-        for (node_id, node) in imported_graph.nodes {
+        for (node_id, mut merged_node) in imported_graph.nodes {
             if node_id == start_id || node_id == exit_id {
                 continue;
             }
 
             let prefixed_id = format!("{placeholder_id}.{node_id}");
-            let mut merged_node = Node::new(&prefixed_id);
-            merged_node.implicit = node.implicit;
+            merged_node.id.clone_from(&prefixed_id);
+            let imported_attrs = std::mem::take(&mut merged_node.attrs);
             merged_node.attrs.clone_from(&placeholder.default_attrs);
-            merged_node.attrs.extend(node.attrs);
+            merged_node.attrs.extend(imported_attrs);
             Self::remap_retry_target(&mut merged_node.attrs, placeholder_id);
 
-            merged_node.classes = node.classes;
             for class_name in &placeholder.class_names {
                 Self::push_class(&mut merged_node.classes, class_name);
             }
@@ -650,7 +649,10 @@ impl PreparedImport {
         self.graph.nodes.iter().all(|(node_id, node)| {
             ImportTransform::is_start_sentinel(node_id, node)
                 || ImportTransform::is_exit_sentinel(node_id, node)
-        })
+        }) && matches!(
+            self.graph.edges.as_slice(),
+            [edge] if edge.from == self.start_id && edge.to == self.exit_id
+        )
     }
 }
 
@@ -910,6 +912,7 @@ mod tests {
         assert!(!graph.nodes.contains_key("validate"));
         assert!(graph.nodes.contains_key("validate.lint"));
         assert!(graph.nodes.contains_key("validate.test"));
+        assert_eq!(graph.nodes["validate.lint"].id, "validate.lint");
         assert!(!graph.nodes.contains_key("validate.start"));
         assert!(!graph.nodes.contains_key("validate.exit"));
 
@@ -953,27 +956,7 @@ mod tests {
     }
 
     #[test]
-    fn imported_node_declarations_survive_splicing() {
-        let dir = tempfile::tempdir().unwrap();
-        write_file(&dir.path().join("validate.fabro"), basic_import_source());
-
-        let graph = apply_import(
-            r#"digraph Deploy {
-                start [shape=Mdiamond]
-                validate [import="./validate.fabro"]
-                exit [shape=Msquare]
-                start -> validate -> exit
-            }"#,
-            dir.path(),
-            None,
-        );
-
-        assert!(!graph.nodes["validate.lint"].implicit);
-        assert!(!graph.nodes["validate.test"].implicit);
-    }
-
-    #[test]
-    fn edge_only_node_in_imported_fragment_stays_undeclared() {
+    fn edge_only_node_in_imported_fragment_stays_missing() {
         let dir = tempfile::tempdir().unwrap();
         write_file(
             &dir.path().join("validate.fabro"),
@@ -996,8 +979,46 @@ mod tests {
             None,
         );
 
-        assert!(graph.nodes["validate.typo"].implicit);
-        assert!(!graph.nodes["validate.lint"].implicit);
+        assert!(!graph.nodes.contains_key("validate.typo"));
+        assert!(graph.nodes.contains_key("validate.lint"));
+    }
+
+    #[test]
+    fn edge_only_body_is_not_treated_as_empty_import() {
+        let dir = tempfile::tempdir().unwrap();
+        write_file(
+            &dir.path().join("validate.fabro"),
+            r"digraph validate {
+                start [shape=Mdiamond]
+                exit [shape=Msquare]
+                start -> typo -> exit
+            }",
+        );
+
+        let graph = apply_import(
+            r#"digraph Deploy {
+                start [shape=Mdiamond]
+                validate [import="./validate.fabro"]
+                exit [shape=Msquare]
+                start -> validate -> exit
+            }"#,
+            dir.path(),
+            None,
+        );
+
+        assert!(!graph.nodes.contains_key("validate.typo"));
+        assert!(
+            graph
+                .edges
+                .iter()
+                .any(|edge| edge.from == "start" && edge.to == "validate.typo")
+        );
+        assert!(
+            graph
+                .edges
+                .iter()
+                .any(|edge| edge.from == "validate.typo" && edge.to == "exit")
+        );
     }
 
     #[test]
@@ -1179,6 +1200,48 @@ mod tests {
         );
     }
 
+    #[test]
+    fn imported_start_and_exit_sentinels_must_be_declared() {
+        let dir = tempfile::tempdir().unwrap();
+        let host = r#"digraph Deploy {
+            start [shape=Mdiamond]
+            validate [import="./validate.fabro"]
+            exit [shape=Msquare]
+            start -> validate -> exit
+        }"#;
+        let cases = [
+            (
+                r#"digraph validate {
+                    work [prompt="Run checks"]
+                    exit [shape=Msquare]
+                    start -> work -> exit
+                }"#,
+                "imported workflow must have exactly one start node, found 0",
+            ),
+            (
+                r#"digraph validate {
+                    start [shape=Mdiamond]
+                    work [prompt="Run checks"]
+                    start -> work -> exit
+                }"#,
+                "imported workflow must have exactly one exit node, found 0",
+            ),
+        ];
+
+        for (source, expected_error) in cases {
+            write_file(&dir.path().join("validate.fabro"), source);
+            let graph = apply_import(host, dir.path(), None);
+
+            assert_eq!(
+                graph.nodes["validate"]
+                    .attrs
+                    .get("import_error")
+                    .and_then(AttrValue::as_str),
+                Some(expected_error)
+            );
+        }
+    }
+
     #[test]
     fn multiple_entry_nodes_poison_placeholder() {
         let dir = tempfile::tempdir().unwrap();

lib/foundation/fabro-types/src/graph.rs

@@ -119,26 +119,20 @@ pub fn shape_to_handler_type(shape: &str) -> Option<&'static str> {
 /// A node in the workflow graph.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct Node {
-    pub id:       String,
-    pub attrs:    HashMap<String, AttrValue>,
+    pub id:      String,
+    pub attrs:   HashMap<String, AttrValue>,
     /// CSS-like classes for model stylesheet targeting (from `class` attr and
     /// subgraph derivation).
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub classes:  Vec<String>,
-    /// True when the node was synthesized from an edge endpoint instead of a
-    /// node declaration. Validation rejects these because an edge-only node in
-    /// an executable workflow is almost always a typo.
-    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
-    pub implicit: bool,
+    pub classes: Vec<String>,
 }
 
 impl Node {
     pub fn new(id: impl Into<String>) -> Self {
         Self {
-            id:       id.into(),
-            attrs:    HashMap::new(),
-            classes:  Vec::new(),
-            implicit: false,
+            id:      id.into(),
+            attrs:   HashMap::new(),
+            classes: Vec::new(),
         }
     }
 

lib/foundation/fabro-types/src/run_event/mod.rs

@@ -926,8 +926,6 @@ impl<'de> Deserialize<'de> for RunEvent {
 
 #[cfg(test)]
 mod tests {
-    use std::collections::HashMap;
-
     use serde_json::json;
 
     use super::*;
@@ -992,21 +990,9 @@ mod tests {
     #[test]
     fn run_event_deserializes_adjacent_layout() {
         let settings = WorkflowSettings::default();
-        let graph = Graph {
-            name:  "test".to_string(),
-            nodes: HashMap::from([("start".to_string(), Node {
-                id:       "start".to_string(),
-                attrs:    HashMap::new(),
-                classes:  Vec::new(),
-                implicit: false,
-            })]),
-            edges: vec![Edge {
-                from:  "start".to_string(),
-                to:    "done".to_string(),
-                attrs: HashMap::new(),
-            }],
-            attrs: HashMap::new(),
-        };
+        let mut graph = Graph::new("test");
+        graph.nodes.insert("start".to_string(), Node::new("start"));
+        graph.edges.push(Edge::new("start", "done"));
 
         let line = json!({
             "id": "evt_2",