From bf63ba8b70d86546e0eeeb962a87ef779eccc633 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Tue, 24 Mar 2026 16:40:34 -0400
Subject: [PATCH] Clarify visit count semantics

---
 lib/crates/fabro-core/src/context.rs          |  4 ++++
 lib/crates/fabro-workflows/src/engine.rs      | 23 +++++++++++++++++--
 .../src/handler/manager_loop.rs               |  6 ++++-
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/lib/crates/fabro-core/src/context.rs b/lib/crates/fabro-core/src/context.rs
index 3e93d2244..133e01728 100644
--- a/lib/crates/fabro-core/src/context.rs
+++ b/lib/crates/fabro-core/src/context.rs
@@ -57,6 +57,10 @@ impl Context {
         self.get_string("current_node", "")
     }
 
+    /// Returns the raw stored node visit count.
+    ///
+    /// This is `0` when the workflow lifecycle has not yet seeded
+    /// `internal.node_visit_count` into the context.
     pub fn node_visit_count(&self) -> usize {
         self.get("internal.node_visit_count")
             .and_then(|v| v.as_u64())
diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs
index 24ab1554f..b2d39eda7 100644
--- a/lib/crates/fabro-workflows/src/engine.rs
+++ b/lib/crates/fabro-workflows/src/engine.rs
@@ -288,9 +288,12 @@ pub fn node_dir(run_dir: &Path, node_id: &str, visit: usize) -> PathBuf {
     }
 }
 
-/// Read the visit count from context, defaulting to 1 if not set.
+/// Read the workflow visit ordinal from context.
+///
+/// The raw context value is `0` when unset; workflow execution code treats
+/// missing counts as the first visit for stage/log naming.
 pub fn visit_from_context(context: &Context) -> usize {
-    context.node_visit_count()
+    context.node_visit_count().max(1)
 }
 
 /// Write status.json for a completed node into {`run_dir}/nodes/{node_id}/status.json`.
@@ -2791,6 +2794,22 @@ mod tests {
         assert_eq!(policy.backoff.factor, 1.0);
     }
 
+    #[test]
+    fn visit_from_context_defaults_to_first_visit() {
+        let ctx = Context::new();
+        assert_eq!(visit_from_context(&ctx), 1);
+    }
+
+    #[test]
+    fn visit_from_context_preserves_stored_visit() {
+        let ctx = Context::new();
+        ctx.set(
+            crate::context::keys::INTERNAL_NODE_VISIT_COUNT,
+            serde_json::json!(3),
+        );
+        assert_eq!(visit_from_context(&ctx), 3);
+    }
+
     #[test]
     fn retry_policy_patient() {
         let policy = RetryPolicy::patient();
diff --git a/lib/crates/fabro-workflows/src/handler/manager_loop.rs b/lib/crates/fabro-workflows/src/handler/manager_loop.rs
index 0d0f7d568..519c087f6 100644
--- a/lib/crates/fabro-workflows/src/handler/manager_loop.rs
+++ b/lib/crates/fabro-workflows/src/handler/manager_loop.rs
@@ -128,7 +128,7 @@ impl Handler for SubWorkflowHandler {
         };
 
         // Build child RunConfig
-        let visit = context.node_visit_count() as u64;
+        let visit = crate::engine::visit_from_context(context) as u64;
         let child_logs = run_dir.join(format!("nodes/{}_{visit}/child", node.id));
         let _ = std::fs::create_dir_all(&child_logs);
 
@@ -300,6 +300,10 @@ mod tests {
             .as_deref()
             .unwrap()
             .contains("Child completed"));
+        assert!(
+            dir.path().join("nodes/manager_1/child").exists(),
+            "child logs should default to first-visit directory naming"
+        );
     }
 
     #[tokio::test]