Sfoglia il codice sorgente

fix: an execution record now says which iteration a node ran in

An execution of the anime pipeline recorded 67 node executions: 52
completed, 3 skipped, and 12 disabled - the 12 being two nodes that never
ran, recorded once per loop iteration. Nothing said which iteration any
record belonged to, and the array came back in an order unrelated to time.
All three made the results panel hard to read, which is where this started.

**Iterations are now named.** A node that runs inside a loop body carries
loopNodeId and loopIteration; a node outside one carries neither, so
absence means "not in a loop" rather than a sentinel. A nested loop tags
with the innermost loop, and the outer iteration is recoverable because
the inner loop node's own record is tagged with the outer.

**Disabled nodes are recorded once per invocation**, not once per
iteration. Recording them at all is deliberate: a node switched off by its
author is worth showing rather than leaving a gap the reader has to
explain.

**Ordering had a root cause worth naming.** node_results is an
unordered_map and was iterated straight into the array, so the order was
hash-bucket order - arbitrary, and stable enough between runs to look
intentional. Records are now sorted by (started_at, seq), seq being a
monotonic counter stamped on each result, which orders nodes that start
within the same millisecond without changing what startedAt means.

Three further bugs surfaced while building this, none of them the task:

- A loop inside another loop's body ran once and never iterated. Nested
  loops were not implemented, only unreported.
- The subgraph walk flattened an inner loop's own loop-output edges into
  the outer body, so those nodes ran twice and the outer loop truncated
  after one pass.
- An internal mirror record - kept so an expression can reference a body
  node by plain id after the loop - was leaking into nodeExecutions as a
  phantom second execution.

Two regression tests cover the tags, the counts and the ordering.

85 passed, 0 failed, 2 skipped.
fszontagh 1 mese fa
parent
commit
8c98b59058

+ 38 - 0
scripts/verify-node.py

@@ -289,6 +289,44 @@ def main():
                     )
             if "output" in want:
                 failures += subset_matches(want["output"], got.get("output"), node_id)
+            if "loopNodeId" in want and got.get("loopNodeId") != want["loopNodeId"]:
+                failures.append(
+                    f"{node_id}: loopNodeId {got.get('loopNodeId')!r}, expected {want['loopNodeId']!r}"
+                )
+            if "loopIteration" in want and got.get("loopIteration") != want["loopIteration"]:
+                failures.append(
+                    f"{node_id}: loopIteration {got.get('loopIteration')!r}, "
+                    f"expected {want['loopIteration']!r}"
+                )
+            if want.get("noLoopTag") and (
+                "loopNodeId" in got or "loopIteration" in got
+            ):
+                failures.append(
+                    f"{node_id}: expected no loop tag, got loopNodeId={got.get('loopNodeId')!r} "
+                    f"loopIteration={got.get('loopIteration')!r}"
+                )
+
+        # How many raw nodeExecutions entries carry a given node id - the
+        # count itself is the assertion for things like "a disabled node
+        # inside a loop body is recorded once, not once per iteration" and
+        # "a loop body node's per-iteration records are not shadowed by a
+        # duplicate mirror entry".
+        all_node_executions = execution.get("nodeExecutions", [])
+        for node_id, want_count in case.get("expectCounts", {}).items():
+            got_count = sum(1 for n in all_node_executions if n.get("nodeId") == node_id)
+            if got_count != want_count:
+                failures.append(
+                    f"{node_id}: {got_count} nodeExecutions entries, expected {want_count}"
+                )
+
+        # nodeExecutions has to come back sorted by startedAt - the array
+        # order is the contract, not something callers are meant to re-sort.
+        if case.get("expectChronological"):
+            starts = [n.get("startedAt", 0) for n in all_node_executions]
+            if starts != sorted(starts):
+                failures.append(
+                    f"nodeExecutions is not in startedAt order: {starts}"
+                )
 
         # What a watcher is told when the run fails. "Loop iteration failed" on
         # its own passed every assertion that looked at nodes, because the nodes

+ 216 - 29
src/runner/workflow_engine.cpp

@@ -239,8 +239,38 @@ nlohmann::json ExecutionResult::toJson() const {
     // would silently reintroduce truncated resume data.
     const bool keep_full_outputs = (status == ExecutionStatus::Waiting);
 
-    j["nodeExecutions"] = nlohmann::json::array();
+    // node_results is an unordered_map, keyed by node id (or "<id>_iter_<n>"
+    // inside a loop) purely so lookups are cheap - its iteration order is
+    // hash-bucket order and has never had anything to do with when nodes
+    // ran. Building the array straight from that iteration is why it used to
+    // come back in an order the caller could not rely on. Sorting by
+    // (startedAt, seq) here fixes that: seq is a monotonic counter stamped
+    // on every result as it is produced, so two nodes that started in the
+    // same millisecond still come out in the order they actually ran, not in
+    // whatever order the hash table happened to visit them.
+    std::vector<const NodeExecutionResult*> ordered;
+    ordered.reserve(node_results.size());
     for (const auto& [id, result] : node_results) {
+        // A loop body node's plain-id entry is a lookup mirror of its last
+        // iteration, kept only so an expression outside the loop can name
+        // that node directly. It is not a second execution and must not be
+        // counted as one.
+        if (result.is_loop_mirror) {
+            continue;
+        }
+        ordered.push_back(&result);
+    }
+    std::sort(ordered.begin(), ordered.end(),
+              [](const NodeExecutionResult* a, const NodeExecutionResult* b) {
+                  if (a->started_at != b->started_at) {
+                      return a->started_at < b->started_at;
+                  }
+                  return a->seq < b->seq;
+              });
+
+    j["nodeExecutions"] = nlohmann::json::array();
+    for (const auto* result_ptr : ordered) {
+        const auto& result = *result_ptr;
         nlohmann::json nr;
         nr["nodeId"] = result.node_id;
         nr["status"] = nodeStatusToString(result.status);
@@ -251,6 +281,20 @@ nlohmann::json ExecutionResult::toJson() const {
         nr["output"] = keep_full_outputs ? result.output : truncateLargeValues(result.output);
         nr["error"] = result.error;
         nr["retryCount"] = result.retry_count;
+        // Absent for a node that did not run inside a loop. Present for one
+        // that did - even a disabled one, recorded once for the whole loop
+        // run rather than once per pass - naming the loop that owns it and,
+        // for a node that actually executed, which 0-based pass produced
+        // this record. A nested loop's body is tagged with the innermost
+        // loop; the outer loop's own pass is recoverable from its own
+        // record in this same array, which is tagged with whatever loop
+        // contains it, in turn.
+        if (result.loop_node_id) {
+            nr["loopNodeId"] = *result.loop_node_id;
+        }
+        if (result.loop_iteration) {
+            nr["loopIteration"] = *result.loop_iteration;
+        }
         j["nodeExecutions"].push_back(nr);
     }
 
@@ -387,6 +431,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
         seeded.status = nodeStatusFromString(it.value().value("status", "completed"));
         seeded.output = it.value().value("output", nlohmann::json::object());
         seeded.started_at = TimeUtils::nowMs();
+        seeded.seq = nextSeq();
         seeded.finished_at = seeded.started_at;
         result.node_results[it.key()] = seeded;
     }
@@ -616,6 +661,23 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 continue;
             }
 
+            // Skip nodes that were already executed (e.g., loop body nodes, or -
+            // on a resume - nodes seeded from the stored execution that ran
+            // before the pause). This is the mechanism that keeps a resume from
+            // re-running work: every seeded node already has a result here.
+            //
+            // This has to come before the disabled check below. A disabled
+            // node inside a loop body is recorded once by the loop itself,
+            // tagged with the loop it belongs to; the topological order still
+            // visits that node's own position afterwards; checking "disabled"
+            // first would have unconditionally overwritten that record with
+            // an untagged one carrying whatever time the outer walk happened
+            // to reach it, which is not when the node was actually reached.
+            if (result.node_results.contains(node_id)) {
+                LOG_DEBUG("Node {} already has results, skipping in main loop", node_id);
+                continue;
+            }
+
             // A disabled node does not run, but what that means downstream
             // depends on what the node does. One that passes work along is
             // simply transparent - the flow carries on without it. One that
@@ -636,6 +698,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     disabled_result.output["_activeBranch"] = "";
                 }
                 disabled_result.started_at = TimeUtils::nowMs();
+                disabled_result.seq = nextSeq();
                 disabled_result.finished_at = disabled_result.started_at;
                 result.node_results[node_id] = disabled_result;
 
@@ -653,15 +716,6 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 continue;
             }
 
-            // Skip nodes that were already executed (e.g., loop body nodes, or -
-            // on a resume - nodes seeded from the stored execution that ran
-            // before the pause). This is the mechanism that keeps a resume from
-            // re-running work: every seeded node already has a result here.
-            if (result.node_results.contains(node_id)) {
-                LOG_DEBUG("Node {} already has results, skipping in main loop", node_id);
-                continue;
-            }
-
             // Skip other trigger nodes when a specific trigger is selected
             auto node_def = registry_.getNode(node->type);
             if (node_def && node_def->is_trigger && node_id != trigger_node_id) {
@@ -671,6 +725,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 skip_result.input = nlohmann::json::object();
                 skip_result.output = nlohmann::json::object();
                 skip_result.started_at = TimeUtils::nowMs();
+                skip_result.seq = nextSeq();
                 skip_result.finished_at = skip_result.started_at;
 
                 result.node_results[node_id] = skip_result;
@@ -704,6 +759,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 skip_result.input = input;
                 skip_result.output = nlohmann::json::object();
                 skip_result.started_at = TimeUtils::nowMs();
+                skip_result.seq = nextSeq();
                 skip_result.finished_at = skip_result.started_at;
 
                 result.node_results[node_id] = skip_result;
@@ -756,6 +812,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 node_result.input = input;
                 node_result.output = cached_outputs[node_id]["output"];
                 node_result.started_at = TimeUtils::nowMs();
+                node_result.seq = nextSeq();
                 node_result.finished_at = node_result.started_at;
                 node_result.from_cache = true;
 
@@ -801,6 +858,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     node_result.output = nlohmann::json::object();
                     node_result.error = overlay_conflict;
                     node_result.started_at = TimeUtils::nowMs();
+                    node_result.seq = nextSeq();
                     node_result.finished_at = node_result.started_at;
                 } else if (!t_disabled_reference_error.empty()) {
                     node_result.node_id = node_id;
@@ -809,6 +867,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     node_result.output = nlohmann::json::object();
                     node_result.error = t_disabled_reference_error;
                     node_result.started_at = TimeUtils::nowMs();
+                    node_result.seq = nextSeq();
                     node_result.finished_at = node_result.started_at;
                 } else {
                     node_result = executeNode(evaluated_node, input, result.execution_id, workflow,
@@ -885,19 +944,18 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     // "Loop iteration failed" on its own says nothing, and it is
                     // what an error workflow forwards to whoever is watching -
                     // so the node that actually failed and what it said are
-                    // carried out with it. The results are keyed
-                    // "<node>_iter_<n>", and the first failure is the one that
-                    // matters; the rest are usually the same thing repeated.
+                    // carried out with it. loop_node_id/loop_iteration (rather
+                    // than parsing the "<node>_iter_<n>" map key) are what make
+                    // this correct for a node inside a nested loop too, where
+                    // the key itself carries the outer loop's own prefix as
+                    // well and cannot be split apart unambiguously.
                     std::string detail;
                     for (const auto& [key, body_result] : result.node_results) {
-                        if (body_result.status != NodeStatus::Failed) {
+                        if (body_result.status != NodeStatus::Failed ||
+                            !body_result.loop_iteration) {
                             continue;
                         }
-                        const auto suffix = key.find("_iter_");
-                        if (suffix == std::string::npos) {
-                            continue;
-                        }
-                        const std::string body_node_id = key.substr(0, suffix);
+                        const std::string& body_node_id = body_result.node_id;
                         std::string body_name = body_node_id;
                         for (const auto& n : workflow.nodes) {
                             if (n.id == body_node_id && !n.name.empty()) {
@@ -906,7 +964,7 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                             }
                         }
                         detail = "Loop body node \"" + body_name + "\" failed on item " +
-                                 key.substr(suffix + 6) + ": " + body_result.error;
+                                 std::to_string(*body_result.loop_iteration) + ": " + body_result.error;
                         break;
                     }
 
@@ -1353,6 +1411,7 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
     result.node_id = node.id;
     result.input = input;
     result.started_at = TimeUtils::nowMs();
+    result.seq = nextSeq();
     result.status = NodeStatus::Running;
 
     // Get node definition - reuse the caller's lookup if it already made one
@@ -2481,7 +2540,8 @@ bool WorkflowEngine::executeLoopBody(
     ExecutionResult& result,
     ExecutionCallback callback,
     const std::string& target_node_id,
-    const nlohmann::json& cached_outputs) {
+    const nlohmann::json& cached_outputs,
+    const std::string& key_prefix) {
 
     // Find nodes connected to "loop" output
     auto body_start_nodes = findLoopBodyNodes(loop_node_id, workflow);
@@ -2515,11 +2575,34 @@ bool WorkflowEngine::executeLoopBody(
 
         body_nodes_set.insert(node_id);
 
+        // A nested loop node belongs to this body - it has to, or it would
+        // never run and never recurse - but what it feeds through its "loop"
+        // output is its own body, discovered separately when the recursive
+        // call below reaches it. Following those edges here as well as there
+        // would fold the inner loop's body into the outer one, so both walks
+        // execute it: once correctly, through the recursion, and once more
+        // directly, on whatever stale input the outer body last gave it.
+        // Only "done" - what happens after the inner loop finishes - is
+        // genuinely a continuation of the outer body.
+        bool node_is_nested_loop = false;
+        if (node_id != loop_node_id) {
+            for (const auto& n : workflow.nodes) {
+                if (n.id == node_id && n.type == "loop") {
+                    node_is_nested_loop = true;
+                    break;
+                }
+            }
+        }
+
         // Find downstream nodes
         for (const auto& conn : workflow.connections) {
-            if (conn.source_node_id == node_id) {
-                to_process.push(conn.target_node_id);
+            if (conn.source_node_id != node_id) {
+                continue;
+            }
+            if (node_is_nested_loop && conn.source_output != "done") {
+                continue;
             }
+            to_process.push(conn.target_node_id);
         }
     }
 
@@ -2740,10 +2823,25 @@ bool WorkflowEngine::executeLoopBody(
                     disabled_result.output["_activeBranch"] = "";
                 }
                 disabled_result.started_at = TimeUtils::nowMs();
+                disabled_result.seq = nextSeq();
                 disabled_result.finished_at = disabled_result.started_at;
+                // Named for the loop it sits in, but not for a particular pass -
+                // it never runs, so no pass produced it.
+                disabled_result.loop_node_id = loop_node_id;
 
                 iteration_results[body_node_id] = disabled_result;
-                result.node_results[body_node_id + "_iter_" + std::to_string(i)] = disabled_result;
+
+                // A node turned off never runs, so it never produces a new
+                // record: one entry per iteration said "the busiest thing in
+                // the workflow" about a node that did nothing sixty times.
+                // But leaving it out entirely would erase a decision someone
+                // made on purpose, so it is recorded once for the whole loop -
+                // on the first iteration that reaches it - rather than once
+                // per pass or not at all.
+                const std::string disabled_key = key_prefix + body_node_id;
+                if (!result.node_results.contains(disabled_key)) {
+                    result.node_results[disabled_key] = disabled_result;
+                }
 
                 // This returns early, before the back-edge check further down,
                 // so the same rule has to be applied here. Everywhere else a
@@ -2866,15 +2964,18 @@ bool WorkflowEngine::executeLoopBody(
                     cached_result.input = node_input;
                     cached_result.output = cache_entry["output"];
                     cached_result.started_at = TimeUtils::nowMs();
+                    cached_result.seq = nextSeq();
                     cached_result.finished_at = cached_result.started_at;
                     cached_result.from_cache = true;
+                    cached_result.loop_node_id = loop_node_id;
+                    cached_result.loop_iteration = static_cast<int>(i);
 
                     // A cached output carries the same markers a fresh one
                     // does, and a replayed run has to stop where the original
                     // did - so it goes through the same handling.
                     const MarkerOutcome cached_outcome = applyNodeMarkers(
                         cached_result, body_node_id,
-                        body_node_id + "_iter_" + std::to_string(i), result,
+                        key_prefix + body_node_id + "_iter_" + std::to_string(i), result,
                         nlohmann::json::object(), {}, static_cast<int>(i));
 
                     iteration_results[body_node_id] = cached_result;
@@ -2973,6 +3074,7 @@ bool WorkflowEngine::executeLoopBody(
                 body_result.output = nlohmann::json::object();
                 body_result.error = body_overlay_conflict;
                 body_result.started_at = TimeUtils::nowMs();
+                body_result.seq = nextSeq();
                 body_result.finished_at = body_result.started_at;
             } else if (!t_disabled_reference_error.empty()) {
                 body_result.node_id = body_node_id;
@@ -2981,19 +3083,79 @@ bool WorkflowEngine::executeLoopBody(
                 body_result.output = nlohmann::json::object();
                 body_result.error = t_disabled_reference_error;
                 body_result.started_at = TimeUtils::nowMs();
+                body_result.seq = nextSeq();
                 body_result.finished_at = body_result.started_at;
             } else {
                 body_result = executeNode(evaluated_body_node, node_input, result.execution_id, workflow,
                                            body_node_def_for_defaults);
             }
 
+            // Named for the loop that runs it and which pass this is - the
+            // same tag a node outside a loop never carries at all.
+            body_result.loop_node_id = loop_node_id;
+            body_result.loop_iteration = static_cast<int>(i);
+
+            // A body node that is itself a loop node does not iterate on its
+            // own - it only ever produces the "_isLoop" marker once, same as
+            // at the top level, and something has to act on that marker or a
+            // loop nested inside another loop's body would silently run its
+            // "body" zero times. This is that recursion: the inner loop's
+            // own pass is this outer iteration's body_result, tagged above
+            // with the outer loop's id and iteration like any other body
+            // node; everything the inner loop runs is tagged with the inner
+            // loop's id and its own 0-based iteration instead, and the key
+            // prefix keeps those results from colliding with the same inner
+            // loop's run under a different outer iteration.
+            bool inner_loop_left_early = false;
+            if (body_result.status == NodeStatus::Completed &&
+                body_result.output.contains("_isLoop") &&
+                body_result.output["_isLoop"].get<bool>()) {
+
+                LoopContext inner_ctx;
+                for (const auto& item : body_result.output["_items"]) {
+                    inner_ctx.items.push_back(item);
+                }
+                inner_ctx.output_field = body_result.output.value("_outputField", "results");
+                inner_ctx.item_variable = body_result.output.value("_itemVariable", "item");
+                inner_ctx.index_variable = body_result.output.value("_indexVariable", "index");
+                inner_ctx.continue_on_error = body_result.output.value("_continueOnError", true);
+
+                const std::string inner_key_prefix =
+                    key_prefix + body_node_id + "_iter_" + std::to_string(i) + "_";
+                const bool inner_success = executeLoopBody(
+                    body_node_id, workflow, inner_ctx, result, callback,
+                    target_node_id, cached_outputs, inner_key_prefix);
+
+                body_result.output["_loopCompleted"] = true;
+                body_result.output[inner_ctx.output_field] = inner_ctx.results;
+                body_result.output["_activeBranch"] = "done";
+
+                nlohmann::json inner_done_data;
+                inner_done_data[inner_ctx.output_field] = inner_ctx.results;
+                inner_done_data["totalProcessed"] = inner_ctx.results.size();
+                inner_done_data["data"] = body_result.output.value("data", nlohmann::json::object());
+                body_result.output["done"] = inner_done_data;
+
+                if (!inner_success && !inner_ctx.continue_on_error) {
+                    inner_loop_left_early = true;
+                }
+            }
+
             // Store in main results with iteration suffix
-            const std::string result_key = body_node_id + "_iter_" + std::to_string(i);
+            const std::string result_key = key_prefix + body_node_id + "_iter_" + std::to_string(i);
 
             const MarkerOutcome body_outcome = applyNodeMarkers(
                 body_result, body_node_id, result_key, result,
                 body_overlay, body_overlay_ignored, static_cast<int>(i));
 
+            if (inner_loop_left_early) {
+                iteration_failed = true;
+                all_succeeded = false;
+                if (!ctx.continue_on_error) {
+                    break;
+                }
+            }
+
             // Only a node that actually ran counts. A node on a branch that was
             // not taken is skipped, and a skipped loop-back edge is exactly the
             // case that should end the iteration.
@@ -3101,14 +3263,38 @@ bool WorkflowEngine::executeLoopBody(
     // Copy results back (ctx was passed by const ref, but we used a mutable copy)
     const_cast<LoopContext&>(loop_ctx).results = ctx.results;
 
-    // Store base results for body nodes so main loop will skip them
-    // (main loop checks result.node_results.contains(node_id))
+    // Store a lookup mirror for every body node under its own plain, never-
+    // prefixed id - never as one of the caller's own results (an expression
+    // outside the loop naming this node directly has to find something), and
+    // always under the same key an enclosing walk checks with
+    // "result.node_results.contains(node_id)" before deciding whether to run
+    // it. That check is what keeps a node from running twice; for a loop
+    // nested inside another loop's body the enclosing walk is itself
+    // recursive and has no idea how deep this one went, so the mirror has to
+    // be reachable by plain id regardless of nesting - key_prefix is only
+    // ever for the real per-iteration records, which do need to stay apart
+    // between one outer pass's run of this loop and the next's.
     for (const auto& body_node_id : sorted_body) {
+        const std::string disabled_key = key_prefix + body_node_id;
+        auto disabled_it = result.node_results.find(disabled_key);
+        if (disabled_it != result.node_results.end() &&
+            disabled_it->second.status == NodeStatus::Disabled) {
+            // At the top level disabled_key already equals body_node_id -
+            // that one real record IS the plain-id entry, and must stay
+            // visible rather than being relabelled a mirror of itself.
+            if (!key_prefix.empty()) {
+                NodeExecutionResult mirror = disabled_it->second;
+                mirror.is_loop_mirror = true;
+                result.node_results[body_node_id] = mirror;
+            }
+            continue;
+        }
+
         // Find the most recent iteration result for this node
         NodeExecutionResult base_result;
         bool found = false;
         for (size_t i = ctx.items.size(); i > 0; --i) {
-            std::string iter_key = body_node_id + "_iter_" + std::to_string(i - 1);
+            std::string iter_key = key_prefix + body_node_id + "_iter_" + std::to_string(i - 1);
             auto it = result.node_results.find(iter_key);
             if (it != result.node_results.end()) {
                 base_result = it->second;
@@ -3119,6 +3305,7 @@ bool WorkflowEngine::executeLoopBody(
         if (found) {
             // Mark as executed in loop so main loop skips it
             base_result.output["_executedInLoop"] = true;
+            base_result.is_loop_mirror = true;
             result.node_results[body_node_id] = base_result;
         }
     }

+ 38 - 1
src/runner/workflow_engine.hpp

@@ -80,6 +80,30 @@ struct NodeExecutionResult {
     int64_t finished_at = 0;
     int retry_count = 0;
     bool from_cache = false;  // True if output was from cached previous execution
+
+    // Which loop this record was produced inside, and which pass of it. Set
+    // only for a node that actually ran (or, for a disabled node, was skipped)
+    // as part of a loop body; a node outside any loop leaves both unset, and
+    // that absence - not a sentinel like -1 - is what "not in a loop" means.
+    // For nested loops this names the innermost loop that owns the record;
+    // the outer loop's own iteration is recoverable from its own record,
+    // which is itself tagged with the loop that contains it, chained all the
+    // way out. See ExecutionResult::toJson for how this is serialized.
+    std::optional<std::string> loop_node_id;
+    std::optional<int> loop_iteration;
+
+    // A monotonic counter stamped on every result as it is produced, used
+    // only to break ties when two nodes share a startedAt millisecond. Never
+    // serialized - see the ordering note on ExecutionResult::toJson.
+    uint64_t seq = 0;
+
+    // True for the internal copy executeLoopBody leaves under a loop body
+    // node's plain id once the loop finishes, so an expression outside the
+    // loop that names that node directly (rather than going through the
+    // loop's own "done" output) still resolves to its last iteration. It is
+    // bookkeeping for lookups, not a second time the node ran - never
+    // serialized, and never emitted as a nodeExecution entry.
+    bool is_loop_mirror = false;
 };
 
 // Execution status
@@ -317,6 +341,14 @@ private:
 
     void storeExecution(const ExecutionResult& result);
 
+    // A process-lifetime counter stamped on every NodeExecutionResult as it
+    // is produced. node_results is an unordered_map, so nothing about
+    // insertion order survives into it; this is what lets toJson recover
+    // that order deterministically even when two nodes share a startedAt
+    // millisecond.
+    uint64_t nextSeq() { return next_seq_.fetch_add(1, std::memory_order_relaxed); }
+    std::atomic<uint64_t> next_seq_{0};
+
     // Loop execution support
     struct LoopContext {
         std::vector<nlohmann::json> items;
@@ -355,13 +387,18 @@ private:
                                    const std::vector<std::string>& overlay_ignored,
                                    int iteration);
 
+    // key_prefix disambiguates result_key across nested loops: a loop node
+    // used inside another loop's body runs once per outer iteration, and
+    // without a prefix its own body nodes would reuse the same
+    // "<id>_iter_<n>" key on every outer pass, each overwriting the last.
     bool executeLoopBody(const std::string& loop_node_id,
                         const Workflow& workflow,
                         const LoopContext& loop_ctx,
                         ExecutionResult& result,
                         ExecutionCallback callback,
                         const std::string& target_node_id = "",
-                        const nlohmann::json& cached_outputs = nlohmann::json::object());
+                        const nlohmann::json& cached_outputs = nlohmann::json::object(),
+                        const std::string& key_prefix = "");
 
     std::vector<std::string> findLoopBodyNodes(const std::string& loop_node_id,
                                                const Workflow& workflow);

+ 47 - 0
tests/nodes/loop-nested-record.json

@@ -0,0 +1,47 @@
+{
+  "name": "verify-loop-nested-record",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "outerItems", "name": "OuterItems", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { items: ['x', 'y'] };"}},
+    {"id": "outerLoop", "name": "OuterLoop", "type": "loop", "position": {"x": 0, "y": 200},
+     "config": {"inputField": "data.result.items", "continueOnError": true}},
+    {"id": "innerItems", "name": "InnerItems", "type": "code", "position": {"x": 0, "y": 300},
+     "config": {"code": "return { items: [1, 2, 3] };"}},
+    {"id": "innerLoop", "name": "InnerLoop", "type": "loop", "position": {"x": 0, "y": 400},
+     "config": {"inputField": "data.result.items", "continueOnError": true}},
+    {"id": "innerWork", "name": "InnerWork", "type": "set-fields", "position": {"x": 0, "y": 500},
+     "config": {"mode": "only-set", "fields": [{"name": "seen", "value": "{{loop.item}}"}]}},
+    {"id": "afterInner", "name": "AfterInner", "type": "code", "position": {"x": 0, "y": 600},
+     "config": {"code": "return { innerDone: true };"}},
+    {"id": "afterOuter", "name": "AfterOuter", "type": "code", "position": {"x": 0, "y": 700},
+     "config": {"code": "return { outerDone: true };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "outerItems", "targetInput": "data"},
+    {"sourceNodeId": "outerItems", "sourceOutput": "main", "targetNodeId": "outerLoop", "targetInput": "data"},
+    {"sourceNodeId": "outerLoop", "sourceOutput": "loop", "targetNodeId": "innerItems", "targetInput": "data"},
+    {"sourceNodeId": "innerItems", "sourceOutput": "main", "targetNodeId": "innerLoop", "targetInput": "data"},
+    {"sourceNodeId": "innerLoop", "sourceOutput": "loop", "targetNodeId": "innerWork", "targetInput": "data"},
+    {"sourceNodeId": "innerWork", "sourceOutput": "main", "targetNodeId": "innerLoop", "targetInput": "data"},
+    {"sourceNodeId": "innerLoop", "sourceOutput": "done", "targetNodeId": "afterInner", "targetInput": "data"},
+    {"sourceNodeId": "afterInner", "sourceOutput": "main", "targetNodeId": "outerLoop", "targetInput": "data"},
+    {"sourceNodeId": "outerLoop", "sourceOutput": "done", "targetNodeId": "afterOuter", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "outerLoop": {"status": "completed", "noLoopTag": true},
+    "afterOuter": {"status": "completed", "noLoopTag": true},
+    "innerItems": {"status": "completed", "loopNodeId": "outerLoop", "loopIteration": 1},
+    "innerLoop": {"status": "completed", "loopNodeId": "outerLoop", "loopIteration": 1},
+    "afterInner": {"status": "completed", "loopNodeId": "outerLoop", "loopIteration": 1},
+    "innerWork": {"status": "completed", "loopNodeId": "innerLoop", "loopIteration": 2}
+  },
+  "expectCounts": {
+    "innerItems": 2,
+    "innerLoop": 2,
+    "afterInner": 2,
+    "innerWork": 6
+  },
+  "expectChronological": true
+}

+ 46 - 0
tests/nodes/loop-record-tags-and-order.json

@@ -0,0 +1,46 @@
+{
+  "name": "verify-loop-record-tags-and-order",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "items", "name": "Items", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { items: ['a', 'b', 'c'] };"}},
+    {"id": "loop", "name": "Loop", "type": "loop", "position": {"x": 0, "y": 200},
+     "config": {"inputField": "data.result.items", "continueOnError": true}},
+    {"id": "work", "name": "Work", "type": "set-fields", "position": {"x": 0, "y": 300},
+     "config": {"mode": "only-set", "fields": [{"name": "seen", "value": "{{loop.item}}"}]}},
+    {"id": "work2", "name": "Work2", "type": "set-fields", "position": {"x": 0, "y": 350},
+     "config": {"mode": "only-set", "fields": [{"name": "seen2", "value": "{{loop.item}}-2"}]}},
+    {"id": "notify", "name": "Notify", "type": "set-fields", "position": {"x": 0, "y": 400},
+     "disabled": true,
+     "config": {"mode": "only-set", "fields": [{"name": "told", "value": "yes"}]}},
+    {"id": "outside", "name": "OutsideNode", "type": "code", "position": {"x": 0, "y": 500},
+     "config": {"code": "return { standalone: true };"}},
+    {"id": "after", "name": "After", "type": "code", "position": {"x": 0, "y": 520},
+     "config": {"code": "return { done: true };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "items", "targetInput": "data"},
+    {"sourceNodeId": "items", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "loop", "targetNodeId": "work", "targetInput": "data"},
+    {"sourceNodeId": "work", "sourceOutput": "main", "targetNodeId": "work2", "targetInput": "data"},
+    {"sourceNodeId": "work2", "sourceOutput": "main", "targetNodeId": "notify", "targetInput": "data"},
+    {"sourceNodeId": "notify", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "done", "targetNodeId": "after", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "outside", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "loop": {"status": "completed", "noLoopTag": true},
+    "outside": {"status": "completed", "noLoopTag": true},
+    "after": {"status": "completed", "noLoopTag": true},
+    "work": {"status": "completed", "loopNodeId": "loop", "loopIteration": 2},
+    "work2": {"status": "completed", "loopNodeId": "loop", "loopIteration": 2},
+    "notify": {"status": "disabled", "loopNodeId": "loop", "loopIteration": null}
+  },
+  "expectCounts": {
+    "work": 3,
+    "work2": 3,
+    "notify": 1
+  },
+  "expectChronological": true
+}