Ver código fonte

feat: give a loop body real, addressable iteration data

data.loop.item was never a data path. The engine rewrote the expression
string before evaluating it, substituting a bare "item" identifier
(workflow_engine.cpp, simplifyLoopVariablePaths). Nothing named "loop"
existed in the data, so no outputSchema could describe it and no tooling
could offer it - which matters now that the editor suggests field paths
from those schemas.

The engine now attaches a real "loop" object to each body node's input:
item, index, currentItem, currentIndex, totalItems, isFirst, isLast. It is
a genuine nested value, addressable like anything else, and named the same
way regardless of what the item and index variables were called.

The textual rewrite stays, labelled as the compatibility shim it is, since
live workflows spell it the old way.

**It also fixes a limitation we had written down and lived with.** The
loop item only ever reached the node wired directly to the loop; every
later node in the body received its predecessor's output and nothing else,
so both spellings went dark after the first step. The iteration context is
now carried onto every body node, merged in a way that only fills keys the
node does not already have, so a real upstream value is never overwritten.

Verified on a two-node body over two iterations:

  iteration 1   first:  legacy=alpha  new=alpha  index=0
                second: legacy=alpha  new=alpha  isLast=false
  iteration 2   first:  legacy=beta   new=beta   index=1
                second: legacy=beta   new=beta   isLast=true

The second node seeing the item at all is the part that used to be
impossible. 83 passed, 0 failed, 2 skipped - the skips are sdcpp cases
gated on a model being loaded.
fszontagh 1 mês atrás
pai
commit
019c799946
1 arquivos alterados com 51 adições e 2 exclusões
  1. 51 2
      src/runner/workflow_engine.cpp

+ 51 - 2
src/runner/workflow_engine.cpp

@@ -2694,6 +2694,20 @@ bool WorkflowEngine::executeLoopBody(
         // Add currentItem/currentIndex to match loop.js node output structure
         loop_input["currentItem"] = ctx.items[i];
         loop_input["currentIndex"] = i;
+        // Real, addressable per-iteration data: a genuine nested object rather
+        // than the textual "data.loop.item" rewrite an expression used to need.
+        // Canonical keys regardless of a custom item/index variable name, so a
+        // schema or a field-path picker can describe "loop.item" as one fixed
+        // path that always exists.
+        loop_input["loop"] = {
+            {"item", ctx.items[i]},
+            {"index", i},
+            {"currentItem", ctx.items[i]},
+            {"currentIndex", i},
+            {"totalItems", ctx.items.size()},
+            {"isFirst", i == 0},
+            {"isLast", i == ctx.items.size() - 1}
+        };
 
         bool iteration_failed = false;
 
@@ -2803,6 +2817,25 @@ bool WorkflowEngine::executeLoopBody(
                 }
             }
 
+            // The loop item only used to reach the node wired directly to the
+            // loop node: every later body node received nothing but its
+            // predecessor's output, so both the real "loop.item" path and the
+            // legacy "data.loop.item" rewrite went dark after the first step.
+            // Carrying the iteration's loop context onto every body node - not
+            // just the first - fixes that for both addressing styles at once,
+            // without touching what a node's own upstream data looks like: the
+            // merge only fills in keys the node doesn't already have, so a
+            // real "data" (or other target_input) from a previous body node is
+            // never overwritten.
+            if (!has_loop_input && node_input.is_object()) {
+                for (const auto& [loop_key, loop_value] : loop_input.items()) {
+                    if (loop_key == "data") continue;
+                    if (!node_input.contains(loop_key)) {
+                        node_input[loop_key] = loop_value;
+                    }
+                }
+            }
+
             // A node fed by other body nodes runs only when one of them actually
             // produced something this iteration. Anything else means its branch
             // was not taken, or the node before it was itself skipped - and the
@@ -3311,9 +3344,17 @@ static std::string convertReservedWordAccess(const std::string& expression) {
     return result;
 }
 
-// Helper function to simplify loop variable paths
+// Legacy compatibility shim: rewrites the pre-existing "data.loop.item"
+// convention into the bare "item" identifier before evaluation.
 // e.g., "data.loop.item.hasAttachments" -> "item.hasAttachments"
-// This allows drag & drop expressions from the UI to work seamlessly
+//
+// This is purely textual - it does not consult any real object path, and
+// "data.loop" is not a nested field that actually exists anywhere in the
+// data an expression sees. It survives only because live workflows already
+// depend on the "data.loop.item" spelling; new work should address the loop
+// item through the real "loop" object the engine now attaches to every loop
+// body node's input (const loop = input.loop, below), which is a genuine
+// nested JSON value and does not need this rewrite.
 static std::string simplifyLoopVariablePaths(const std::string& expression) {
     // Loop variables that should be simplified
     static const std::vector<std::string> loop_vars = {
@@ -3424,6 +3465,14 @@ nlohmann::json WorkflowEngine::evaluateJavaScriptExpression(
             const totalItems = input.totalItems;
             const isFirst = input.isFirst;
             const isLast = input.isLast;
+            // Real, addressable per-iteration data. input.loop is a genuine
+            // nested object the engine attaches to every loop body node's
+            // input (see executeLoopBody), not a string rewrite - so
+            // "loop.item", "loop.index", etc. resolve as ordinary property
+            // access. The legacy "data.loop.item" text substitution above
+            // still runs first and keeps producing the bare "item" form for
+            // expressions written before this existed.
+            const loop = input.loop || {};
 
             return )" + safe_expression + R"(;
         }