diff --git a/docs/public/examples/nlspec-conformance.mdx b/docs/public/examples/nlspec-conformance.mdx index cfeb30fec..6a1161769 100644 --- a/docs/public/examples/nlspec-conformance.mdx +++ b/docs/public/examples/nlspec-conformance.mdx @@ -123,7 +123,7 @@ Do not rewrite working code. Make targeted fixes to the specific failures. ### Max visits as a safety valve -`max_visits=5` on the `fix` node prevents infinite loops. If the agent can't pass in 5 iterations, the workflow moves on with the best result so far. Tune this based on spec complexity: a 30-line spec might need 2 iterations, a 2,000-line spec might need 10. +`max_visits=5` on the `fix` node prevents infinite loops. The node can execute up to 5 times; a sixth visit fails the run rather than looping forever. Tune this based on spec complexity: a 30-line spec might need 2 iterations, a 2,000-line spec might need 10. ### Goal gate on full conformance diff --git a/lib/foundation/fabro-core/src/executor.rs b/lib/foundation/fabro-core/src/executor.rs index 3a94eb332..6701e30be 100644 --- a/lib/foundation/fabro-core/src/executor.rs +++ b/lib/foundation/fabro-core/src/executor.rs @@ -179,8 +179,11 @@ impl Executor { } } - // Check visit limits (>= matches fabro-workflow semantics) - let visits = state.increment_visits(node.id()); + // Check visit limits before entry: a node with a limit of N may + // execute N times, matching the documented contract. The count + // covers previously admitted entries, so the refused visit is + // not reported as one. + let visits = state.visits(node.id()); if let Some(max) = node.max_visits() { if visits >= max { return Err(Error::VisitLimitExceeded { @@ -201,6 +204,7 @@ impl Executor { }); } } + state.increment_visits(node.id()); // before_node lifecycle let node_result = match self.lifecycle.before_node(&node, &state).await? { @@ -807,7 +811,8 @@ mod tests { #[tokio::test] async fn executor_visit_limit_per_node() { - // Node with max_visits=2, loops back — fails on 2nd visit (>= semantics) + // Node with max_visits=2, loops back — executes exactly twice, then + // the third entry is refused. The error reports completed visits. let g = TestGraph::new( vec![ TestNode::new("loop_node").with_max_visits(2), @@ -821,11 +826,20 @@ mod tests { "loop_node", ); let state = ExecutionState::new(&g).unwrap(); + let handler = Arc::new(CountingHandler::new(vec![])); let executor = - ExecutorBuilder::new(Arc::new(AlwaysSucceedHandler) as Arc>) - .build(); + ExecutorBuilder::new(Arc::clone(&handler) as Arc>).build(); let result = executor.run(&g, state).await; - assert!(matches!(result, Err(Error::VisitLimitExceeded { .. }))); + match result { + Err(Error::VisitLimitExceeded { visits, limit, .. }) => { + assert_eq!(visits, 2); + assert_eq!(limit, 2); + } + Err(other) => panic!("expected VisitLimitExceeded, got {other:?}"), + Ok(_) => panic!("expected VisitLimitExceeded, got success"), + } + // Two full loop_node -> other iterations ran before the refusal. + assert_eq!(handler.calls(), 4); } #[tokio::test] diff --git a/lib/foundation/fabro-core/src/state.rs b/lib/foundation/fabro-core/src/state.rs index ca3b8e56b..c44eeeb7d 100644 --- a/lib/foundation/fabro-core/src/state.rs +++ b/lib/foundation/fabro-core/src/state.rs @@ -79,6 +79,10 @@ impl ExecutionState { graph.get_node(&self.current_node_id) } + pub fn visits(&self, node_id: &str) -> usize { + self.node_visits.get(node_id).copied().unwrap_or(0) + } + pub fn increment_visits(&mut self, node_id: &str) -> usize { let count = self.node_visits.entry(node_id.to_string()).or_insert(0); *count += 1;