From 0ee5b079c5d401b2321067dc63ef43e13446784b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 13 Mar 2026 20:58:08 -0400 Subject: [PATCH] Add thread_id_requires_fidelity_full lint rule Warn when thread_id is set without fidelity=full, since session reuse only works with full fidelity. Checks node-level, edge-level, and graph-level default_thread attributes. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../fabro-workflows/src/validation/rules.rs | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/lib/crates/fabro-workflows/src/validation/rules.rs b/lib/crates/fabro-workflows/src/validation/rules.rs index 91821c907..e041e830b 100644 --- a/lib/crates/fabro-workflows/src/validation/rules.rs +++ b/lib/crates/fabro-workflows/src/validation/rules.rs @@ -31,6 +31,7 @@ pub fn built_in_rules() -> Vec> { Box::new(ScriptAbsoluteCdRule), Box::new(StylesheetModelKnownRule), Box::new(UnresolvedFileRefRule), + Box::new(ThreadIdRequiresFidelityFullRule), ] } @@ -1019,6 +1020,76 @@ impl LintRule for UnresolvedFileRefRule { } } +// --- Rule 22: thread_id_requires_fidelity_full (WARNING) --- + +struct ThreadIdRequiresFidelityFullRule; + +impl ThreadIdRequiresFidelityFullRule { + const FIX: &str = "Add fidelity=\"full\" to enable session reuse, or remove thread_id"; +} + +impl LintRule for ThreadIdRequiresFidelityFullRule { + fn name(&self) -> &'static str { + "thread_id_requires_fidelity_full" + } + + fn apply(&self, graph: &Graph) -> Vec { + let mut diagnostics = Vec::new(); + let graph_default_full = graph.default_fidelity() == Some("full"); + + for node in graph.nodes.values() { + if node.thread_id().is_some() && node.fidelity() != Some("full") && !graph_default_full + { + diagnostics.push(Diagnostic { + rule: self.name().to_string(), + severity: Severity::Warning, + 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()), + }); + } + } + + for edge in &graph.edges { + if edge.thread_id().is_some() { + let edge_full = edge.fidelity() == Some("full"); + let target_full = + 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(), + severity: Severity::Warning, + 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()), + }); + } + } + } + + if graph.default_thread().is_some() && !graph_default_full { + diagnostics.push(Diagnostic { + 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()), + }); + } + + diagnostics + } +} + #[cfg(test)] mod tests { use super::*; @@ -2942,4 +3013,157 @@ mod tests { let d = rule.apply(&g); assert!(d.is_empty()); } + + // thread_id_requires_fidelity_full rule tests + + #[test] + fn thread_id_requires_fidelity_full_node_warns() { + let mut g = minimal_graph(); + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + g.nodes.insert("work".to_string(), node); + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Warning); + assert_eq!(d[0].node_id, Some("work".to_string())); + } + + #[test] + fn thread_id_requires_fidelity_full_node_ok() { + let mut g = minimal_graph(); + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + node.attrs.insert( + "fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + g.nodes.insert("work".to_string(), node); + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn thread_id_requires_fidelity_full_node_graph_default_ok() { + let mut g = minimal_graph(); + g.attrs.insert( + "default_fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + g.nodes.insert("work".to_string(), node); + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn thread_id_requires_fidelity_full_edge_warns() { + let mut g = minimal_graph(); + let mut edge = Edge::new("start", "exit"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + g.edges = vec![edge]; + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Warning); + assert_eq!(d[0].edge, Some(("start".to_string(), "exit".to_string()))); + } + + #[test] + fn thread_id_requires_fidelity_full_edge_ok() { + let mut g = minimal_graph(); + let mut edge = Edge::new("start", "exit"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + edge.attrs.insert( + "fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + g.edges = vec![edge]; + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn thread_id_requires_fidelity_full_edge_target_node_ok() { + let mut g = minimal_graph(); + if let Some(exit_node) = g.nodes.get_mut("exit") { + exit_node.attrs.insert( + "fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + } + let mut edge = Edge::new("start", "exit"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("session1".to_string()), + ); + g.edges = vec![edge]; + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn thread_id_requires_fidelity_full_graph_warns() { + let mut g = minimal_graph(); + g.attrs.insert( + "default_thread".to_string(), + AttrValue::String("session1".to_string()), + ); + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Warning); + assert!(d[0].node_id.is_none()); + assert!(d[0].edge.is_none()); + } + + #[test] + fn thread_id_requires_fidelity_full_graph_ok() { + let mut g = minimal_graph(); + g.attrs.insert( + "default_thread".to_string(), + AttrValue::String("session1".to_string()), + ); + g.attrs.insert( + "default_fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + + let rule = ThreadIdRequiresFidelityFullRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } }