Let a node execute max_visits times before the cycle guard fires

The executor incremented a node's visit count on entry and refused the
visit once the count reached the limit, so a node with max_visits=N
executed at most N-1 times. The documented contract in
stages-and-nodes.mdx is "Max times this node can execute in a run",
and both published examples describe bounded retry loops under that
reading. A graph with max_visits=2 on a designed
one-correction loop therefore failed as "stuck in a cycle" before the
correction could run.

Check the completed-visit count before entry instead: a node with
max_visits=N now executes exactly N times, and the refused entry is
not reported as a visit, so the error's count names the executions
that actually happened. Also correct the nlspec example prose, which
claimed the workflow "moves on with the best result" at the limit;
exceeding max_visits fails the run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-08-05 21:01:53 -04:00
parent 0abf2297c0
commit f6932529fa
No known key found for this signature in database
3 changed files with 25 additions and 7 deletions

View file

@ -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

View file

@ -179,8 +179,11 @@ impl<G: Graph + 'static> Executor<G> {
}
}
// 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 completed entries only, 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<G: Graph + 'static> Executor<G> {
});
}
}
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<dyn NodeHandler<TestGraph>>)
.build();
ExecutorBuilder::new(Arc::clone(&handler) as Arc<dyn NodeHandler<TestGraph>>).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]

View file

@ -79,6 +79,10 @@ impl<M: OutcomeMeta> ExecutionState<M> {
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;