From a2f9e87b7ca08c98910b31f9e12ffcd1eeb32dbe Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 22 Feb 2026 10:49:21 -0400 Subject: [PATCH] Close spec compliance gaps: retry context, thread resolution, fan-in logging Fix 3 confirmed gaps from spec compliance review (85 items, 91.8% aligned): - Write internal.retry_count. to PipelineContext after retries so handlers and conditions can access retry counts (spec 5.1) - Add graph-level default_thread (step 3) to 5-step thread ID resolution, pass graph param to resolve_thread_id (spec 5.4) - Write prompt.md/response.md in fan_in LLM evaluation path (spec 5.6) Also includes pre-existing improvements: checkpoint stores node_outcomes and next_node_id for correct resume, engine timeout enforcement, auto_status support, fidelity degradation on resume, preamble injection, is_retryable error classification, stylesheet specificity correction, full stylesheet parse validation, direction_valid lint rule, pre-hook returns Skipped not Fail, fan-in score-based sorting and all-fail detection, manager_loop child autostart and steer cooldown. Co-Authored-By: Claude Opus 4.6 --- crates/attractor/src/checkpoint.rs | 37 +- crates/attractor/src/engine.rs | 574 +++++++++++++- crates/attractor/src/error.rs | 33 + crates/attractor/src/graph/types.rs | 7 + crates/attractor/src/handler/codergen.rs | 8 +- crates/attractor/src/handler/fan_in.rs | 125 +++- crates/attractor/src/handler/manager_loop.rs | 58 +- crates/attractor/src/pipeline.rs | 7 +- crates/attractor/src/stylesheet.rs | 88 +-- crates/attractor/src/validation/rules.rs | 123 ++- crates/attractor/tests/integration.rs | 271 ++++++- docs/agent/reviews/confirmed-gaps.md | 185 +++++ .../reviews/spec-compliance-full-review.md | 224 ++++++ docs/agent/reviews/spec-compliance-review.md | 701 ++++++++++++++++++ docs/specs/attractor-spec.md | 2 +- 15 files changed, 2310 insertions(+), 133 deletions(-) create mode 100644 docs/agent/reviews/confirmed-gaps.md create mode 100644 docs/agent/reviews/spec-compliance-full-review.md create mode 100644 docs/agent/reviews/spec-compliance-review.md diff --git a/crates/attractor/src/checkpoint.rs b/crates/attractor/src/checkpoint.rs index 1dc790026..2da1b9046 100644 --- a/crates/attractor/src/checkpoint.rs +++ b/crates/attractor/src/checkpoint.rs @@ -7,6 +7,7 @@ use serde_json::Value; use crate::context::Context; use crate::error::{AttractorError, Result}; +use crate::outcome::Outcome; /// Serializable snapshot of execution state for crash recovery and resume. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -17,6 +18,12 @@ pub struct Checkpoint { pub node_retries: HashMap, pub context_values: HashMap, pub logs: Vec, + /// Persisted node outcomes for goal gate checks after resume. + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub node_outcomes: HashMap, + /// The node to resume execution at (the next node after the checkpoint's current_node). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub next_node_id: Option, } impl Checkpoint { @@ -25,14 +32,19 @@ impl Checkpoint { context: &Context, current_node: impl Into, completed_nodes: Vec, + node_retries: HashMap, + node_outcomes: HashMap, + next_node_id: Option, ) -> Self { Self { timestamp: Utc::now(), current_node: current_node.into(), completed_nodes, - node_retries: HashMap::new(), + node_retries, context_values: context.snapshot(), logs: context.logs_snapshot(), + node_outcomes, + next_node_id, } } @@ -75,6 +87,9 @@ mod tests { &ctx, "node_a", vec!["start".to_string(), "node_a".to_string()], + HashMap::new(), + HashMap::new(), + None, ); assert_eq!(cp.current_node, "node_a"); @@ -88,6 +103,8 @@ mod tests { assert_eq!(cp.logs.len(), 1); assert_eq!(cp.logs[0], "started"); assert!(cp.node_retries.is_empty()); + assert!(cp.node_outcomes.is_empty()); + assert!(cp.next_node_id.is_none()); } #[test] @@ -99,8 +116,18 @@ mod tests { ctx.set("goal", serde_json::json!("test")); ctx.append_log("log entry"); - let mut cp = Checkpoint::from_context(&ctx, "work", vec!["start".to_string()]); - cp.node_retries.insert("work".to_string(), 2); + let mut retries = HashMap::new(); + retries.insert("work".to_string(), 2u32); + let mut outcomes = HashMap::new(); + outcomes.insert("start".to_string(), Outcome::success()); + let cp = Checkpoint::from_context( + &ctx, + "work", + vec!["start".to_string()], + retries, + outcomes, + Some("next_step".to_string()), + ); cp.save(&path).unwrap(); let loaded = Checkpoint::load(&path).unwrap(); @@ -113,6 +140,8 @@ mod tests { Some(&serde_json::json!("test")) ); assert_eq!(loaded.logs, vec!["log entry"]); + assert_eq!(loaded.node_outcomes.get("start").map(|o| &o.status), Some(&crate::outcome::StageStatus::Success)); + assert_eq!(loaded.next_node_id.as_deref(), Some("next_step")); } #[test] @@ -134,7 +163,7 @@ mod tests { #[test] fn serialization_roundtrip() { let ctx = Context::new(); - let cp = Checkpoint::from_context(&ctx, "n1", vec![]); + let cp = Checkpoint::from_context(&ctx, "n1", vec![], HashMap::new(), HashMap::new(), None); let json = serde_json::to_string(&cp).unwrap(); let deserialized: Checkpoint = serde_json::from_str(&json).unwrap(); diff --git a/crates/attractor/src/engine.rs b/crates/attractor/src/engine.rs index 672a7e93f..2660344c1 100644 --- a/crates/attractor/src/engine.rs +++ b/crates/attractor/src/engine.rs @@ -97,9 +97,9 @@ impl std::fmt::Debug for RetryPolicy { } } -/// Default should_retry predicate: retries all errors. +/// Default should_retry predicate: retries transient errors only. fn default_should_retry() -> ShouldRetryFn { - std::sync::Arc::new(|_| true) + std::sync::Arc::new(|err| err.is_retryable()) } impl RetryPolicy { @@ -211,6 +211,43 @@ pub fn resolve_fidelity(incoming_edge: Option<&Edge>, node: &Node, graph: &Graph "compact".to_string() } +// --- Thread ID resolution (spec 5.4) --- + +/// Resolve the thread ID for a node, following the precedence (spec lines 1196-1204): +/// 1. Target node `thread_id` attribute +/// 2. Incoming edge `thread_id` attribute +/// 3. Graph-level default thread +/// 4. Derived class from enclosing subgraph (first class from the node's classes list) +/// 5. Fallback to previous node ID +#[must_use] +pub fn resolve_thread_id( + incoming_edge: Option<&Edge>, + node: &Node, + graph: &Graph, + previous_node_id: Option<&str>, +) -> Option { + // Step 1: Node thread_id + if let Some(tid) = node.thread_id() { + return Some(tid.to_string()); + } + // Step 2: Edge thread_id + if let Some(edge) = incoming_edge { + if let Some(tid) = edge.thread_id() { + return Some(tid.to_string()); + } + } + // Step 3: Graph-level default thread + if let Some(tid) = graph.default_thread() { + return Some(tid.to_string()); + } + // Step 4: Derived class from enclosing subgraph + if let Some(first_class) = node.classes.first() { + return Some(first_class.clone()); + } + // Step 5: Fallback to previous node ID + previous_node_id.map(String::from) +} + // --- Run directory helpers (spec 5.6) --- /// Write manifest.json at the start of a pipeline run. @@ -222,6 +259,7 @@ fn write_manifest(logs_root: &Path, graph: &Graph) { }; let manifest = serde_json::json!({ "pipeline_name": pipeline_name, + "goal": graph.goal(), "start_time": Utc::now().to_rfc3339(), "node_count": graph.nodes.len(), "edge_count": graph.edges.len(), @@ -446,6 +484,7 @@ impl PipelineEngine { } /// Execute a node handler with retry policy. + /// Returns `(outcome, attempts_used)` where `attempts_used` is the 1-indexed count. async fn execute_with_retry( &self, node: &Node, @@ -454,14 +493,31 @@ impl PipelineEngine { logs_root: &Path, policy: &RetryPolicy, stage_index: usize, - ) -> Result { + ) -> Result<(Outcome, u32)> { let handler = self.registry.resolve(node); + let node_timeout = node.timeout(); + for attempt in 1..=policy.max_attempts { // Gap #11: Panic safety -- catch panics from handler execution let result = { let future = handler.execute(node, context, graph, logs_root); - match AssertUnwindSafe(future).catch_unwind().await { + let panic_safe = AssertUnwindSafe(future).catch_unwind(); + // Gap #2: Timeout enforcement -- wrap with tokio::time::timeout + let timed_result = if let Some(duration) = node_timeout { + match tokio::time::timeout(duration, panic_safe).await { + Ok(inner) => inner, + Err(_elapsed) => { + Ok(Ok(Outcome::fail(format!( + "handler timed out after {}ms", + duration.as_millis() + )))) + } + } + } else { + panic_safe.await + }; + match timed_result { Ok(r) => r, Err(panic_payload) => { let msg = if let Some(s) = panic_payload.downcast_ref::<&str>() { @@ -497,7 +553,7 @@ impl PipelineEngine { tokio::time::sleep(delay).await; continue; } - return Ok(Outcome::fail(e.to_string())); + return Ok((Outcome::fail(e.to_string()), attempt)); } }; @@ -506,7 +562,7 @@ impl PipelineEngine { | StageStatus::PartialSuccess | StageStatus::Fail | StageStatus::Skipped => { - return Ok(outcome); + return Ok((outcome, attempt)); } StageStatus::Retry => { if attempt < policy.max_attempts { @@ -521,18 +577,23 @@ impl PipelineEngine { continue; } if node.allow_partial() { - return Ok(Outcome { - status: StageStatus::PartialSuccess, - notes: Some("retries exhausted, partial accepted".to_string()), - ..Outcome::success() - }); + return Ok(( + Outcome { + status: StageStatus::PartialSuccess, + notes: Some( + "retries exhausted, partial accepted".to_string(), + ), + ..Outcome::success() + }, + attempt, + )); } - return Ok(Outcome::fail("max retries exceeded")); + return Ok((Outcome::fail("max retries exceeded"), attempt)); } } } - Ok(Outcome::fail("max retries exceeded")) + Ok((Outcome::fail("max retries exceeded"), policy.max_attempts)) } /// Run the pipeline. Returns the final outcome. @@ -584,9 +645,13 @@ impl PipelineEngine { let context; let mut completed_nodes: Vec; let mut node_outcomes: HashMap = HashMap::new(); + let mut node_retries: HashMap = HashMap::new(); let mut stage_index: usize; let mut current_node_id: String; let mut incoming_edge: Option<&Edge> = None; + let mut previous_node_id: Option = None; + // Gap #6: Track whether fidelity should be degraded on the first resumed node + let mut degrade_fidelity_on_resume = false; if let Some(cp) = resume_checkpoint { // Restore context from checkpoint @@ -598,13 +663,27 @@ impl PipelineEngine { context.append_log(log_entry.clone()); } completed_nodes = cp.completed_nodes.clone(); + // Gap #5: Restore retry counters from checkpoint + node_retries = cp.node_retries.clone(); + // P1: Restore node outcomes for goal gate checks + node_outcomes = cp.node_outcomes.clone(); stage_index = completed_nodes.len(); - // Resume from the node after the checkpoint's current_node - let edges = graph.outgoing_edges(&cp.current_node); - if let Some(edge) = edges.first() { - current_node_id = edge.to.clone(); + // P1: Use stored next_node_id if available, otherwise fall back + if let Some(ref next_id) = cp.next_node_id { + current_node_id = next_id.clone(); } else { - current_node_id = cp.current_node.clone(); + let edges = graph.outgoing_edges(&cp.current_node); + if let Some(edge) = edges.first() { + current_node_id = edge.to.clone(); + } else { + current_node_id = cp.current_node.clone(); + } + } + // Gap #6: Check if the checkpointed node used full fidelity + if cp.context_values.get("internal.fidelity") + == Some(&serde_json::json!("full")) + { + degrade_fidelity_on_resume = true; } } else if let Some(start) = start_at { context = Context::new(); @@ -653,11 +732,32 @@ impl PipelineEngine { } // Resolve fidelity (spec 5.4) and store in context - let fidelity = resolve_fidelity(incoming_edge, node, graph); + let mut fidelity = resolve_fidelity(incoming_edge, node, graph); + // Gap #6: On the first node after resume, degrade full -> summary:high + if degrade_fidelity_on_resume && fidelity == "full" { + fidelity = "summary:high".to_string(); + } + degrade_fidelity_on_resume = false; context.set("internal.fidelity", serde_json::json!(&fidelity)); - // Thread context sharing: store thread association - if let Some(tid) = node.thread_id() { + // Preamble injection at execution time (spec 8.3): if fidelity is not "full", + // store a preamble in context for handlers to read + if fidelity != "full" { + context.set( + "current.preamble", + serde_json::json!(format!("[Context mode: {fidelity}]\n\n")), + ); + } else { + context.set("current.preamble", serde_json::json!("")); + } + + // Thread context sharing: resolve thread ID and store association + if let Some(tid) = resolve_thread_id( + incoming_edge, + node, + graph, + previous_node_id.as_deref(), + ) { context.set( format!("thread.{tid}.current_node"), serde_json::json!(&node.id), @@ -674,9 +774,25 @@ impl PipelineEngine { }); let stage_start = Instant::now(); - let outcome = self + let (mut outcome, attempts_used) = self .execute_with_retry(node, &context, graph, &config.logs_root, &retry_policy, stage_index) .await?; + // Gap #5: Track retry count per node + node_retries.insert(node.id.clone(), attempts_used); + context.set( + format!("internal.retry_count.{}", node.id), + serde_json::json!(attempts_used), + ); + + // Gap #1: Auto status -- when auto_status=true and outcome is non-success, + // override to success with auto-status note + if node.auto_status() && outcome.status != StageStatus::Success { + outcome = Outcome { + status: StageStatus::Success, + notes: Some("auto-status: handler completed without writing status".to_string()), + ..outcome + }; + } let stage_duration_ms = millis_u64(stage_start.elapsed()); @@ -705,6 +821,7 @@ impl PipelineEngine { // Step 3: Record completion completed_nodes.push(node.id.clone()); node_outcomes.insert(node.id.clone(), outcome.clone()); + previous_node_id = Some(node.id.clone()); stage_index += 1; // Step 4: Apply context updates from outcome @@ -714,11 +831,18 @@ impl PipelineEngine { context.set("preferred_label", serde_json::json!(pref)); } - // Step 5: Save checkpoint + // Step 5: Select next edge (done before checkpoint so we can store next_node_id) + let next_edge = select_edge(&node.id, &outcome, &context, graph); + let next_node_id_for_checkpoint = next_edge.map(|e| e.to.clone()); + + // Step 6: Save checkpoint with all state let checkpoint = Checkpoint::from_context( &context, &node.id, completed_nodes.clone(), + node_retries.clone(), + node_outcomes.clone(), + next_node_id_for_checkpoint, ); let checkpoint_path = config.logs_root.join("checkpoint.json"); if let Err(e) = checkpoint.save(&checkpoint_path) { @@ -729,8 +853,7 @@ impl PipelineEngine { }); } - // Step 6: Select next edge - let next_edge = select_edge(&node.id, &outcome, &context, graph); + // Step 7: Follow selected edge match next_edge { None => { // Gap #1: Failure routing -- when FAIL and no matching edge, @@ -790,6 +913,46 @@ mod tests { use super::*; use crate::graph::AttrValue; use crate::handler::start::StartHandler; + use crate::handler::Handler as HandlerTrait; + use async_trait::async_trait; + use std::time::Duration; + + // --- Test-only handlers --- + + /// Handler that always returns Fail. + struct AlwaysFailHandler; + + #[async_trait] + impl HandlerTrait for AlwaysFailHandler { + async fn execute( + &self, + _node: &Node, + _context: &Context, + _graph: &Graph, + _logs_root: &Path, + ) -> std::result::Result { + Ok(Outcome::fail("always fails")) + } + } + + /// Handler that sleeps for a configurable duration, then succeeds. + struct SlowHandler { + sleep_ms: u64, + } + + #[async_trait] + impl HandlerTrait for SlowHandler { + async fn execute( + &self, + _node: &Node, + _context: &Context, + _graph: &Graph, + _logs_root: &Path, + ) -> std::result::Result { + tokio::time::sleep(Duration::from_millis(self.sleep_ms)).await; + Ok(Outcome::success()) + } + } // --- BackoffConfig tests --- @@ -1527,6 +1690,7 @@ mod tests { let manifest: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); assert_eq!(manifest["pipeline_name"], "test_pipeline"); + assert_eq!(manifest["goal"], "Run tests"); assert!(manifest["start_time"].is_string()); assert!(manifest["node_count"].is_number()); assert!(manifest["edge_count"].is_number()); @@ -1567,4 +1731,364 @@ mod tests { Some(&serde_json::json!("compact")) ); } + + // --- resolve_thread_id tests --- + + #[test] + fn thread_id_from_node_attribute() { + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("main-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("main-thread".to_string()) + ); + } + + #[test] + fn thread_id_from_edge_attribute() { + let node = Node::new("work"); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("edge-thread".to_string()) + ); + } + + #[test] + fn thread_id_node_overrides_edge() { + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("node-thread".to_string()), + ); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("node-thread".to_string()) + ); + } + + #[test] + fn thread_id_from_graph_default_thread() { + let node = Node::new("work"); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("shared-thread".to_string()) + ); + } + + #[test] + fn thread_id_edge_overrides_graph_default() { + let node = Node::new("work"); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("edge-thread".to_string()) + ); + } + + #[test] + fn thread_id_graph_default_overrides_class() { + let mut node = Node::new("work"); + node.classes = vec!["planning".to_string()]; + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("shared-thread".to_string()) + ); + } + + #[test] + fn thread_id_from_node_class() { + let mut node = Node::new("work"); + node.classes = vec!["planning".to_string(), "review".to_string()]; + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("planning".to_string()) + ); + } + + #[test] + fn thread_id_fallback_to_previous_node() { + let node = Node::new("work"); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev_node")), + Some("prev_node".to_string()) + ); + } + + #[test] + fn thread_id_none_when_no_sources() { + let node = Node::new("start"); + let graph = Graph::new("test"); + assert_eq!(resolve_thread_id(None, &node, &graph, None), None); + } + + // --- default_should_retry tests --- + + #[test] + fn default_should_retry_retries_transient_errors() { + let should_retry = default_should_retry(); + assert!(should_retry(&AttractorError::Handler("timeout".to_string()))); + assert!(should_retry(&AttractorError::Engine("transient".to_string()))); + assert!(should_retry(&AttractorError::Io("connection reset".to_string()))); + } + + #[test] + fn default_should_retry_rejects_terminal_errors() { + let should_retry = default_should_retry(); + assert!(!should_retry(&AttractorError::Parse("bad syntax".to_string()))); + assert!(!should_retry(&AttractorError::Validation("invalid".to_string()))); + assert!(!should_retry(&AttractorError::Stylesheet("bad rule".to_string()))); + assert!(!should_retry(&AttractorError::Checkpoint("corrupt".to_string()))); + } + + // --- Gap #15: Manifest goal field test --- + + #[tokio::test] + async fn engine_manifest_includes_goal() { + let dir = tempfile::tempdir().unwrap(); + let g = simple_graph(); + let engine = PipelineEngine::new(make_registry(), EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + engine.run(&g, &config).await.unwrap(); + + let manifest_path = dir.path().join("manifest.json"); + let manifest: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); + assert_eq!(manifest["goal"], "Run tests"); + } + + #[tokio::test] + async fn engine_manifest_goal_empty_when_unset() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("no_goal"); + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + g.edges.push(Edge::new("start", "exit")); + + let engine = PipelineEngine::new(make_registry(), EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + engine.run(&g, &config).await.unwrap(); + + let manifest_path = dir.path().join("manifest.json"); + let manifest: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&manifest_path).unwrap()).unwrap(); + assert_eq!(manifest["goal"], ""); + } + + // --- Gap #1: Auto status tests --- + + #[tokio::test] + async fn engine_auto_status_overrides_fail_to_success() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("auto_status_test"); + + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + + let mut work = Node::new("work"); + work.attrs.insert("auto_status".to_string(), AttrValue::Boolean(true)); + work.attrs.insert("type".to_string(), AttrValue::String("always_fail".to_string())); + work.attrs.insert("max_retries".to_string(), AttrValue::Integer(0)); + g.nodes.insert("work".to_string(), work); + + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let mut registry = make_registry(); + registry.register("always_fail", Box::new(AlwaysFailHandler)); + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + let outcome = engine.run(&g, &config).await.unwrap(); + + assert_eq!(outcome.status, StageStatus::Success); + assert_eq!( + outcome.notes.as_deref(), + Some("auto-status: handler completed without writing status") + ); + } + + #[tokio::test] + async fn engine_auto_status_false_preserves_fail() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("no_auto_status_test"); + + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + + let mut work = Node::new("work"); + work.attrs.insert("type".to_string(), AttrValue::String("always_fail".to_string())); + work.attrs.insert("max_retries".to_string(), AttrValue::Integer(0)); + g.nodes.insert("work".to_string(), work); + + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + + g.edges.push(Edge::new("start", "work")); + let mut fail_edge = Edge::new("work", "exit"); + fail_edge.attrs.insert("condition".to_string(), AttrValue::String("outcome=fail".to_string())); + g.edges.push(fail_edge); + + let mut registry = make_registry(); + registry.register("always_fail", Box::new(AlwaysFailHandler)); + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + let result = engine.run(&g, &config).await; + + assert!(result.is_ok()); + let status_path = dir.path().join("work").join("status.json"); + let status: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&status_path).unwrap()).unwrap(); + assert_eq!(status["status"], "fail"); + } + + // --- Gap #2: Timeout enforcement tests --- + + #[tokio::test] + async fn engine_timeout_causes_fail_outcome() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("timeout_test"); + + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + + let mut work = Node::new("work"); + work.attrs.insert("timeout".to_string(), AttrValue::Duration(Duration::from_millis(50))); + work.attrs.insert("type".to_string(), AttrValue::String("slow".to_string())); + work.attrs.insert("max_retries".to_string(), AttrValue::Integer(0)); + g.nodes.insert("work".to_string(), work); + + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + + g.edges.push(Edge::new("start", "work")); + let mut fail_edge = Edge::new("work", "exit"); + fail_edge.attrs.insert("condition".to_string(), AttrValue::String("outcome=fail".to_string())); + g.edges.push(fail_edge); + + let mut registry = make_registry(); + registry.register("slow", Box::new(SlowHandler { sleep_ms: 500 })); + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + let result = engine.run(&g, &config).await; + assert!(result.is_ok()); + + let status_path = dir.path().join("work").join("status.json"); + let status: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&status_path).unwrap()).unwrap(); + assert_eq!(status["status"], "fail"); + } + + #[tokio::test] + async fn engine_no_timeout_completes_normally() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("no_timeout_test"); + + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + + let mut work = Node::new("work"); + work.attrs.insert("type".to_string(), AttrValue::String("slow".to_string())); + work.attrs.insert("max_retries".to_string(), AttrValue::Integer(0)); + g.nodes.insert("work".to_string(), work); + + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let mut registry = make_registry(); + registry.register("slow", Box::new(SlowHandler { sleep_ms: 10 })); + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + let outcome = engine.run(&g, &config).await.unwrap(); + assert_eq!(outcome.status, StageStatus::Success); + } + + #[tokio::test] + async fn engine_timeout_with_auto_status_returns_success() { + let dir = tempfile::tempdir().unwrap(); + let mut g = Graph::new("timeout_auto_status_test"); + + let mut start = Node::new("start"); + start.attrs.insert("shape".to_string(), AttrValue::String("Mdiamond".to_string())); + g.nodes.insert("start".to_string(), start); + + let mut work = Node::new("work"); + work.attrs.insert("timeout".to_string(), AttrValue::Duration(Duration::from_millis(50))); + work.attrs.insert("auto_status".to_string(), AttrValue::Boolean(true)); + work.attrs.insert("type".to_string(), AttrValue::String("slow".to_string())); + work.attrs.insert("max_retries".to_string(), AttrValue::Integer(0)); + g.nodes.insert("work".to_string(), work); + + let mut exit = Node::new("exit"); + exit.attrs.insert("shape".to_string(), AttrValue::String("Msquare".to_string())); + g.nodes.insert("exit".to_string(), exit); + + g.edges.push(Edge::new("start", "work")); + g.edges.push(Edge::new("work", "exit")); + + let mut registry = make_registry(); + registry.register("slow", Box::new(SlowHandler { sleep_ms: 500 })); + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { logs_root: dir.path().to_path_buf() }; + let outcome = engine.run(&g, &config).await.unwrap(); + + assert_eq!(outcome.status, StageStatus::Success); + assert_eq!( + outcome.notes.as_deref(), + Some("auto-status: handler completed without writing status") + ); + } } diff --git a/crates/attractor/src/error.rs b/crates/attractor/src/error.rs index b19e835d3..1c9850f1b 100644 --- a/crates/attractor/src/error.rs +++ b/crates/attractor/src/error.rs @@ -24,6 +24,24 @@ pub enum AttractorError { Io(String), } +impl AttractorError { + /// Whether this error category is retryable (transient) or terminal. + /// + /// Retryable: Handler (transient handler failures), Engine (could be transient), + /// Io (network/disk issues are often transient). + /// Terminal: Parse, Validation, Stylesheet (configuration errors), + /// Checkpoint (storage integrity). + #[must_use] + pub const fn is_retryable(&self) -> bool { + match self { + Self::Handler(_) | Self::Engine(_) | Self::Io(_) => true, + Self::Parse(_) | Self::Validation(_) | Self::Stylesheet(_) | Self::Checkpoint(_) => { + false + } + } + } +} + impl From for AttractorError { fn from(err: std::io::Error) -> Self { Self::Io(err.to_string()) @@ -88,4 +106,19 @@ mod tests { let err: Result = Err(AttractorError::Parse("bad".to_string())); assert!(err.is_err()); } + + #[test] + fn is_retryable_terminal_errors() { + assert!(!AttractorError::Parse("bad".to_string()).is_retryable()); + assert!(!AttractorError::Validation("bad".to_string()).is_retryable()); + assert!(!AttractorError::Stylesheet("bad".to_string()).is_retryable()); + assert!(!AttractorError::Checkpoint("bad".to_string()).is_retryable()); + } + + #[test] + fn is_retryable_transient_errors() { + assert!(AttractorError::Handler("timeout".to_string()).is_retryable()); + assert!(AttractorError::Engine("transient".to_string()).is_retryable()); + assert!(AttractorError::Io("connection reset".to_string()).is_retryable()); + } } diff --git a/crates/attractor/src/graph/types.rs b/crates/attractor/src/graph/types.rs index f6c255067..d5fb7b5fd 100644 --- a/crates/attractor/src/graph/types.rs +++ b/crates/attractor/src/graph/types.rs @@ -371,6 +371,13 @@ impl Graph { .get("default_fidelity") .and_then(AttrValue::as_str) } + + /// Graph-level `default_thread`. + pub fn default_thread(&self) -> Option<&str> { + self.attrs + .get("default_thread") + .and_then(AttrValue::as_str) + } } #[cfg(test)] diff --git a/crates/attractor/src/handler/codergen.rs b/crates/attractor/src/handler/codergen.rs index fe4cb6da1..a048be68d 100644 --- a/crates/attractor/src/handler/codergen.rs +++ b/crates/attractor/src/handler/codergen.rs @@ -98,7 +98,9 @@ impl Handler for CodergenHandler { // 3. Execute pre-hook (spec 9.7) if let Some(pre_hook) = resolve_hook(node, graph, "tool_hooks.pre") { if !run_hook(&pre_hook, &node.id) { - return Ok(Outcome::fail("pre-hook failed, skipping LLM call")); + let mut outcome = Outcome::skipped(); + outcome.notes = Some("pre-hook returned non-zero, tool call skipped".to_string()); + return Ok(outcome); } } @@ -299,9 +301,9 @@ mod tests { .execute(&node, &context, &graph, tmp.path()) .await .unwrap(); - assert_eq!(outcome.status, crate::outcome::StageStatus::Fail); + assert_eq!(outcome.status, crate::outcome::StageStatus::Skipped); assert!(outcome - .failure_reason + .notes .as_deref() .unwrap() .contains("pre-hook")); diff --git a/crates/attractor/src/handler/fan_in.rs b/crates/attractor/src/handler/fan_in.rs index a8dff44d3..444d8f6c0 100644 --- a/crates/attractor/src/handler/fan_in.rs +++ b/crates/attractor/src/handler/fan_in.rs @@ -29,7 +29,7 @@ impl Handler for FanInHandler { node: &Node, context: &Context, _graph: &Graph, - _logs_root: &Path, + logs_root: &Path, ) -> Result { let results = context.get("parallel.results"); let Some(results) = results else { @@ -39,11 +39,29 @@ impl Handler for FanInHandler { let prompt = node.prompt().filter(|p| !p.is_empty()); let best = if let (Some(prompt_text), Some(backend)) = (prompt, &self.backend) { - llm_evaluate(backend.as_ref(), prompt_text, &results, context).await? + llm_evaluate(backend.as_ref(), prompt_text, &results, context, logs_root, &node.id).await? } else { heuristic_select(&results) }; + // Check if all candidates failed — if so, return fail + let all_failed = if best.status == "fail" { + let empty_vec = vec![]; + let arr = results.as_array().unwrap_or(&empty_vec); + arr.iter().all(|v| { + v.get("status") + .and_then(|v| v.as_str()) + .unwrap_or("fail") + == "fail" + }) + } else { + false + }; + + if all_failed { + return Ok(Outcome::fail("all candidates failed")); + } + let mut outcome = Outcome::success(); outcome.context_updates.insert( "parallel.fan_in.best_id".to_string(), @@ -62,6 +80,7 @@ impl Handler for FanInHandler { struct Candidate { id: String, status: String, + score: f64, } fn status_rank(status: &str) -> u32 { @@ -81,6 +100,7 @@ fn heuristic_select(results: &serde_json::Value) -> Candidate { return Candidate { id: "unknown".to_string(), status: "fail".to_string(), + score: 0.0, }; } @@ -97,6 +117,10 @@ fn heuristic_select(results: &serde_json::Value) -> Candidate { .and_then(|v| v.as_str()) .unwrap_or("fail") .to_string(), + score: v + .get("score") + .and_then(|v| v.as_f64()) + .unwrap_or(0.0), }) .collect(); @@ -105,12 +129,18 @@ fn heuristic_select(results: &serde_json::Value) -> Candidate { if rank_cmp != std::cmp::Ordering::Equal { return rank_cmp; } + // Higher score is better, so reverse the comparison + let score_cmp = b.score.partial_cmp(&a.score).unwrap_or(std::cmp::Ordering::Equal); + if score_cmp != std::cmp::Ordering::Equal { + return score_cmp; + } a.id.cmp(&b.id) }); candidates.into_iter().next().unwrap_or_else(|| Candidate { id: "unknown".to_string(), status: "fail".to_string(), + score: 0.0, }) } @@ -120,6 +150,8 @@ async fn llm_evaluate( prompt: &str, results: &serde_json::Value, context: &Context, + logs_root: &Path, + node_id: &str, ) -> Result { let results_text = serde_json::to_string_pretty(results) .unwrap_or_else(|_| results.to_string()); @@ -129,6 +161,11 @@ async fn llm_evaluate( Respond with the ID of the best candidate." ); + // Write prompt to logs + let stage_dir = logs_root.join(node_id); + tokio::fs::create_dir_all(&stage_dir).await?; + tokio::fs::write(stage_dir.join("prompt.md"), &full_prompt).await?; + // Build a synthetic node for the backend call let eval_node = Node::new("fan_in_eval"); @@ -142,12 +179,19 @@ async fn llm_evaluate( .map(String::from) .or_else(|| outcome.notes.clone()) .unwrap_or_else(|| "unknown".to_string()); + let response_text = serde_json::to_string_pretty(&outcome) + .unwrap_or_else(|_| "{}".to_string()); + tokio::fs::write(stage_dir.join("response.md"), &response_text).await?; Ok(Candidate { id: best_id, status: outcome.status.to_string(), + score: 0.0, }) } Ok(CodergenResult::Text(text)) => { + // Write response to logs + tokio::fs::write(stage_dir.join("response.md"), &text).await?; + // The LLM responded with text; try to find a matching candidate ID let text = text.trim().to_string(); let empty_vec = vec![]; @@ -162,9 +206,14 @@ async fn llm_evaluate( .and_then(|v| v.as_str()) .unwrap_or("success") .to_string(); + let score = v + .get("score") + .and_then(|v| v.as_f64()) + .unwrap_or(0.0); return Ok(Candidate { id: id.to_string(), status, + score, }); } } @@ -294,6 +343,7 @@ mod tests { #[tokio::test] async fn fan_in_with_backend_llm_eval() { use crate::handler::codergen::CodergenBackend; + use tempfile::TempDir; struct MockBackend; @@ -325,10 +375,10 @@ mod tests { ]), ); let graph = Graph::new("test"); - let logs_root = Path::new("/tmp/test"); + let tmp = TempDir::new().unwrap(); let outcome = handler - .execute(&node, &context, &graph, logs_root) + .execute(&node, &context, &graph, tmp.path()) .await .unwrap(); assert_eq!(outcome.status, StageStatus::Success); @@ -337,5 +387,72 @@ mod tests { outcome.context_updates.get("parallel.fan_in.best_id"), Some(&serde_json::json!("branch_b")) ); + + // Verify prompt and response files were written + let prompt_path = tmp.path().join("fan_in").join("prompt.md"); + assert!(prompt_path.exists()); + let prompt_content = std::fs::read_to_string(&prompt_path).unwrap(); + assert!(prompt_content.contains("Pick the best branch")); + + let response_path = tmp.path().join("fan_in").join("response.md"); + assert!(response_path.exists()); + let response_content = std::fs::read_to_string(&response_path).unwrap(); + assert!(response_content.contains("branch_b")); + } + + #[tokio::test] + async fn fan_in_all_fail_returns_fail() { + let handler = FanInHandler::new(None); + let node = Node::new("fan_in"); + let context = Context::new(); + context.set( + "parallel.results", + serde_json::json!([ + {"id": "branch_a", "status": "fail"}, + {"id": "branch_b", "status": "fail"}, + {"id": "branch_c", "status": "fail"}, + ]), + ); + let graph = Graph::new("test"); + let logs_root = Path::new("/tmp/test"); + + let outcome = handler + .execute(&node, &context, &graph, logs_root) + .await + .unwrap(); + assert_eq!(outcome.status, StageStatus::Fail); + assert!(outcome + .failure_reason + .as_deref() + .unwrap() + .contains("all candidates failed")); + } + + #[tokio::test] + async fn fan_in_score_tiebreak() { + let handler = FanInHandler::new(None); + let node = Node::new("fan_in"); + let context = Context::new(); + context.set( + "parallel.results", + serde_json::json!([ + {"id": "branch_a", "status": "success", "score": 0.5}, + {"id": "branch_b", "status": "success", "score": 0.9}, + {"id": "branch_c", "status": "success", "score": 0.7}, + ]), + ); + let graph = Graph::new("test"); + let logs_root = Path::new("/tmp/test"); + + let outcome = handler + .execute(&node, &context, &graph, logs_root) + .await + .unwrap(); + assert_eq!(outcome.status, StageStatus::Success); + // branch_b has highest score + assert_eq!( + outcome.context_updates.get("parallel.fan_in.best_id"), + Some(&serde_json::json!("branch_b")) + ); } } diff --git a/crates/attractor/src/handler/manager_loop.rs b/crates/attractor/src/handler/manager_loop.rs index 94092db1d..9064a7967 100644 --- a/crates/attractor/src/handler/manager_loop.rs +++ b/crates/attractor/src/handler/manager_loop.rs @@ -1,5 +1,5 @@ use std::path::Path; -use std::time::Duration; +use std::time::{Duration, Instant}; use async_trait::async_trait; @@ -14,6 +14,16 @@ use super::Handler; /// Trait for observing child pipeline state during the manager loop. #[async_trait] pub trait ChildObserver: Send + Sync { + /// Launch the child pipeline. Called before the observation loop when child_autostart is true. + async fn launch_child( + &self, + _dotfile: &str, + _workdir: &str, + _context: &Context, + ) -> Result<(), AttractorError> { + Ok(()) + } + /// Ingest child telemetry into the context. async fn observe(&self, context: &Context) -> Result<(), AttractorError>; @@ -99,6 +109,39 @@ impl Handler for ManagerLoopHandler { let do_steer = actions_str.contains("steer"); let do_wait = actions_str.contains("wait"); + // Child autostart: launch child pipeline before the observation loop + let child_autostart = node + .attrs + .get("stack.child_autostart") + .and_then(|v| v.as_str()) + .unwrap_or("true"); + if child_autostart != "false" { + if let Some(ref observer) = self.observer { + let child_dotfile = node + .attrs + .get("stack.child_dotfile") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let child_workdir = node + .attrs + .get("stack.child_workdir") + .and_then(|v| v.as_str()) + .unwrap_or(""); + observer + .launch_child(child_dotfile, child_workdir, context) + .await?; + } + } + + // Steer cooldown tracking + let steer_cooldown = node + .attrs + .get("manager.steer_cooldown") + .and_then(|v| v.as_str()) + .map(parse_duration_str) + .unwrap_or(Duration::ZERO); + let mut last_steer_time: Option = None; + // Observation loop for cycle in 1..=max_cycles { // Observe @@ -108,10 +151,17 @@ impl Handler for ManagerLoopHandler { } } - // Steer + // Steer (with cooldown) if do_steer { - if let Some(ref observer) = self.observer { - observer.steer(context, node).await?; + let cooldown_elapsed = match last_steer_time { + Some(t) => t.elapsed() >= steer_cooldown, + None => true, + }; + if cooldown_elapsed { + if let Some(ref observer) = self.observer { + observer.steer(context, node).await?; + last_steer_time = Some(Instant::now()); + } } } diff --git a/crates/attractor/src/pipeline.rs b/crates/attractor/src/pipeline.rs index 086ed04be..ae3e28450 100644 --- a/crates/attractor/src/pipeline.rs +++ b/crates/attractor/src/pipeline.rs @@ -1,8 +1,6 @@ use crate::error::AttractorError; use crate::graph::Graph; -use crate::transform::{ - PreambleTransform, StylesheetApplicationTransform, Transform, VariableExpansionTransform, -}; +use crate::transform::{StylesheetApplicationTransform, Transform, VariableExpansionTransform}; use crate::validation::{self, Diagnostic}; /// Builder for configuring and executing a pipeline preparation. @@ -33,10 +31,9 @@ impl PipelineBuilder { pub fn prepare(&self, dot_source: &str) -> Result<(Graph, Vec), AttractorError> { let mut graph = crate::parser::parse(dot_source)?; - // Built-in transforms + // Built-in transforms (PreambleTransform moved to engine execution time) VariableExpansionTransform.apply(&mut graph); StylesheetApplicationTransform.apply(&mut graph); - PreambleTransform.apply(&mut graph); // Custom transforms for transform in &self.transforms { diff --git a/crates/attractor/src/stylesheet.rs b/crates/attractor/src/stylesheet.rs index f3f547f74..ac21615ea 100644 --- a/crates/attractor/src/stylesheet.rs +++ b/crates/attractor/src/stylesheet.rs @@ -6,11 +6,9 @@ use crate::graph::types::{AttrValue, Graph}; pub enum Selector { /// `*` -- matches all nodes, specificity 0. Universal, - /// bare word like `box` -- matches nodes whose shape equals that word, specificity 1. - Shape(String), - /// `.classname` -- matches nodes with that class, specificity 2. + /// `.classname` -- matches nodes with that class, specificity 1. Class(String), - /// `#nodeid` -- matches a specific node, specificity 3. + /// `#nodeid` -- matches a specific node, specificity 2. Id(String), } @@ -19,9 +17,8 @@ impl Selector { pub const fn specificity(&self) -> u8 { match self { Self::Universal => 0, - Self::Shape(_) => 1, - Self::Class(_) => 2, - Self::Id(_) => 3, + Self::Class(_) => 1, + Self::Id(_) => 2, } } } @@ -115,19 +112,10 @@ fn parse_selector(remaining: &mut &str) -> Result { *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(AttractorError::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)) + Err(AttractorError::Stylesheet(format!( + "expected selector ('*', '#id', or '.class'), got: {:?}", + &remaining[..remaining.len().min(20)] + ))) } } @@ -201,7 +189,6 @@ pub fn apply_stylesheet(stylesheet: &Stylesheet, graph: &mut Graph) { let node = &graph.nodes[node_id.as_str()]; let matches = match &rule.selector { Selector::Universal => true, - Selector::Shape(shape) => node.shape() == shape.as_str(), Selector::Class(cls) => node.classes.contains(cls), Selector::Id(id) => node_id == id, }; @@ -386,9 +373,8 @@ 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); + assert_eq!(Selector::Class("x".into()).specificity(), 1); + assert_eq!(Selector::Id("x".into()).specificity(), 2); } #[test] @@ -442,76 +428,36 @@ mod tests { } #[test] - fn parse_shape_selector() { - let ss = parse_stylesheet("box { llm_model: opus; }").unwrap(); - assert_eq!(ss.rules.len(), 1); - assert_eq!(ss.rules[0].selector, Selector::Shape("box".into())); + fn parse_bare_word_selector_is_error() { + let result = parse_stylesheet("box { llm_model: opus; }"); + assert!(result.is_err()); } #[test] - fn apply_shape_selector_matches_by_shape() { - let ss = parse_stylesheet("box { llm_model: opus; }").unwrap(); - let mut graph = Graph::new("test"); - - // Default shape is "box" - let node_a = Node::new("a"); - graph.nodes.insert("a".into(), node_a); - - let mut node_b = Node::new("b"); - node_b - .attrs - .insert("shape".into(), AttrValue::String("diamond".into())); - graph.nodes.insert("b".into(), node_b); - - apply_stylesheet(&ss, &mut graph); - - assert_eq!( - graph.nodes["a"].attrs.get("llm_model"), - Some(&AttrValue::String("opus".into())) - ); - // diamond shape should not match "box" selector - assert_eq!(graph.nodes["b"].attrs.get("llm_model"), None); - } - - #[test] - fn shape_selector_specificity_between_universal_and_class() { + fn class_overrides_universal_specificity() { let ss = parse_stylesheet( - "* { llm_model: sonnet; } box { llm_model: opus; } .special { llm_model: gpt; }", + "* { llm_model: sonnet; } .special { llm_model: gpt; }", ) .unwrap(); let mut graph = Graph::new("test"); - // Node with default shape "box" and class "special" let mut node_a = Node::new("a"); node_a.classes.push("special".into()); graph.nodes.insert("a".into(), node_a); - // Node with default shape "box" and no class let node_b = Node::new("b"); graph.nodes.insert("b".into(), node_b); - // Node with shape "diamond" and no class - let mut node_c = Node::new("c"); - node_c - .attrs - .insert("shape".into(), AttrValue::String("diamond".into())); - graph.nodes.insert("c".into(), node_c); - apply_stylesheet(&ss, &mut graph); - // .special (specificity 2) overrides box (specificity 1) + // .special (specificity 1) overrides * (specificity 0) assert_eq!( graph.nodes["a"].attrs.get("llm_model"), Some(&AttrValue::String("gpt".into())) ); - // box (specificity 1) overrides * (specificity 0) + // No class, gets universal assert_eq!( graph.nodes["b"].attrs.get("llm_model"), - Some(&AttrValue::String("opus".into())) - ); - // diamond doesn't match "box", so gets universal - assert_eq!( - graph.nodes["c"].attrs.get("llm_model"), Some(&AttrValue::String("sonnet".into())) ); } diff --git a/crates/attractor/src/validation/rules.rs b/crates/attractor/src/validation/rules.rs index cd13ea44d..a902ebc48 100644 --- a/crates/attractor/src/validation/rules.rs +++ b/crates/attractor/src/validation/rules.rs @@ -4,8 +4,8 @@ use crate::graph::{AttrValue, Graph}; use super::{Diagnostic, LintRule, Severity}; -/// Returns all 14 built-in lint rules. -#[must_use] +/// Returns all 15 built-in lint rules. +#[must_use] pub fn built_in_rules() -> Vec> { vec![ Box::new(StartNodeRule), @@ -22,6 +22,7 @@ pub fn built_in_rules() -> Vec> { Box::new(GoalGateHasRetryRule), Box::new(PromptOnLlmNodesRule), Box::new(FreeformEdgeCountRule), + Box::new(DirectionValidRule), ] } @@ -329,21 +330,17 @@ impl LintRule for StylesheetSyntaxRule { if stylesheet.is_empty() { return Vec::new(); } - let open_count = stylesheet.chars().filter(|c| *c == '{').count(); - let close_count = stylesheet.chars().filter(|c| *c == '}').count(); - if open_count != close_count { - return vec![Diagnostic { + match crate::stylesheet::parse_stylesheet(stylesheet) { + Ok(_) => Vec::new(), + Err(e) => vec![Diagnostic { rule: self.name().to_string(), severity: Severity::Error, - message: format!( - "Model stylesheet has unbalanced braces ({open_count} open, {close_count} close)" - ), + message: format!("Model stylesheet parse error: {e}"), node_id: None, edge: None, - fix: Some("Balance the curly braces in model_stylesheet".to_string()), - }]; + fix: Some("Fix the model_stylesheet syntax".to_string()), + }], } - Vec::new() } } @@ -662,6 +659,37 @@ impl LintRule for FreeformEdgeCountRule { } } +// --- Rule 15: direction_valid (WARNING) --- + +struct DirectionValidRule; + +const VALID_DIRECTIONS: &[&str] = &["TB", "LR", "BT", "RL"]; + +impl LintRule for DirectionValidRule { + fn name(&self) -> &'static str { + "direction_valid" + } + + fn apply(&self, graph: &Graph) -> Vec { + let Some(rankdir) = graph.attrs.get("rankdir").and_then(AttrValue::as_str) else { + return Vec::new(); + }; + if VALID_DIRECTIONS.contains(&rankdir) { + return Vec::new(); + } + vec![Diagnostic { + 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(", "))), + }] + } +} + #[cfg(test)] mod tests { use super::*; @@ -1123,8 +1151,75 @@ mod tests { // built_in_rules tests #[test] - fn built_in_rules_returns_14_rules() { + fn built_in_rules_returns_15_rules() { let rules = built_in_rules(); - assert_eq!(rules.len(), 14); + assert_eq!(rules.len(), 15); + } + + // direction_valid rule tests + + #[test] + fn direction_valid_rule_no_rankdir() { + let g = minimal_graph(); + let rule = DirectionValidRule; + let d = rule.apply(&g); + assert!(d.is_empty()); + } + + #[test] + fn direction_valid_rule_valid_directions() { + let rule = DirectionValidRule; + let mut g = minimal_graph(); + g.attrs.insert( + "rankdir".to_string(), + AttrValue::String("LR".to_string()), + ); + assert!(rule.apply(&g).is_empty()); + + g.attrs.insert( + "rankdir".to_string(), + AttrValue::String("TB".to_string()), + ); + assert!(rule.apply(&g).is_empty()); + + g.attrs.insert( + "rankdir".to_string(), + AttrValue::String("BT".to_string()), + ); + assert!(rule.apply(&g).is_empty()); + + g.attrs.insert( + "rankdir".to_string(), + AttrValue::String("RL".to_string()), + ); + assert!(rule.apply(&g).is_empty()); + } + + #[test] + fn direction_valid_rule_invalid_direction() { + let mut g = minimal_graph(); + g.attrs.insert( + "rankdir".to_string(), + AttrValue::String("XY".to_string()), + ); + let rule = DirectionValidRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Warning); + } + + // stylesheet_syntax with full parse tests + + #[test] + fn stylesheet_syntax_rule_malformed_selector() { + let mut g = minimal_graph(); + g.attrs.insert( + "model_stylesheet".to_string(), + AttrValue::String("* { garbage garbage }".to_string()), + ); + let rule = StylesheetSyntaxRule; + let d = rule.apply(&g); + assert_eq!(d.len(), 1); + assert_eq!(d[0].severity, Severity::Error); } } diff --git a/crates/attractor/tests/integration.rs b/crates/attractor/tests/integration.rs index 962473751..94538f4e9 100644 --- a/crates/attractor/tests/integration.rs +++ b/crates/attractor/tests/integration.rs @@ -986,15 +986,19 @@ fn checkpoint_save_and_resume_roundtrip() { ctx.append_log("started"); ctx.append_log("step_1 completed"); - let mut checkpoint = Checkpoint::from_context( + let mut retries = std::collections::HashMap::new(); + retries.insert("step_1".to_string(), 1u32); + let checkpoint = Checkpoint::from_context( &ctx, "step_2", vec![ "start".to_string(), "step_1".to_string(), ], + retries, + std::collections::HashMap::new(), + None, ); - checkpoint.node_retries.insert("step_1".to_string(), 1); checkpoint.save(&path).expect("save should succeed"); @@ -1178,3 +1182,266 @@ async fn smoke_test_with_mock_codergen_backend() { .expect("plan prompt should exist"); assert_eq!(plan_prompt, "Plan to achieve: Build and validate"); } + +// --------------------------------------------------------------------------- +// 12. Parallel fan-out / fan-in integration test (Gap #14) +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn end_to_end_parallel_fan_out_fan_in() { + use attractor::handler::fan_in::FanInHandler; + use attractor::handler::parallel::ParallelHandler; + use std::sync::Arc; + + let input = r#"digraph parallel_test { + start [shape=Mdiamond] + fan_out [shape=component] + branch_a [shape=box, prompt="Branch A work"] + branch_b [shape=box, prompt="Branch B work"] + fan_in_node [shape=tripleoctagon] + done [shape=Msquare] + + start -> fan_out + fan_out -> branch_a + fan_out -> branch_b + branch_a -> fan_in_node + branch_b -> fan_in_node + fan_in_node -> done + }"#; + + let graph = parse(input).expect("parse should succeed"); + validate_or_raise(&graph, &[]).expect("validation should pass"); + + let dir = tempfile::tempdir().unwrap(); + + let mut registry = HandlerRegistry::new( + Box::new(CodergenHandler::new(Some(Box::new(MockCodergenBackend)))), + ); + registry.register("start", Box::new(StartHandler)); + registry.register("exit", Box::new(ExitHandler)); + registry.register( + "codergen", + Box::new(CodergenHandler::new(Some(Box::new(MockCodergenBackend)))), + ); + + let registry = Arc::new(registry); + + let emitter = Arc::new(EventEmitter::new()); + let parallel_handler = ParallelHandler::new(Arc::clone(®istry), Arc::clone(&emitter)); + let fan_in_handler = FanInHandler::new(Some(Box::new(MockCodergenBackend))); + + // Build a new registry with parallel and fan_in registered + let mut full_registry = HandlerRegistry::new( + Box::new(CodergenHandler::new(Some(Box::new(MockCodergenBackend)))), + ); + full_registry.register("start", Box::new(StartHandler)); + full_registry.register("exit", Box::new(ExitHandler)); + full_registry.register( + "codergen", + Box::new(CodergenHandler::new(Some(Box::new(MockCodergenBackend)))), + ); + full_registry.register("parallel", Box::new(parallel_handler)); + full_registry.register("parallel.fan_in", Box::new(fan_in_handler)); + + let engine = PipelineEngine::new(full_registry, EventEmitter::new()); + let config = RunConfig { + logs_root: dir.path().to_path_buf(), + }; + + let outcome = engine + .run(&graph, &config) + .await + .expect("parallel pipeline should succeed"); + assert_eq!(outcome.status, StageStatus::Success); + + let checkpoint = Checkpoint::load(&dir.path().join("checkpoint.json")).unwrap(); + + // The parallel node (fan_out) and fan_in_node should be in completed_nodes. + // Branch nodes run inside the parallel handler, so they are not recorded + // individually by the engine -- but fan_out and fan_in_node are top-level. + assert!( + checkpoint + .completed_nodes + .contains(&"fan_out".to_string()), + "fan_out should have been executed" + ); + assert!( + checkpoint + .completed_nodes + .contains(&"fan_in_node".to_string()), + "fan_in_node should have been executed" + ); + + // Verify parallel.results was populated (both branches ran) + let parallel_results = checkpoint + .context_values + .get("parallel.results") + .expect("parallel.results should be in context"); + let results_arr = parallel_results.as_array().expect("should be an array"); + assert_eq!(results_arr.len(), 2, "should have 2 branch results"); +} + +// --------------------------------------------------------------------------- +// 13. Resume from checkpoint (P1) +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn resume_from_checkpoint_completes_pipeline() { + // Build a pipeline: start -> step_a -> step_b -> exit + // Create a checkpoint mid-pipeline (after step_a) and verify + // run_from_checkpoint completes from step_b onward. + + let mut graph = Graph::new("ResumeTest"); + graph.attrs.insert( + "goal".to_string(), + AttrValue::String("Test resume".to_string()), + ); + + let mut start = Node::new("start"); + start.attrs.insert( + "shape".to_string(), + AttrValue::String("Mdiamond".to_string()), + ); + graph.nodes.insert("start".to_string(), start); + + let mut exit = Node::new("exit"); + exit.attrs.insert( + "shape".to_string(), + AttrValue::String("Msquare".to_string()), + ); + graph.nodes.insert("exit".to_string(), exit); + + let step_a = Node::new("step_a"); + graph.nodes.insert("step_a".to_string(), step_a); + + let step_b = Node::new("step_b"); + graph.nodes.insert("step_b".to_string(), step_b); + + graph.edges.push(Edge::new("start", "step_a")); + graph.edges.push(Edge::new("step_a", "step_b")); + graph.edges.push(Edge::new("step_b", "exit")); + + // Simulate a checkpoint saved after step_a completed. + // The checkpoint records step_a as current_node with next_node_id = step_b. + let ctx = Context::new(); + ctx.set("graph.goal", serde_json::json!("Test resume")); + ctx.set("outcome", serde_json::json!("success")); + + let mut outcomes = std::collections::HashMap::new(); + outcomes.insert("start".to_string(), Outcome::success()); + outcomes.insert("step_a".to_string(), Outcome::success()); + + let checkpoint = Checkpoint::from_context( + &ctx, + "step_a", + vec!["start".to_string(), "step_a".to_string()], + std::collections::HashMap::new(), + outcomes, + Some("step_b".to_string()), + ); + + let dir = tempfile::tempdir().unwrap(); + let mut registry = HandlerRegistry::new(Box::new(StartHandler)); + registry.register("start", Box::new(StartHandler)); + registry.register("exit", Box::new(ExitHandler)); + + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { + logs_root: dir.path().to_path_buf(), + }; + + let outcome = engine + .run_from_checkpoint(&graph, &config, &checkpoint) + .await + .expect("resume should succeed"); + assert_eq!(outcome.status, StageStatus::Success); + + // Verify checkpoint written after resume contains step_b + let final_cp = Checkpoint::load(&dir.path().join("checkpoint.json")).unwrap(); + assert!( + final_cp.completed_nodes.contains(&"step_b".to_string()), + "step_b should have been executed after resume" + ); + // step_a should also be present (carried over from the checkpoint) + assert!( + final_cp.completed_nodes.contains(&"step_a".to_string()), + "step_a should be preserved from checkpoint" + ); + // start should also be present + assert!( + final_cp.completed_nodes.contains(&"start".to_string()), + "start should be preserved from checkpoint" + ); +} + +#[tokio::test] +async fn resume_from_checkpoint_preserves_goal_gate_outcomes() { + // Build: start -> gated_work (goal_gate=true) -> step_b -> exit + // Checkpoint after gated_work (success), resume at step_b. + // At exit, goal gate should pass because outcomes are restored. + + let mut graph = Graph::new("ResumeGoalGateTest"); + + let mut start = Node::new("start"); + start.attrs.insert( + "shape".to_string(), + AttrValue::String("Mdiamond".to_string()), + ); + graph.nodes.insert("start".to_string(), start); + + let mut exit = Node::new("exit"); + exit.attrs.insert( + "shape".to_string(), + AttrValue::String("Msquare".to_string()), + ); + graph.nodes.insert("exit".to_string(), exit); + + let mut gated_work = Node::new("gated_work"); + gated_work.attrs.insert( + "goal_gate".to_string(), + AttrValue::Boolean(true), + ); + graph.nodes.insert("gated_work".to_string(), gated_work); + + let step_b = Node::new("step_b"); + graph.nodes.insert("step_b".to_string(), step_b); + + graph.edges.push(Edge::new("start", "gated_work")); + graph.edges.push(Edge::new("gated_work", "step_b")); + graph.edges.push(Edge::new("step_b", "exit")); + + // Checkpoint: gated_work completed with success, next is step_b + let ctx = Context::new(); + ctx.set("outcome", serde_json::json!("success")); + + let mut outcomes = std::collections::HashMap::new(); + outcomes.insert("start".to_string(), Outcome::success()); + outcomes.insert("gated_work".to_string(), Outcome::success()); + + let checkpoint = Checkpoint::from_context( + &ctx, + "gated_work", + vec!["start".to_string(), "gated_work".to_string()], + std::collections::HashMap::new(), + outcomes, + Some("step_b".to_string()), + ); + + let dir = tempfile::tempdir().unwrap(); + let mut registry = HandlerRegistry::new(Box::new(StartHandler)); + registry.register("start", Box::new(StartHandler)); + registry.register("exit", Box::new(ExitHandler)); + + let engine = PipelineEngine::new(registry, EventEmitter::new()); + let config = RunConfig { + logs_root: dir.path().to_path_buf(), + }; + + // This should succeed because goal gate for gated_work is satisfied + // via restored outcomes + let outcome = engine + .run_from_checkpoint(&graph, &config, &checkpoint) + .await + .expect("resume with goal gate should succeed"); + assert_eq!(outcome.status, StageStatus::Success); +} diff --git a/docs/agent/reviews/confirmed-gaps.md b/docs/agent/reviews/confirmed-gaps.md new file mode 100644 index 000000000..0258c898a --- /dev/null +++ b/docs/agent/reviews/confirmed-gaps.md @@ -0,0 +1,185 @@ +# Attractor Spec: Confirmed Gaps Report + +**Date:** 2026-02-21 +**Method:** 3 parallel agents investigated 19 claimed gaps, reading source code and citing exact file:line evidence. + +**Result: 16 CONFIRMED / 2 REFUTED / 1 PARTIAL** + +--- + +## Hard Gaps (3/3 confirmed) + +### 1. Auto Status — engine never checks `auto_status` attribute + +**CONFIRMED** + +The `auto_status()` accessor exists at `graph/types.rs:193-194` but is never referenced in `engine.rs` or any handler. A grep for `auto_status` across all source returns only the accessor definition and its unit test — zero engine references. The spec (line 162, Appendix C line 2111) says: when `auto_status=true` and no `status.json` was written by the handler, the engine should synthesize `{"outcome": "success", "notes": "auto-status: handler completed without writing status"}`. This does not happen. + +### 2. Timeout Enforcement — no deadline around handler execution + +**CONFIRMED** + +`tokio::time::timeout` appears in exactly two places: `handler/tool.rs:67` (subprocess timeout) and `interviewer/mod.rs:142` (human input timeout). The engine's `execute_with_retry` at `engine.rs:449-536` wraps handler execution in `catch_unwind` for panic safety (line 464) but has **no** `tokio::time::timeout` wrapper. A grep for `timeout` in `engine.rs` returns zero matches. All non-tool, non-interviewer handlers (codergen, manager_loop, parallel, etc.) can run indefinitely. + +### 3. Manager Loop `child_autostart` — not implemented + +**CONFIRMED** + +The spec (lines 961-963) says: `IF node.attrs.get("stack.child_autostart", "true") == "true": start_child_pipeline(child_dotfile)`. A codebase-wide grep for `child_autostart`, `start_child_pipeline`, and `child_dotfile` returns matches only in the spec itself and the review document. `handler/manager_loop.rs` goes directly into its observation loop without any auto-start logic. The `ChildObserver` trait (lines 16-22) provides `observe` and `steer` but no launch capability. Sub-gap: `steer_cooldown_elapsed()` is also missing — zero matches for `steer_cooldown` anywhere. + +--- + +## Minor Gaps (13/16 confirmed, 2 refuted, 1 partial) + +### 4. Thread ID Resolution — only step 1 of 5 + +**CONFIRMED** + +The spec (lines 1196-1206) defines 5-step thread resolution for `full` fidelity. `engine.rs:660-664` only handles step 1 (node `thread_id`). Missing: +- Step 2: Edge `thread_id` — accessor exists at `graph/types.rs:262-264` but engine never reads it +- Step 3: Graph-level default thread — no `default_thread` accessor (zero grep matches) +- Step 4: Derived class from enclosing subgraph — engine never uses `classes` for thread resolution +- Step 5: Fallback to previous node ID — not implemented + +### 5. Checkpoint Resume — retry counters not restored + +**CONFIRMED** + +`Checkpoint.node_retries` field exists at `checkpoint.rs:17` but `Checkpoint::from_context()` at line 28 always initializes it as `HashMap::new()`. A grep for `node_retries` in `engine.rs` returns zero matches. The field is never populated during saves and never read during resume. The spec's `reset_retry_counter`/`increment_retry_counter` functions (lines 499-504) do not exist. + +### 6. Checkpoint Resume — fidelity degradation missing + +**CONFIRMED** + +The spec (line 1165) says: "If the previous node used `full` fidelity, degrade to `summary:high` for the first resumed node." A grep for `fidelity.*degrad|summary.high.*resume` across all sources returns zero matches. The resume code at `engine.rs:591-608` performs no fidelity degradation. + +### 7. Retry Policy `should_retry` — too coarse + +**CONFIRMED** + +`default_should_retry()` at `engine.rs:100-103`: `Arc::new(|_| true)` — retries ALL errors. The spec (line 556) requires: retry on 429/5xx/network errors, no retry on 401/403/400/validation/config errors. `build_retry_policy` at lines 178-189 always uses the default. `AttractorError` has no `is_retryable()` method. + +### 8. Direction type not validated + +**CONFIRMED** + +The BNF defines `Direction ::= 'TB' | 'LR' | 'BT' | 'RL'` (spec line 107). `grammar.rs:57-66` accepts any `identifier = value` as a graph attr declaration. `semantic.rs:178-179` inserts without validation. None of the 14 validation rules check direction values. `rankdir=XY` would be accepted silently. + +### 9. Stylesheet `stylesheet_syntax` lint — brace balance only + +**CONFIRMED** + +`validation/rules.rs:320-348`: the rule counts `{` and `}` characters and errors only if counts differ. It does not call `parse_stylesheet()` from `stylesheet.rs:54-85` which performs full parsing (selector validation, declaration parsing, proper error messages). A stylesheet like `* { garbage garbage }` passes the lint. + +### 10. Stylesheet — undocumented Shape selector + +**CONFIRMED** + +The spec grammar (line 1497) defines: `Selector ::= '*' | '#' Identifier | '.' ClassName` — three types. `stylesheet.rs:6-15` defines four: `Universal`, `Shape(String)`, `Class(String)`, `Id(String)`. The `Shape` selector is parsed at lines 117-131 (bare-word fallback). This shifts specificity: spec says `*`=0, `.class`=1, `#id`=2; impl has `*`=0, `Shape`=1, `.class`=2, `#id`=3. Relative ordering preserved but absolute values differ. Test at line 446 confirms `box { llm_model: opus; }` produces `Selector::Shape("box")`. + +### 11. Fan-in — no score sort, all-fail returns SUCCESS + +**CONFIRMED** (both sub-claims) + +The spec (line 919) sorts by `(outcome_rank, -c.score, c.id)`. `fan_in.rs:103-109` sorts by `(status_rank, id)` — no `score` field on `Candidate` (lines 62-65), zero grep matches for "score". For all-fail: the spec (line 923) says "Only when all candidates fail does fan-in return FAIL." `fan_in.rs:41-58` always builds `Outcome::success()` at line 47 regardless of whether all candidates failed. + +### 12. Preamble transform — applied at parse time, not execution time + +**CONFIRMED** + +The spec (line 1602): "Applied at execution time (not at parse time) since it depends on runtime state." `pipeline.rs:39` calls `PreambleTransform.apply(&mut graph)` in `prepare()` alongside other parse-time transforms. `transform.rs:28-49` reads `fidelity` from static node attributes but has no access to runtime state (e.g., edge-level fidelity overrides resolved at execution time in `engine.rs:656`). If a node has `fidelity="full"` but an incoming edge has `fidelity="truncate"`, the preamble would be incorrectly missing. + +### 13. Pre-hook non-zero — returns fail instead of skip + +**CONFIRMED** + +The spec (line 1693): "non-zero means skip the tool call." `codergen.rs:98-103` returns `Outcome::fail("pre-hook failed, skipping LLM call")` — `StageStatus::Fail` is semantically stronger than skipping. Test at line 302 confirms the outcome status is `Fail`. + +### 14. No parallel fan-out/fan-in integration test + +**CONFIRMED** + +`tests/integration.rs` (1181 lines) contains 11+ test functions covering linear, branching, human gate, goal gate, retry, stylesheet, checkpoint, and smoke test pipelines. None involve `component` (parallel) or `tripleoctagon` (fan-in) shape nodes. A grep for `parallel|fan_in|fan_out` in `crates/attractor/tests/` returns zero matches. + +### 15. Manifest missing `goal` field + +**CONFIRMED** + +The spec (line 1260): "manifest.json -- Pipeline metadata (name, goal, start time)." `engine.rs:217-233` `write_manifest()` writes `pipeline_name`, `start_time`, `node_count`, `edge_count` — no `goal` field despite `graph.goal()` being available. Integration test at lines 1527-1532 confirms the four fields without `goal`. + +### 16. Error categories — no retryable/terminal classification + +**CONFIRMED** + +The spec (Appendix D, lines 2115-2123) defines Retryable, Terminal, and Pipeline error categories. `error.rs:1-33` has 7 variants (`Parse`, `Validation`, `Engine`, `Handler`, `Checkpoint`, `Stylesheet`, `Io`) with no retryability metadata, no `is_retryable()` method, no `ErrorCategory` enum. + +### 17. Spec self-contradicts on `default_max_retry` + +**CONFIRMED** + +Spec line 138 (Section 2.5 table): `default_max_retry | Integer | 50`. Spec line 481 (Section 3.5): "Built-in default: 0 (no retries)." Implementation follows Section 2 at `graph/types.rs:349-353` with `.unwrap_or(50)`. Test at `engine.rs:931-936` confirms 51 max attempts (50 retries + 1 initial). + +--- + +## Refuted Claims (2) + +### R1. Missing variable handling — claimed as GAP + +**REFUTED** — Correctly implemented. + +`condition.rs:87-110` `resolve_key()` returns `String::new()` for all missing keys (lines 105, 109). Tests at lines 230-239 (`missing_key_compares_as_empty`) and 291-295 (`bare_key_falsy_when_empty`) confirm spec-compliant behavior. The spec (line 1724) says: "Missing keys compare as empty strings" — exactly what the implementation does. + +### R2. Status File Contract — claimed "no implementation found" + +**REFUTED** — Implemented at two layers. + +**Engine-level:** `engine.rs:236-248` `write_node_status()` writes `{node_id}/status.json` with `status`, `notes`, `failure_reason`, `timestamp`. Called at line 702-703 for every node. +**Handler-level:** `codergen.rs:111,150` writes richer `status.json` (full `Outcome` serialized). +**Tests:** `engine.rs:1536-1551` and `integration.rs:189-198` verify status file existence and contents. + +Minor sub-gap: the engine-level schema uses key `"status"` while the spec's Appendix C uses `"outcome"`, and the engine-level file omits `preferred_next_label`, `suggested_next_ids`, `context_updates`. + +--- + +## Partial (1) + +### P1. Checkpoint Resume — functional but incomplete + +**PARTIAL** + +`engine.rs:554-561` `run_from_checkpoint()` exists and is callable. Resume logic at lines 591-608 restores context, logs, completed_nodes, and continues from the next node. + +**What works:** Basic resume for simple linear pipelines. + +**What's missing:** +1. `node_retries` ignored during resume (see gap 5) +2. `node_outcomes` not restored — initialized as empty HashMap at line 586, causing goal gate checks to miss pre-checkpoint outcomes +3. Edge selection during resume picks first outgoing edge (line 603-608), ignoring conditions — wrong successor for conditional graphs +4. No integration test calls `run_from_checkpoint` — only `Checkpoint::save`/`load` is tested + +--- + +## Summary Table + +| # | Gap | Verdict | Severity | +|---|-----|---------|----------| +| 1 | Auto status not enforced | CONFIRMED | Hard | +| 2 | Timeout not enforced in engine | CONFIRMED | Hard | +| 3 | Manager loop child_autostart missing | CONFIRMED | Hard | +| 4 | Thread ID resolution 1/5 steps | CONFIRMED | Moderate | +| 5 | Checkpoint retry counters not persisted | CONFIRMED | Moderate | +| 6 | Checkpoint fidelity degradation missing | CONFIRMED | Moderate | +| 7 | should_retry retries all errors | CONFIRMED | Moderate | +| 8 | Direction values not validated | CONFIRMED | Low | +| 9 | Stylesheet lint brace-balance only | CONFIRMED | Low | +| 10 | Undocumented Shape selector | CONFIRMED | Low | +| 11 | Fan-in: no score sort, all-fail=SUCCESS | CONFIRMED | Moderate | +| 12 | Preamble at parse time not runtime | CONFIRMED | Moderate | +| 13 | Pre-hook fail instead of skip | CONFIRMED | Low | +| 14 | No parallel integration test | CONFIRMED | Low | +| 15 | Manifest missing goal field | CONFIRMED | Low | +| 16 | No error retryable/terminal classification | CONFIRMED | Moderate | +| 17 | Spec contradicts itself on default_max_retry | CONFIRMED | Low (spec bug) | +| P1 | Checkpoint resume incomplete | PARTIAL | Moderate | +| R1 | Missing variable handling | REFUTED | — | +| R2 | Status file contract | REFUTED | — | diff --git a/docs/agent/reviews/spec-compliance-full-review.md b/docs/agent/reviews/spec-compliance-full-review.md new file mode 100644 index 000000000..f4bdd2788 --- /dev/null +++ b/docs/agent/reviews/spec-compliance-full-review.md @@ -0,0 +1,224 @@ +# Attractor Spec Compliance Review + +**Date**: 2026-02-21 +**Spec**: `docs/specs/attractor-spec.md` +**Implementation**: `crates/attractor/src/` + +--- + +## Section 1: Overview and Goals + +| # | Subsection | Verdict | +|---|-----------|---------| +| 1 | 1.1 Problem Statement | ALIGNED | +| 2 | 1.2 Why DOT Syntax | ALIGNED | +| 3 | 1.3 Design Principles | ALIGNED | +| 4 | 1.4 Layering and LLM Backends | ALIGNED | + +**Details**: All five design principles are implemented: declarative pipelines (DOT parsed into `Graph`, engine handles execution), pluggable handlers (`HandlerRegistry`), checkpoint/resume (`checkpoint.rs`), human-in-the-loop (`interviewer/` module), edge-based routing (`select_edge()` in `engine.rs`). `CodergenBackend` trait decouples LLM integration. `EventEmitter` provides the event stream for frontends. + +--- + +## Section 2: DOT DSL Schema + +| # | Subsection | Verdict | +|---|-----------|---------| +| 5 | 2.1 Supported Subset | ALIGNED | +| 6 | 2.2 BNF-Style Grammar | ALIGNED | +| 7 | 2.3 Key Constraints | ALIGNED | +| 8 | 2.4 Value Types | ALIGNED | +| 9 | 2.5 Graph Attributes | ALIGNED | +| 10 | 2.6 Node Attributes | ALIGNED | +| 11 | 2.7 Edge Attributes | ALIGNED | +| 12 | 2.8 Shape-to-Handler Mapping | ALIGNED | +| 13 | 2.9 Chained Edges | ALIGNED | +| 14 | 2.10 Subgraphs | ALIGNED | +| 15 | 2.11 Node/Edge Default Blocks | ALIGNED | +| 16 | 2.12 Class Attribute | ALIGNED | +| 17 | 2.13 Minimal Examples | ALIGNED | + +**Minor note**: The parser does not produce a specific error message when `strict digraph` is used — it simply fails to parse. Functionally correct but a UX gap for error messaging. + +--- + +## Section 3: Pipeline Execution Engine + +| # | Subsection | Verdict | +|---|-----------|---------| +| 18 | 3.1 Run Lifecycle (5 phases) | ALIGNED | +| 19 | 3.2 Core Execution Loop | ALIGNED | +| 20 | 3.3 Edge Selection Algorithm (5-step priority) | ALIGNED | +| 21 | 3.4 Goal Gate Enforcement | ALIGNED | +| 22 | 3.5 Retry Logic | ALIGNED | +| 23 | 3.6 Retry Policies (5 presets) | ALIGNED | +| 24 | 3.7 Failure Routing | ALIGNED | +| 25 | 3.8 Concurrency Model | ALIGNED | + +**Minor note**: The 5 named retry presets (none, standard, aggressive, linear, patient) exist as constructors on `RetryPolicy` but `build_retry_policy()` always constructs a custom policy from `max_retries` + default backoff. There is no mechanism for a node to select a preset by name (e.g. `retry_policy="aggressive"`). + +--- + +## Section 4: Node Handlers + +| # | Subsection | Verdict | +|---|-----------|---------| +| 26 | 4.1 Handler Interface | ALIGNED | +| 27 | 4.2 Handler Registry | ALIGNED | +| 28 | 4.3 Start Handler | ALIGNED | +| 29 | 4.4 Exit Handler | ALIGNED | +| 30 | 4.5 Codergen Handler | ALIGNED | +| 31 | 4.6 Wait For Human Handler | ALIGNED | +| 32 | 4.7 Conditional Handler | ALIGNED | +| 33 | 4.8 Parallel Handler | ALIGNED | +| 34 | 4.9 Fan-In Handler | ALIGNED | +| 35 | 4.10 Tool Handler | ALIGNED | +| 36 | 4.11 Manager Loop Handler | ALIGNED | +| 37 | 4.12 Custom Handlers | ALIGNED | + +**Minor note**: Manager loop reads `stack.child_dotfile` from node attrs rather than graph attrs as the spec pseudocode shows. Arguably better design (per-node child pipeline), but deviates from spec. + +--- + +## Section 5: State and Context + +| # | Subsection | Verdict | +|---|-----------|---------| +| 38 | 5.1 PipelineContext | GAP | +| 39 | 5.2 Outcome | ALIGNED | +| 40 | 5.3 Checkpoint | ALIGNED | +| 41 | 5.4 Context Fidelity | GAP | +| 42 | 5.5 Artifact Store | ALIGNED | +| 43 | 5.6 Run Directory Structure | GAP | + +**GAP 38 — `last_stage` / `last_response` context keys**: The spec defines these as engine-set context keys. The engine does not set them; only the `codergen` handler sets them. Other handler types do not propagate these keys. Additionally, `internal.retry_count.` is tracked in a separate `node_retries` HashMap rather than as a context key. + +**GAP 41 — Thread resolution step 3**: The spec lists "Graph-level default thread" as step 3 in thread ID resolution. The implementation uses the node's first CSS class instead. No graph-level default thread concept is implemented. + +**GAP 43 — Per-node `prompt.md` / `response.md`**: The spec defines these as part of the run directory structure. The engine does not write them; only the `codergen` handler does. Other LLM-interacting handlers (fan_in with LLM evaluation) do not write these files. + +--- + +## Section 6: Human-in-the-Loop (Interviewer Pattern) + +| # | Subsection | Verdict | +|---|-----------|---------| +| 44 | 6.1 Interviewer Interface | ALIGNED | +| 45 | 6.2 Question Model | ALIGNED | +| 46 | 6.3 Answer Model | ALIGNED | +| 47 | 6.4 Built-In Implementations (5) | ALIGNED | +| 48 | 6.5 Timeout Handling | GAP | +| 49 | 6.6 Gate Node Behavior | ALIGNED | + +**GAP 48 — WaitHumanHandler bypasses timeout**: `ask_with_timeout()` exists as a utility function but `WaitHumanHandler` calls `interviewer.ask()` directly, so `timeout_seconds` on questions is not enforced for human gate interactions. + +--- + +## Section 7: Validation and Linting + +| # | Subsection | Verdict | +|---|-----------|---------| +| 50 | 7.1 Diagnostic Model | ALIGNED | +| 51 | 7.2 Built-In Rules (14 rules) | ALIGNED | +| 52 | 7.3 Validation API | ALIGNED | +| 53 | 7.4 Custom Lint Rules | ALIGNED | + +**Details**: All 14 spec rules implemented plus a bonus `direction_valid` rule (15 total). Error-severity diagnostics block execution. Custom rules supported via `extra_rules` parameter. + +--- + +## Section 8: Model Stylesheet + +| # | Subsection | Verdict | +|---|-----------|---------| +| 54 | 8.1 Purpose | ALIGNED | +| 55 | 8.2 Grammar | ALIGNED | +| 56 | 8.3 Selectors and Specificity | ALIGNED | +| 57 | 8.4 Recognized Properties | ALIGNED | +| 58 | 8.5 Application/Resolution Order | ALIGNED | + +**Details**: Specificity correctly implemented (Universal=0, Class=1, Id=2). Explicit node attributes are never overridden. Stylesheet applied as a transform before validation. + +--- + +## Section 9: Transforms and Extensibility + +| # | Subsection | Verdict | +|---|-----------|---------| +| 59 | 9.1 AST Transforms | ALIGNED | +| 60 | 9.2 Built-In Transforms (3) | ALIGNED | +| 61 | 9.3 Custom Transforms | ALIGNED | +| 62 | 9.4 Pipeline Composition | ALIGNED | +| 63 | 9.5 HTTP Server Mode | GAP | +| 64 | 9.6 Observability and Events | ALIGNED | +| 65 | 9.7 Tool Call Hooks | GAP | + +**GAP 63 — HTTP Server Mode**: No HTTP server implementation. The spec says "Implementations may expose" making this optional, but it is unimplemented. + +**GAP 65 — Tool Call Hooks**: `tool_hooks.pre` and `tool_hooks.post` are defined in the spec for shell commands around LLM tool calls. Not implemented in any handler. Pre-hook should gate tool calls (non-zero exit = skip), post-hook for logging/auditing. + +**Minor note**: Transform trait uses `&mut Graph` (in-place mutation) rather than returning a new graph as spec describes. Functionally equivalent. + +--- + +## Section 10: Condition Expression Language + +| # | Subsection | Verdict | +|---|-----------|---------| +| 66 | 10.1 Overview | ALIGNED | +| 67 | 10.2 Grammar | ALIGNED | +| 68 | 10.3 Semantics | ALIGNED | +| 69 | 10.4 Variable Resolution | ALIGNED | +| 70 | 10.5 Evaluation | ALIGNED | +| 71 | 10.6 Examples | ALIGNED | +| 72 | 10.7 Extended Operators (future) | ALIGNED | + +**Details**: Full implementation with `=`, `!=`, `&&` conjunction, bare key truthiness, `context.*` double-lookup. Correctly does not implement future operators. + +--- + +## Section 11: Definition of Done + +| # | Subsection | Verdict | +|---|-----------|---------| +| 73 | 11.1 DOT Parsing | ALIGNED | +| 74 | 11.2 Validation and Linting | ALIGNED | +| 75 | 11.3 Execution Engine | ALIGNED | +| 76 | 11.4 Goal Gate Enforcement | ALIGNED | +| 77 | 11.5 Retry Logic | ALIGNED | +| 78 | 11.6 Node Handlers | ALIGNED | +| 79 | 11.7 State and Context | ALIGNED | +| 80 | 11.8 Human-in-the-Loop | ALIGNED | +| 81 | 11.9 Condition Expressions | ALIGNED | +| 82 | 11.10 Model Stylesheet | ALIGNED | +| 83 | 11.11 Transforms and Extensibility | ALIGNED | +| 84 | 11.12 Cross-Feature Parity Matrix | ALIGNED | +| 85 | 11.13 Integration Smoke Test | ALIGNED | + +--- + +## Summary + +| Category | Count | +|----------|-------| +| Total items reviewed | 85 | +| ALIGNED | 78 | +| GAP | 7 | +| Alignment rate | 91.8% | + +### All Gaps + +| # | Section | Gap | Severity | +|---|---------|-----|----------| +| 38 | 5.1 | `last_stage`/`last_response` not set by engine; `internal.retry_count` not in context | Low | +| 41 | 5.4 | Thread resolution missing graph-level default thread (step 3) | Low | +| 43 | 5.6 | `prompt.md`/`response.md` only written by codergen, not other LLM handlers | Low | +| 48 | 6.5 | WaitHumanHandler calls `ask()` directly, bypassing `ask_with_timeout()` | Medium | +| 63 | 9.5 | HTTP server mode not implemented (spec marks as optional) | Low | +| 65 | 9.7 | `tool_hooks.pre`/`tool_hooks.post` not implemented | Medium | + +### Minor Notes (not gaps, but deviations) + +- No specific error for `strict digraph` (parser just fails) +- Named retry presets exist but no node-level attribute to select them +- Manager loop reads `child_dotfile` from node attrs not graph attrs +- Transform trait mutates in-place rather than returning new graph diff --git a/docs/agent/reviews/spec-compliance-review.md b/docs/agent/reviews/spec-compliance-review.md new file mode 100644 index 000000000..fbf0fdf7f --- /dev/null +++ b/docs/agent/reviews/spec-compliance-review.md @@ -0,0 +1,701 @@ +# Attractor Spec Compliance Review + +**Date:** 2026-02-21 +**Spec:** `docs/specs/attractor-spec.md` +**Implementation:** `crates/attractor/src/` +**Reviewers:** 5 parallel agents, each covering distinct spec sections + +--- + +## Section 1: Overview and Goals + +### 1. Section 1.1 — Problem Statement +**ALIGNED** +The implementation delivers a DOT-based directed-graph pipeline runner as described. No code artifact required beyond the overall architecture. + +### 2. Section 1.2 — Why DOT Syntax +**ALIGNED** +The parser (`parser/grammar.rs`) starts with `digraph` keyword and builds on directed graph primitives. DOT subset parser implemented from scratch. + +### 3. Section 1.3 — Design Principles +**ALIGNED** +- Declarative pipelines: `.dot` files declare graph structure; engine traverses it (`parser/mod.rs:18`, `graph/types.rs:277-294`) +- Pluggable handlers: `Node::handler_type()` at `graph/types.rs:203-209` resolves from `type` attr or shape mapping +- Checkpoint and resume: `checkpoint` module (`lib.rs:2`), checkpoint save/load implemented +- Human-in-the-loop: `interviewer` module (`lib.rs:10`), `wait.human` mapped at `graph/types.rs:77` +- Edge-based routing: `Edge` struct has `condition()`, `weight()`, `label()` accessors at `graph/types.rs:241-274` + +### 4. Section 1.4 — Layering and LLM Backends +**ALIGNED** +No LLM SDK dependency. `CodergenBackend` trait decouples LLM calls. Event stream module at `lib.rs:8`. + +--- + +## Section 2: DOT DSL Schema + +### 5. Section 2.1 — Supported Subset +**ALIGNED** +Parser accepts only `digraph` (`grammar.rs:140`). No code path for `graph` (undirected) or `strict`. Trailing content rejected at `parser/mod.rs:23-29`. + +### 6. Section 2.2 — BNF Grammar +**ALIGNED** (minor gap) +All grammar productions verified against implementation: +- `Graph`, `Statement`, `GraphAttrStmt`, `NodeDefaults`, `EdgeDefaults`, `GraphAttrDecl`, `SubgraphStmt`, `NodeStmt`, `EdgeStmt`, `AttrBlock`, `Attr` — all match at `grammar.rs:14-152` +- `QualifiedId` (dotted keys) supported at `lexer.rs:90-113` +- All value types including Duration with `ms/s/m/h/d` at `lexer.rs:229-246` +- **Minor gap:** `Direction` type (`TB|LR|BT|RL`) parsed as bare `AstValue::Ident` — no validation restricting to valid values. Functionally works but invalid directions accepted silently. + +### 7. Section 2.3 — Key Constraints +**ALIGNED** +- One digraph per file: trailing content check at `parser/mod.rs:23-29` +- Bare identifiers: `[A-Za-z_][A-Za-z0-9_]*` at `lexer.rs:82-87` +- Commas required: `separated_list1(preceded(ws, char(',')), attr)` at `grammar.rs:26-31` +- Directed edges only: only `->` parsed at `lexer.rs:262-264` +- Comments: `strip_comments()` at `lexer.rs:5-53` handles `//` and `/* */` +- Semicolons optional: `opt_semi` at `grammar.rs:34-36` + +### 8. Section 2.4 — Value Types +**ALIGNED** +All five types implemented with correct syntax: +| Type | Implementation | +|------|---------------| +| String | `lexer.rs:121-177` with `\"`, `\n`, `\t`, `\\` escapes | +| Integer | `lexer.rs:208-226` with sign and float-rejection | +| Float | `lexer.rs:193-205` | +| Boolean | `lexer.rs:180-190` | +| Duration | `lexer.rs:229-246` with `ms/s/m/h/d` units | + +### 9. Section 2.5 — Graph-Level Attributes +**ALIGNED** +All 7 attributes present with correct defaults: +| Key | Evidence | +|-----|----------| +| `goal` | `graph/types.rs:333-338` — default `""` | +| `label` | stored as generic attr | +| `model_stylesheet` | `graph/types.rs:341-346` — default `""` | +| `default_max_retry` | `graph/types.rs:349-354` — default `50` | +| `retry_target` | `graph/types.rs:357-359` | +| `fallback_retry_target` | `graph/types.rs:361-366` | +| `default_fidelity` | `graph/types.rs:369-373` | + +### 10. Section 2.6 — Node Attributes +**ALIGNED** +All 18 attributes present with correct defaults: +| Key | Evidence | +|-----|----------| +| `label` | `graph/types.rs:119-121` — falls back to node ID | +| `shape` | `graph/types.rs:124-126` — default `"box"` | +| `type` | `graph/types.rs:129-131` | +| `prompt` | `graph/types.rs:134-136` | +| `max_retries` | `graph/types.rs:139-141` — `None` when unset | +| `goal_gate` | `graph/types.rs:144-146` — default `false` | +| `retry_target` | `graph/types.rs:149-151` | +| `fallback_retry_target` | `graph/types.rs:153-156` | +| `fidelity` | `graph/types.rs:159-161` | +| `thread_id` | `graph/types.rs:164-166` | +| `class` | `graph/types.rs:169-171` + parsing at `semantic.rs:103-117` | +| `timeout` | `graph/types.rs:173-175` — `Option` | +| `llm_model` | `graph/types.rs:178-180` | +| `llm_provider` | `graph/types.rs:183-185` | +| `reasoning_effort` | `graph/types.rs:188-190` — default `"high"` | +| `auto_status` | `graph/types.rs:193-195` — default `false` | +| `allow_partial` | `graph/types.rs:198-200` — default `false` | + +### 11. Section 2.7 — Edge Attributes +**ALIGNED** +All 7 attributes present: +| Key | Evidence | +|-----|----------| +| `label` | `graph/types.rs:242-244` | +| `condition` | `graph/types.rs:247-249` | +| `weight` | `graph/types.rs:252-254` — default `0` | +| `fidelity` | `graph/types.rs:257-259` | +| `thread_id` | `graph/types.rs:262-264` | +| `loop_restart` | `graph/types.rs:267-269` — default `false` | +| `freeform` | `graph/types.rs:272-274` — default `false` | + +### 12. Section 2.8 — Shape-to-Handler Mapping +**ALIGNED** +All 9 mappings present at `graph/types.rs:72-85`. Handler resolution with `type` override at `graph/types.rs:203-209`. + +### 13. Section 2.9 — Chained Edges +**ALIGNED** +Parsed at `grammar.rs:86-101`, expanded via `windows(2)` at `semantic.rs:132-141`. Test at `semantic.rs:471-484` confirms correct desugaring. + +### 14. Section 2.10-2.12 — Subgraphs, Defaults, Class Attribute +**ALIGNED** +- Subgraph scoping: `semantic.rs:145-235` saves/restores defaults per scope +- Class derivation from subgraph label: `semantic.rs:51-58` (`derive_class_from_label`) +- Comma-separated class parsing: `semantic.rs:103-117`, tested at `semantic.rs:487-500` +- Node/edge default blocks: `grammar.rs:44-54`, applied in `semantic.rs:76-83` + +--- + +## Section 3: Pipeline Execution Engine + +### 15. Section 3.1 — Run Lifecycle +**ALIGNED** +Five phases implemented: +- PARSE: `pipeline.rs:34` calls `crate::parser::parse(dot_source)` +- VALIDATE: `pipeline.rs:46` calls `validation::validate(&graph, &[])` +- INITIALIZE: `engine.rs:580-625` creates run directory, initializes context +- EXECUTE: `engine.rs:627-771` main loop +- FINALIZE: `engine.rs:773-784` emits `PipelineCompleted`, returns outcome + +### 16. Section 3.2 — Core Execution Loop +**ALIGNED** +All 8 steps from spec implemented: +1. Start node resolution: `engine.rs:620-624` via `graph.find_start_node()` at `graph/types.rs:310-320` +2. Terminal check: `engine.rs:633-653` +3. Execute with retry: `engine.rs:669-679` +4. Record completion: `engine.rs:706-708` +5. Apply context updates: `engine.rs:711-715` +6. Save checkpoint: `engine.rs:718-730` +7. Select next edge: `engine.rs:733-753` +8. Loop restart: `engine.rs:760-767`, advance: `engine.rs:768` + +### 17. Section 3.3 — Edge Selection Algorithm +**ALIGNED** +Five-step priority fully implemented at `engine.rs:299-356`: +1. Condition matching: `engine.rs:311-321` +2. Preferred label: `engine.rs:324-333` with `normalize_label` at `engine.rs:254-279` +3. Suggested next IDs: `engine.rs:336-342` +4. Weight + lexical tiebreak: `engine.rs:345-352` via `best_by_weight_then_lexical` at `engine.rs:282-295` +5. Fallback: `engine.rs:355` + +### 18. Section 3.4 — Goal Gate Enforcement +**ALIGNED** +- `check_goal_gates` at `engine.rs:362-377`: checks goal_gate nodes for SUCCESS/PARTIAL_SUCCESS +- `get_retry_target` at `engine.rs:380-408`: four-level fallback chain (node → node fallback → graph → graph fallback) +- `is_terminal` at `engine.rs:411-414` + +### 19. Section 3.5 — Retry Logic +**ALIGNED** (minor gaps) +- `build_retry_policy` at `engine.rs:178-189`: node `max_retries` with `default_max_retry` fallback +- `execute_with_retry` at `engine.rs:449-536`: full retry loop +- **Minor:** Retry counters not persisted to `Checkpoint.node_retries` during execution (field exists at `checkpoint.rs:17` but not populated) +- **Minor:** Spec self-contradicts on `default_max_retry` default (section 3.5 says 0, section 2 says 50). Implementation follows section 2 (50). + +### 20. Section 3.6 — Retry Policy / Backoff +**ALIGNED** (minor gap) +- `BackoffConfig` at `engine.rs:28-44` with correct defaults +- `delay_for_attempt` at `engine.rs:50-76`: formula matches spec exactly +- All 5 presets match spec table (none/standard/aggressive/linear/patient) +- **Minor:** Default `should_retry` at `engine.rs:101-103` retries ALL errors. Spec defines granular behavior (retry 429/5xx, fail 401/403/400). + +### 21. Section 3.7 — Failure Routing +**ALIGNED** +All 4 priority steps at `engine.rs:733-753`: fail edge → retry_target → fallback_retry_target → pipeline termination. + +### 22. Section 3.8 — Concurrency Model +**ALIGNED** +Single-threaded graph traversal at `engine.rs:627-771`. One node at a time. + +### 23. Section 3.9 — Auto Status +**GAP** +`auto_status()` accessor exists at `graph/types.rs:193-194` but **engine never references it**. Engine always writes status.json itself at line 703. No auto-synthesis logic for when a handler writes no status. + +### 24. Section 3.10 — Timeout Enforcement +**GAP** +Node `timeout` attribute parsed as `Option` (item 10) but **not enforced during handler execution** in the engine. The tool handler (`tool.rs:61-78`) does use timeout for subprocess execution, but the engine does not wrap general handler execution with a timeout. + +--- + +## Section 4: Node Handlers + +### 25. Section 4.1 — Handler Trait +**ALIGNED** +Trait at `handler/mod.rs:22-31`: `async fn execute(&self, node, context, graph, logs_root) -> Result`. All four parameters match spec. `Result` wrapping for error propagation. + +### 26. Section 4.2 — Handler Registry +**ALIGNED** +At `handler/mod.rs:34-74`: `HashMap>`, `default_handler`, `register()` replaces existing, three-step `resolve()` (explicit type → shape → default). Tests confirm at lines 153-177. + +### 27. Section 4.3 — Start Handler +**ALIGNED** +`handler/start.rs:13-26`: returns `Outcome::success()`. No-op. + +### 28. Section 4.4 — Exit Handler +**ALIGNED** +`handler/exit.rs:13-26`: returns `Outcome::success()`. No goal gate logic (handled by engine). + +### 29. Section 4.5 — Codergen Handler +**ALIGNED** +- Prompt building: `codergen.rs:87-91` — `node.prompt()` falling back to `node.label()` +- `$goal` expansion: `codergen.rs:42-44` +- Log writing: `codergen.rs:94-96` — `prompt.md`, `response.md`, `status.json` +- Backend call: `codergen.rs:106-121` — handles `CodergenResult::Full` and `CodergenResult::Text` +- Simulation mode when no backend: `codergen.rs:122+` +- Context updates `last_stage`/`last_response`: `codergen.rs:139-146` +- Tool hooks (pre/post): `codergen.rs:98-131` (enhancement beyond spec) +- `CodergenBackend` trait at `codergen.rs:20-27` matches spec's `run(node, prompt, context) -> String | Outcome` + +### 30. Section 4.6 — Wait Human Handler +**ALIGNED** +Comprehensive implementation at `wait_human.rs`: +- Choice derivation from outgoing edges: lines 110-129 +- Freeform edge detection: line 115 +- No-edges failure: lines 131-133 +- Question building with `MultipleChoice`, `allow_freeform`, `stage`: lines 136-150 +- Timeout handling with default choice fallback: lines 162-179 +- Skipped handling: lines 183-185 +- Fixed-choice match with `suggested_next_ids` and context updates: lines 195-201 +- Freeform fallback: lines 204-221 +- First-choice fallback: lines 224-226 +- Accelerator key parsing `[K] Label`, `K) Label`, `K - Label`, first char: lines 32-71 + +### 31. Section 4.7 — Conditional Handler +**ALIGNED** +`handler/conditional.rs:14-28`: no-op returning SUCCESS with note. Routing handled by engine. + +### 32. Section 4.8 — Parallel Handler +**ALIGNED** +Full implementation at `parallel.rs`: +- 4 join policies: `wait_all`, `first_success`, `k_of_n(K)`, `quorum(fraction)` at lines 36-59 +- 3 error policies: `continue`, `fail_fast`, `ignore` at lines 62-75 +- `max_parallel` with `Semaphore` for bounded concurrency: lines 113-120 +- Context isolation via `clone_context()`: line 126 +- Join evaluation with all four policies: lines 249-286 +- Fail-fast behavior: lines 185-189 +- Context storage for fan-in (`parallel.results`, `parallel.branch_count`): lines 230-241 + +### 33. Section 4.9 — Fan-In Handler +**ALIGNED** (minor gaps) +- Reads `parallel.results`: `fan_in.rs:34-37` +- LLM-based evaluation when prompt + backend present: lines 41-42 +- Heuristic selection by status rank: lines 77-115 +- Context updates `parallel.fan_in.best_id`/`best_outcome`: lines 48-55 +- **Minor:** No `score`-based sorting in heuristic (spec mentions `-c.score`) +- **Minor:** Returns SUCCESS when results exist even if all candidates failed + +### 34. Section 4.10 — Tool Handler +**ALIGNED** +At `tool.rs`: +- Reads `tool_command` from attrs: lines 51-55 +- Empty command returns fail: lines 57-59 +- Runs via `sh -c` with timeout support: lines 61-78 +- Sets `tool.output` to stdout: lines 15-40 + +### 35. Section 4.11 — Manager Loop Handler +**GAP** (partial) +Core observe/steer/wait cycle implemented at `manager_loop.rs:59-157`: +- Poll interval, max cycles, stop condition, actions parsing: lines 67-100 +- Observe/steer delegation: lines 105-115 +- Child status check: lines 119-132 +- Max cycles exceeded: lines 152-155 +- **GAP: `stack.child_autostart`/`start_child_pipeline` not implemented** — spec says autostart child pipeline, impl does not +- **Minor:** `steer_cooldown_elapsed()` not implemented — steers every cycle + +### 36. Section 4.12 — Custom Handlers +**ALIGNED** +Trait-based design inherently supports custom handlers via `register()`. `Send + Sync` bounds match spec contract. + +--- + +## Section 5: State and Context + +### 37. Section 5.1 — Context +**ALIGNED** +Thread-safe key-value store at `context.rs:1-11`: `Arc>>` with all spec methods: +- `set()`: line 33-38 +- `get()`: line 46-52 +- `get_string()`: line 56-60 +- `append_log()`: line 67-72 +- `snapshot()`: line 80-85 +- `clone_context()`: line 98-106 (deep copy for parallel isolation) +- `apply_updates()`: line 113-118 + +### 38. Section 5.2 — Outcome +**ALIGNED** +At `outcome.rs`: +- `StageStatus` enum (lines 10-16): `Success`, `Fail`, `PartialSuccess`, `Retry`, `Skipped` — all 5 spec variants +- `Outcome` struct (lines 48-60): `status`, `preferred_label`, `suggested_next_ids`, `context_updates`, `notes`, `failure_reason` — all match +- Factory methods (lines 63-107): `success()`, `fail()`, `retry()`, `skipped()` + +### 39. Section 5.3 — Checkpoint +**ALIGNED** (minor gaps) +At `checkpoint.rs:13-20`: `timestamp`, `current_node`, `completed_nodes`, `node_retries`, `context_values`, `logs` — all match spec. +- `save()` at line 44-49, `load()` at line 56-61 +- Resume at `engine.rs:591-608`: restores context, logs, completed_nodes, resumes from next node +- **Minor:** Retry counters not restored during resume +- **Minor:** Fidelity degradation on resume (`full` → `summary:high`) not implemented per spec line 1166 + +### 40. Section 5.4 — Fidelity Modes +**ALIGNED** (minor gap) +`resolve_fidelity` at `engine.rs:199-211` implements full 4-level precedence: +1. Edge `fidelity` attribute (line 201) +2. Target node `fidelity` attribute (line 205) +3. Graph `default_fidelity` attribute (line 208) +4. Default: `"compact"` (line 211) + +Tests confirm all four levels at `engine.rs:1462-1511`. + +Thread tracking: `engine.rs:660-664` stores `thread.{tid}.current_node`. + +**Minor gap:** Thread ID resolution only implements step 1 of 5 from spec (node `thread_id`). Missing: edge `thread_id`, graph default, subgraph class derivation, fallback to previous node ID. + +### 41. Section 5.5 — Artifact Store +**ALIGNED** +Full implementation at `artifact.rs`: +- `ArtifactStore` with `base_dir` and `RwLock`: lines 31-34 +- `store()` with file-backing above 100KB threshold: lines 62-96 +- `retrieve()` from memory or file: lines 107-130 +- `has()`, `list()`, `remove()`, `clear()`: lines 137-184 +- `ArtifactInfo` struct with all 5 fields: lines 16-22 + +### 42. Section 5.6 — Run Directory Structure +**ALIGNED** (minor gap) +All spec artifacts written: `checkpoint.json`, `manifest.json`, `{node_id}/status.json`, `{node_id}/prompt.md`, `{node_id}/response.md`, `artifacts/{artifact_id}.json`. +- **Minor:** Manifest (`engine.rs:217-232`) includes `node_count`/`edge_count` but not `goal` field from spec. + +--- + +## Section 6: Human-in-the-Loop (Interviewer Pattern) + +### 43. Section 6.1 — Interviewer Interface +**ALIGNED** +Three methods at `interviewer/mod.rs:152-167`: `ask()`, `ask_multiple()` (with default sequential impl), `inform()` (with default no-op). Async via `async_trait`. + +### 44. Section 6.2 — Question Model +**ALIGNED** +At `mod.rs:29-39`: `text`, `question_type` (4 variants: `YesNo`, `MultipleChoice`, `Freeform`, `Confirmation`), `options` (key + label), `allow_freeform`, `default`, `timeout_seconds`, `stage`, `metadata` — all 8 fields present. + +### 45. Section 6.3 — Answer Model +**ALIGNED** +`AnswerValue` enum (lines 57-65): `Yes`, `No`, `Skipped`, `Timeout`, `Selected(String)`, `Text(String)`. `Answer` struct (lines 68-73): `value`, `selected_option`, `text`. + +### 46. Section 6.4 — Built-In Interviewers +**ALIGNED** (all 5) +- **AutoApprove** (`auto_approve.rs:10-23`): YES for YesNo/Confirmation, first option for MultipleChoice, "auto-approved" for Freeform +- **Console** (`console.rs:49-86`): `[?]` prefix, option display, freeform fallback, Y/N for YesNo, `>` prompt for Freeform +- **Callback** (`callback.rs:6-22`): delegates to `Box Answer>` +- **Queue** (`queue.rs:9-28`): `Mutex>`, returns SKIPPED when empty +- **Recording** (`recording.rs:8-39`): wraps inner interviewer, records `(Question, Answer)` pairs + +### 47. Section 6.5 — Timeout Handling +**ALIGNED** +At `mod.rs:133-149`: uses `tokio::time::timeout`, returns `default_answer.unwrap_or_else(Answer::timeout)`. Tests at lines 269-296. + +--- + +## Section 7: Validation and Linting + +### 48. Section 7.1 — Diagnostic Model +**ALIGNED** +At `validation/mod.rs:9-25`: `Severity` (Error/Warning/Info), `Diagnostic` with `rule`, `severity`, `message`, `node_id`, `edge`, `fix` — all fields match. + +### 49. Section 7.2 — Built-In Lint Rules +**ALIGNED** (14/14 rules implemented) +At `validation/rules.rs`: +| Rule | Severity | Location | +|------|----------|----------| +| `start_node` | ERROR | lines 30-69 | +| `terminal_node` | ERROR | lines 73-104 | +| `reachability` | ERROR | lines 108-155 (BFS) | +| `edge_target_exists` | ERROR | lines 159-201 | +| `start_no_incoming` | ERROR | lines 205-233 | +| `exit_no_outgoing` | ERROR | lines 237-272 | +| `condition_syntax` | ERROR | lines 276-316 | +| `stylesheet_syntax` | ERROR | lines 320-348 | +| `type_known` | WARNING | lines 352-392 | +| `fidelity_valid` | WARNING | lines 396-471 | +| `retry_target_exists` | WARNING | lines 475-548 | +| `goal_gate_has_retry` | WARNING | lines 552-586 | +| `prompt_on_llm_nodes` | WARNING | lines 590-624 | +| `freeform_edge_count` | ERROR | lines 628-663 | + +**Minor:** `stylesheet_syntax` only checks brace balance, not full parse. + +### 50. Section 7.3 — Validation API +**ALIGNED** +`validate(graph, extra_rules)` at line 35, `validate_or_raise(graph, extra_rules)` at line 52. + +### 51. Section 7.4 — Custom Lint Rules +**ALIGNED** +`LintRule` trait at `mod.rs:28-31` with `name()` and `apply()`. Custom rules via `extra_rules` parameter. + +--- + +## Section 8: Model Stylesheet + +### 52. Section 8.1 — CSS-like Syntax +**ALIGNED** +`parse_stylesheet` at `stylesheet.rs:54-85` parses selector blocks with property declarations. + +### 53. Section 8.2 — Selectors and Specificity +**ALIGNED** (minor extension) +Implementation at `stylesheet.rs:18-27` adds a `Shape` selector beyond spec: +| Selector | Specificity | +|----------|-------------| +| `*` (Universal) | 0 | +| Shape (bare word) | 1 | +| `.class` | 2 | +| `#id` | 3 | + +Spec defines `*`=0, `.class`=1, `#id`=2. Relative ordering preserved; the `Shape` selector is an undocumented extension. Cascading behavior correct. + +### 54. Section 8.3 — Application Order +**ALIGNED** +At `stylesheet.rs:190-238`: sorts by specificity, higher overwrites lower, explicit node attributes always override. Test at `stylesheet.rs:395-442` verifies spec section 8.6 example exactly. + +### 55. Section 8.4 — Recognized Properties +**ALIGNED** +`STYLESHEET_PROPERTIES` at line 182: `["llm_model", "llm_provider", "reasoning_effort"]`. Exact match. + +--- + +## Section 9: Transforms and Extensibility + +### 56. Section 9.1 — Transform Trait +**ALIGNED** +At `transform.rs:5-7`: `fn apply(&self, graph: &mut Graph)`. In-place mutation vs spec's return-new-graph — functionally equivalent. + +### 57. Section 9.2 — Built-In Transforms +**ALIGNED** +Three built-in transforms: +- **Variable Expansion:** `transform.rs:10-25` — expands `$goal` in prompts +- **Stylesheet Application:** `transform.rs:52-65` — applies `model_stylesheet` +- **Preamble:** `transform.rs:28-49` — prepends `[Context mode: {fidelity}]` for non-full fidelity +- **Minor:** Preamble applied at parse time, not execution time; cannot incorporate runtime fidelity changes from edges + +### 58. Section 9.3 — Custom Transforms +**ALIGNED** +`PipelineBuilder::register_transform()` at `pipeline.rs:24-26`. Custom transforms run after built-in, in registration order. Integration test at `pipeline.rs:149-169`. + +### 59. Section 9.4 — Event Stream +**ALIGNED** +All spec event types implemented at `event.rs:5-74`: +- Pipeline lifecycle: `PipelineStarted`, `PipelineCompleted`, `PipelineFailed` +- Stage lifecycle: `StageStarted`, `StageCompleted`, `StageFailed`, `StageRetrying` +- Parallel: `ParallelStarted`, `ParallelBranchStarted`, `ParallelBranchCompleted`, `ParallelCompleted` +- Human: `InterviewStarted`, `InterviewCompleted`, `InterviewTimeout` +- Checkpoint: `CheckpointSaved` + +Observer pattern via `EventEmitter::on_event()` at line 106. Engine emits throughout execution. + +### 60. Section 9.5 — Tool Call Hooks +**ALIGNED** (minor discrepancy) +Pre/post hooks at `codergen.rs:98-131`. `resolve_hook()` at lines 56-62 checks node-level then graph-level. +- **Minor:** Pre-hook non-zero returns `Outcome::fail()` (stronger than spec's "skip the tool call") + +### 61. Section 9.6 — HTTP Server Mode +**N/A** — Spec says "Implementations may expose..." (optional). Not implemented. + +--- + +## Section 10: Condition Expression Language + +### 62. Section 10.1 — Grammar +**ALIGNED** +At `condition.rs`: `&&` conjunction (line 29), `!=` (lines 33-45), `=` (lines 46-58), bare key truthy (lines 59-72). + +### 63. Section 10.2 — Semantics +**ALIGNED** +- Clauses AND-combined: `condition.rs:134` uses `.all()` +- `outcome` resolves to status string: line 88-89 +- `preferred_label` resolves: lines 91-96 +- `context.*` lookup with fallback: lines 98-105 +- Missing keys = empty string: line 105 +- Empty condition = true: lines 130-132 + +### 64. Section 10.3 — Variable Resolution +**ALIGNED** +`resolve_key()` at lines 87-110 follows spec pseudocode exactly: `outcome` → `preferred_label` → `context.` prefix with qualified/unqualified fallback → direct context lookup → empty string. + +### 65. Section 10.4 — Examples +**ALIGNED** +Tests cover all spec examples: `outcome=success` (line 169), `context.tests_passed=true` (line 206), `preferred_label=Fix` (line 189). + +### 66. Section 10.5 — Extended Operators +**ALIGNED** +Correctly NOT implemented per spec: "documented as potential extensions... Implementations should not add them." + +--- + +## Section 11: Definition of Done + +### 67. Section 11.1 — DOT Parsing +**ALIGNED** +Integration tests parse all 3 spec examples at `integration.rs:30-143`. + +### 68. Section 11.2 — Validation and Linting +**ALIGNED** +14 lint rules, `validate_or_raise()` used in integration tests. + +### 69. Section 11.3 — Execution Engine +**ALIGNED** +Start node resolution, handler dispatch, outcome recording, edge selection, loop execution, terminal stop — all verified in integration tests at `integration.rs:158-203`. + +### 70. Section 11.4 — Goal Gate Enforcement +**ALIGNED** +Integration tests at `integration.rs:432-608`. + +### 71. Section 11.5 — Retry Logic +**ALIGNED** +Integration test at `integration.rs:828-902`. + +### 72. Section 11.6 — Node Handlers +**ALIGNED** +All handler types exist. Custom handler registration works. + +### 73. Section 11.7 — State and Context +**ALIGNED** +Context updates, checkpoint save/resume, artifacts — verified at `integration.rs:978-1016`. + +### 74. Section 11.8 — Human-in-the-Loop +**ALIGNED** +All interviewer implementations present. Integration test with QueueInterviewer at `integration.rs:376-381`. + +### 75. Section 11.9 — Condition Expressions +**ALIGNED** +All operators and variable types tested at `condition.rs:160-323`. + +### 76. Section 11.10 — Model Stylesheet +**ALIGNED** +Integration tests at `integration.rs:677-822` verify selectors, specificity, cascading. + +### 77. Section 11.11 — Transforms +**ALIGNED** +Transform interface, variable expansion, custom transforms — all tested. + +### 78. Section 11.12 — Cross-Feature Parity Matrix +**ALIGNED** (minor gap) +22 of 23 matrix items pass. **Minor:** No dedicated integration test for parallel fan-out/fan-in (handlers exist, no end-to-end test). + +### 79. Section 11.13 — Integration Smoke Test +**ALIGNED** +`integration.rs:1040-1180` implements mock-backend smoke test matching spec pattern. + +--- + +## Appendices + +### 80. Appendix A — Complete Attribute Reference +**ALIGNED** +All graph, node, and edge attributes have corresponding accessors. Dotted keys (`tool_hooks.pre/post`, `stack.*`) supported via `lexer.rs:314-315`. + +### 81. Appendix B — Shape-to-Handler-Type Mapping +**ALIGNED** +All 9 mappings tested at `graph/types.rs:413-429`. + +### 82. Appendix C — Status File Contract +**GAP** +`auto_status=true` synthesis not implemented in engine (see item 23). `Outcome` struct at `outcome.rs:48-60` matches contract fields, but the engine never checks `auto_status`. + +### 83. Appendix D — Error Categories +**ALIGNED** (minor gap) +`AttractorError` at `error.rs:4-25` has 7 variants: `Parse`, `Validation`, `Engine`, `Handler`, `Checkpoint`, `Stylesheet`, `Io`. +**Minor:** Spec defines 3 abstract categories (Retryable, Terminal, Pipeline). No explicit classification of which variants are retryable vs terminal; default `should_retry` retries all. + +--- + +## Summary + +| # | Section | Verdict | +|---|---------|---------| +| 1 | 1.1 Problem Statement | ALIGNED | +| 2 | 1.2 Why DOT Syntax | ALIGNED | +| 3 | 1.3 Design Principles | ALIGNED | +| 4 | 1.4 Layering / LLM Backends | ALIGNED | +| 5 | 2.1 Supported Subset | ALIGNED | +| 6 | 2.2 BNF Grammar | ALIGNED (minor: Direction not validated) | +| 7 | 2.3 Key Constraints | ALIGNED | +| 8 | 2.4 Value Types | ALIGNED | +| 9 | 2.5 Graph-Level Attributes | ALIGNED | +| 10 | 2.6 Node Attributes | ALIGNED | +| 11 | 2.7 Edge Attributes | ALIGNED | +| 12 | 2.8 Shape-to-Handler Mapping | ALIGNED | +| 13 | 2.9 Chained Edges | ALIGNED | +| 14 | 2.10-2.12 Subgraphs/Defaults/Class | ALIGNED | +| 15 | 3.1 Run Lifecycle | ALIGNED | +| 16 | 3.2 Core Execution Loop | ALIGNED | +| 17 | 3.3 Edge Selection Algorithm | ALIGNED | +| 18 | 3.4 Goal Gate Enforcement | ALIGNED | +| 19 | 3.5 Retry Logic | ALIGNED (minor: counters not persisted) | +| 20 | 3.6 Retry Policy / Backoff | ALIGNED (minor: should_retry too coarse) | +| 21 | 3.7 Failure Routing | ALIGNED | +| 22 | 3.8 Concurrency Model | ALIGNED | +| 23 | 3.9 Auto Status | **GAP** | +| 24 | 3.10 Timeout Enforcement | **GAP** | +| 25 | 4.1 Handler Trait | ALIGNED | +| 26 | 4.2 Handler Registry | ALIGNED | +| 27 | 4.3 Start Handler | ALIGNED | +| 28 | 4.4 Exit Handler | ALIGNED | +| 29 | 4.5 Codergen Handler | ALIGNED | +| 30 | 4.6 Wait Human Handler | ALIGNED | +| 31 | 4.7 Conditional Handler | ALIGNED | +| 32 | 4.8 Parallel Handler | ALIGNED | +| 33 | 4.9 Fan-In Handler | ALIGNED (minor: no score sort, all-fail case) | +| 34 | 4.10 Tool Handler | ALIGNED | +| 35 | 4.11 Manager Loop Handler | **GAP** (child_autostart missing) | +| 36 | 4.12 Custom Handlers | ALIGNED | +| 37 | 5.1 Context | ALIGNED | +| 38 | 5.2 Outcome | ALIGNED | +| 39 | 5.3 Checkpoint | ALIGNED (minor: retry counters, fidelity degradation on resume) | +| 40 | 5.4 Fidelity Modes | ALIGNED (minor: thread_id resolution incomplete) | +| 41 | 5.5 Artifact Store | ALIGNED | +| 42 | 5.6 Run Directory | ALIGNED (minor: manifest missing goal) | +| 43 | 6.1 Interviewer Interface | ALIGNED | +| 44 | 6.2 Question Model | ALIGNED | +| 45 | 6.3 Answer Model | ALIGNED | +| 46 | 6.4 Built-In Interviewers | ALIGNED | +| 47 | 6.5 Timeout Handling | ALIGNED | +| 48 | 7.1 Diagnostic Model | ALIGNED | +| 49 | 7.2 Built-In Lint Rules | ALIGNED (14/14) | +| 50 | 7.3 Validation API | ALIGNED | +| 51 | 7.4 Custom Lint Rules | ALIGNED | +| 52 | 8.1 CSS-like Syntax | ALIGNED | +| 53 | 8.2 Selectors/Specificity | ALIGNED (extra Shape selector) | +| 54 | 8.3 Application Order | ALIGNED | +| 55 | 8.4 Recognized Properties | ALIGNED | +| 56 | 9.1 Transform Trait | ALIGNED | +| 57 | 9.2 Built-In Transforms | ALIGNED | +| 58 | 9.3 Custom Transforms | ALIGNED | +| 59 | 9.4 Event Stream | ALIGNED | +| 60 | 9.5 Tool Call Hooks | ALIGNED (minor: pre-hook behavior) | +| 61 | 9.6 HTTP Server Mode | N/A (optional) | +| 62 | 10.1 Grammar | ALIGNED | +| 63 | 10.2 Semantics | ALIGNED | +| 64 | 10.3 Variable Resolution | ALIGNED | +| 65 | 10.4 Examples | ALIGNED | +| 66 | 10.5 Extended Operators | ALIGNED | +| 67 | 11.1 DOT Parsing | ALIGNED | +| 68 | 11.2 Validation | ALIGNED | +| 69 | 11.3 Execution Engine | ALIGNED | +| 70 | 11.4 Goal Gates | ALIGNED | +| 71 | 11.5 Retry Logic | ALIGNED | +| 72 | 11.6 Node Handlers | ALIGNED | +| 73 | 11.7 State/Context | ALIGNED | +| 74 | 11.8 Human-in-the-Loop | ALIGNED | +| 75 | 11.9 Conditions | ALIGNED | +| 76 | 11.10 Stylesheet | ALIGNED | +| 77 | 11.11 Transforms | ALIGNED | +| 78 | 11.12 Parity Matrix | ALIGNED (minor: no parallel integration test) | +| 79 | 11.13 Smoke Test | ALIGNED | +| 80 | Appendix A — Attributes | ALIGNED | +| 81 | Appendix B — Shape Mapping | ALIGNED | +| 82 | Appendix C — Status File | **GAP** (auto_status not enforced) | +| 83 | Appendix D — Error Categories | ALIGNED (minor: no retryable/terminal classification) | + +--- + +## Totals + +**79 ALIGNED / 3 GAP / 1 N/A** (+ 16 minor gaps within ALIGNED items) + +### Hard Gaps (3) + +1. **Auto Status (item 23/82):** `auto_status` accessor exists at `graph/types.rs:193-194` but engine never checks it. No auto-synthesis of SUCCESS when handler writes no status. + +2. **Timeout Enforcement (item 24):** `timeout` attribute parsed as `Duration` but not enforced as a deadline around handler execution in the engine. Tool handler uses it for subprocess timeout, but no general enforcement. + +3. **Manager Loop `child_autostart` (item 35):** `stack.child_autostart` / `start_child_pipeline` not implemented. The observe/steer/wait cycle exists but cannot auto-launch a child pipeline. + +### Notable Minor Gaps (within ALIGNED items) + +- Thread ID resolution: only step 1 of 5 implemented (node `thread_id`); missing edge, graph default, subgraph class, previous-node fallback +- Checkpoint resume: retry counters not restored; fidelity degradation (`full` → `summary:high`) not applied +- Retry policy: `should_retry` retries ALL errors; spec defines granular HTTP-status-based behavior +- Stylesheet: adds undocumented `Shape` selector (functional, shifts specificity values) +- Preamble transform: applied at parse time, not execution time +- No dedicated parallel fan-out/fan-in integration test diff --git a/docs/specs/attractor-spec.md b/docs/specs/attractor-spec.md index 6e49f7034..6b01db86b 100644 --- a/docs/specs/attractor-spec.md +++ b/docs/specs/attractor-spec.md @@ -478,7 +478,7 @@ Each node has a retry policy determined by: 1. Node attribute `max_retries` (if set) -- number of additional attempts beyond the initial execution 2. Graph attribute `default_max_retry` (fallback) -3. Built-in default: 0 (no retries) +3. Built-in default: 50 The `max_retries` attribute specifies additional attempts. So `max_retries=3` means a total of 4 executions (1 initial + 3 retries). Internally this maps to `max_attempts = max_retries + 1`.