Skip to content

Commit 6db7c0f

Browse files
eamiriclaude
authored andcommitted
DYN-10572: Honor per-port Use Levels on variadic DSVarArgFunction nodes
Variadic zero-touch functions (String.Concat, String.Join, List.Join, ...) pack their variadic inputs into a single ExprList argument before the function call. AtLevel / replication-guide annotations inside that list literal are inert at runtime, and DesignScript cannot call a params method with separate arguments, so per-port Use Levels and lacing were silently dropped on every variadic port. When any port has Use Levels enabled, route the node through a generated wrapper function whose formal parameters are the individual ports. The per-port AtLevel and the node's lacing replication guides land on the wrapper-call arguments -- replicating exactly like a non-variadic node -- and the wrapper body packs the already-replicated values per invocation. Graphs with no Use Levels keep the original pack-then-call path, so their behavior is unchanged. This replaces the earlier pre-pack approach (and its NodeModel LevelAndReplicationGuideInputCount hook), which wrapped inputs that were then re-buried inside the inert packed list literal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8c02603 commit 6db7c0f

6 files changed

Lines changed: 110 additions & 141 deletions

File tree

src/DynamoCore/Graph/Nodes/NodeModel.cs

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,15 +1643,7 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
16431643
if (inputs == null || !inputs.Any())
16441644
return;
16451645

1646-
// Variadic nodes pack tail inputs into a single argument, so per-port
1647-
// level/lacing handling for the variadic slots must happen before packing.
1648-
// The hook lets such nodes scope this post-pack pass to the non-variadic
1649-
// prefix, preventing double-wrap.
1650-
int boundedCount = Math.Min(inputs.Count, LevelAndReplicationGuideInputCount(inputs.Count));
1651-
if (boundedCount <= 0)
1652-
return;
1653-
1654-
for (int i = 0; i < boundedCount; i++)
1646+
for (int i = 0; i < inputs.Count; i++)
16551647
{
16561648
if (InPorts[i].UseLevels)
16571649
{
@@ -1662,15 +1654,15 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
16621654
switch (ArgumentLacing)
16631655
{
16641656
case LacingStrategy.Auto:
1665-
for (int i = 0; i < boundedCount; ++i)
1657+
for (int i = 0; i < inputs.Count(); ++i)
16661658
{
16671659
if (InPorts[i].UseLevels)
16681660
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, false);
16691661
}
16701662
break;
16711663

16721664
case LacingStrategy.Shortest:
1673-
for (int i = 0; i < boundedCount; ++i)
1665+
for (int i = 0; i < inputs.Count(); ++i)
16741666
{
16751667
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, false);
16761668
}
@@ -1679,7 +1671,7 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
16791671

16801672
case LacingStrategy.Longest:
16811673

1682-
for (int i = 0; i < boundedCount; ++i)
1674+
for (int i = 0; i < inputs.Count(); ++i)
16831675
{
16841676
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { 1 }, true);
16851677
}
@@ -1688,29 +1680,14 @@ public void UseLevelAndReplicationGuide(List<AssociativeNode> inputs)
16881680
case LacingStrategy.CrossProduct:
16891681

16901682
int guide = 1;
1691-
for (int i = 0; i < boundedCount; ++i)
1683+
for (int i = 0; i < inputs.Count(); ++i)
16921684
{
16931685
inputs[i] = AstFactory.AddReplicationGuide(inputs[i], new List<int> { guide }, false);
16941686
guide++;
16951687
}
16961688
break;
16971689
}
16981690
}
1699-
1700-
/// <summary>
1701-
/// Returns the number of leading inputs that <see cref="UseLevelAndReplicationGuide"/>
1702-
/// should wrap with <c>AtLevel</c> / replication guides.
1703-
/// </summary>
1704-
/// <remarks>
1705-
/// Variadic nodes (e.g. <c>DSVarArgFunction</c>) pack their tail inputs into a single
1706-
/// packed argument before this post-pack pass runs. Wrapping the packed slot here would
1707-
/// be either a no-op or a double-wrap, so variadic nodes override this hook to expose only
1708-
/// the non-variadic prefix and handle per-port level / lacing on the unpacked variadic
1709-
/// inputs themselves.
1710-
/// </remarks>
1711-
/// <param name="rawInputCount">Number of raw input AST nodes available to the pass.</param>
1712-
/// <returns>Number of inputs (counted from index 0) that should be wrapped.</returns>
1713-
protected virtual int LevelAndReplicationGuideInputCount(int rawInputCount) => rawInputCount;
17141691
#endregion
17151692

17161693
#region Input and Output Connections

src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs

Lines changed: 93 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Dynamo.Library;
88
using Newtonsoft.Json;
99
using ProtoCore.AST.AssociativeAST;
10+
using ProtoCore.Utils;
1011

1112
namespace Dynamo.Graph.Nodes.ZeroTouch
1213
{
@@ -99,25 +100,6 @@ internal override bool HandleModelEventCore(string eventName, int value, UndoRed
99100
[JsonIgnore]
100101
public VariableInputNodeController VarInputController { get; private set; }
101102

102-
/// <summary>
103-
/// Scopes <see cref="NodeModel.UseLevelAndReplicationGuide"/> to the non-variadic
104-
/// prefix of this node's inputs.
105-
/// </summary>
106-
/// <remarks>
107-
/// Variadic zero-touch functions pack their tail inputs into a single
108-
/// <c>ExprListNode</c> argument before <see cref="NodeModel.UseLevelAndReplicationGuide"/>
109-
/// runs. AtLevel / replication-guide annotations on an <c>ExprListNode</c> are inert
110-
/// at runtime, so per-port Use Levels and lacing on the variadic slots must be applied
111-
/// pre-pack inside <c>ZeroTouchVarArgNodeController.BuildOutputAst</c>. This override
112-
/// limits the post-pack pass to the non-variadic prefix so the packed variadic slot
113-
/// is not touched twice.
114-
/// </remarks>
115-
protected override int LevelAndReplicationGuideInputCount(int rawInputCount)
116-
{
117-
int prefix = Controller.Definition.Parameters.Count() - 1;
118-
return prefix < 0 ? 0 : prefix;
119-
}
120-
121103
#region VarInput Controller
122104
private sealed class ZeroTouchVarInputController : VariableInputNodeController
123105
{
@@ -185,99 +167,109 @@ protected override void InitializeFunctionParameters(NodeModel model, IEnumerabl
185167

186168
protected override void BuildOutputAst(NodeModel model, List<AssociativeNode> inputAstNodes, List<AssociativeNode> resultAst)
187169
{
188-
// All inputs are provided, then we should pack all inputs that
189-
// belong to variable input parameter into a single array.
170+
// A variadic zero-touch function foo(x1, ..., xn, params y) is invoked at
171+
// runtime as foo(x1, ..., xn, {y1, ..., ym}) -- every variadic input is
172+
// packed into a single array argument. Suppose paramCount == n + 1 and the
173+
// node inputs are i1, ..., in, y1, ..., ym.
190174
if (!model.IsPartiallyApplied)
191175
{
192-
var paramCount = Definition.Parameters.Count();
176+
int variadicStart = Definition.Parameters.Count() - 1;
193177

194-
// Suppose a function foo() with var args, its signature is:
195-
//
196-
// foo(x1, x2, ..., xn, params y)
197-
//
198-
// so paramCount == n + 1 here, and suppose inputs are
199-
//
200-
// i1, i2, ...., in, y1, y2, ..., ym
201-
//
202-
// Why: per-port Use Levels and lacing-driven replication guides
203-
// on the variadic inputs (y1..ym) must be applied *before* we
204-
// pack them into an ExprListNode. AtLevel / replication-guide
205-
// annotations carried inside an ExprListNode are inert at runtime,
206-
// so packing first would silently drop per-port settings on every
207-
// variadic port (the original DYN-10572 defect). The non-variadic
208-
// prefix (i1..in) is handled by the post-pack
209-
// NodeModel.UseLevelAndReplicationGuide pass, which DSVarArgFunction
210-
// scopes to indices [0, paramCount - 1) via the
211-
// LevelAndReplicationGuideInputCount override -- preventing the
212-
// packed slot from being wrapped twice.
213-
int variadicStart = paramCount - 1;
214-
if (variadicStart >= 0 && variadicStart < inputAstNodes.Count)
178+
// When any port uses levels, per-port replication must be honored. AtLevel /
179+
// replication-guide annotations are only consumed at function-call argument
180+
// positions and are inert inside the packed ExprList literal, and DesignScript
181+
// cannot call a params method with separate arguments. So we route through a
182+
// generated wrapper whose formal parameters are the individual ports: the
183+
// per-port AtLevel and the node's lacing replication guides land on the
184+
// wrapper-call arguments (replicating exactly like a non-variadic node), and
185+
// the wrapper body packs the already-replicated values per invocation. See
186+
// DYN-10572. Graphs with no Use Levels keep the original pack-then-call path,
187+
// so their behavior (including their lacing) is unchanged.
188+
if (variadicStart >= 0 && variadicStart < inputAstNodes.Count
189+
&& AnyPortUsesLevels(model, inputAstNodes.Count)
190+
&& TryBuildReplicatingOutputAst(model, inputAstNodes, resultAst, variadicStart))
215191
{
216-
for (int i = variadicStart; i < inputAstNodes.Count; i++)
217-
{
218-
if (model.InPorts[i].UseLevels)
219-
{
220-
inputAstNodes[i] = AstFactory.AddAtLevel(
221-
inputAstNodes[i],
222-
-model.InPorts[i].Level,
223-
model.InPorts[i].KeepListStructure);
224-
}
225-
}
192+
return;
193+
}
226194

227-
switch (model.ArgumentLacing)
228-
{
229-
case LacingStrategy.Auto:
230-
for (int i = variadicStart; i < inputAstNodes.Count; i++)
231-
{
232-
if (model.InPorts[i].UseLevels)
233-
{
234-
inputAstNodes[i] = AstFactory.AddReplicationGuide(
235-
inputAstNodes[i], new List<int> { 1 }, false);
236-
}
237-
}
238-
break;
195+
// Default path: pack all var arguments into an array {y1, ..., ym}
196+
// (skipping the first n == paramCount - 1 inputs).
197+
var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(variadicStart).ToList());
198+
inputAstNodes = inputAstNodes.Take(variadicStart).ToList();
199+
inputAstNodes.Add(argPack);
200+
}
239201

240-
case LacingStrategy.Shortest:
241-
for (int i = variadicStart; i < inputAstNodes.Count; i++)
242-
{
243-
inputAstNodes[i] = AstFactory.AddReplicationGuide(
244-
inputAstNodes[i], new List<int> { 1 }, false);
245-
}
246-
break;
202+
base.BuildOutputAst(model, inputAstNodes, resultAst);
203+
}
247204

248-
case LacingStrategy.Longest:
249-
for (int i = variadicStart; i < inputAstNodes.Count; i++)
250-
{
251-
inputAstNodes[i] = AstFactory.AddReplicationGuide(
252-
inputAstNodes[i], new List<int> { 1 }, true);
253-
}
254-
break;
205+
/// <summary>
206+
/// Returns true when any port has Use Levels enabled. Those settings cannot survive
207+
/// the default pack-then-call path (they are inert inside the packed ExprList), so the
208+
/// node must route through the per-port replicating wrapper instead.
209+
/// </summary>
210+
private static bool AnyPortUsesLevels(NodeModel model, int inputCount)
211+
{
212+
for (int i = 0; i < inputCount; i++)
213+
{
214+
if (model.InPorts[i].UseLevels)
215+
return true;
216+
}
217+
return false;
218+
}
255219

256-
case LacingStrategy.CrossProduct:
257-
// Continue cross-product indices from where the prefix
258-
// pass would have stopped: the prefix uses guides
259-
// 1..paramCount-1, so the first variadic port starts at
260-
// paramCount and each subsequent variadic port gets a
261-
// distinct rank.
262-
int guide = paramCount;
263-
for (int i = variadicStart; i < inputAstNodes.Count; i++)
264-
{
265-
inputAstNodes[i] = AstFactory.AddReplicationGuide(
266-
inputAstNodes[i], new List<int> { guide }, false);
267-
guide++;
268-
}
269-
break;
270-
}
271-
}
220+
/// <summary>
221+
/// Emits a generated wrapper function that exposes every port as a separate formal
222+
/// parameter and packs the variadic tail per invocation, then calls it with per-port
223+
/// Use Levels and lacing replication guides applied to the individual arguments.
224+
/// Returns false (leaving <paramref name="resultAst"/> untouched) if the wrapper
225+
/// source cannot be parsed, so the caller falls back to the default pack path.
226+
/// </summary>
227+
private bool TryBuildReplicatingOutputAst(
228+
NodeModel model, List<AssociativeNode> inputAstNodes, List<AssociativeNode> resultAst, int variadicStart)
229+
{
230+
// Build the call to the underlying function exactly as GetFunctionApplication
231+
// would: Class.Function for member functions, the bare name for globals.
232+
string callName = Definition.Type == FunctionType.GenericFunction
233+
? Definition.FunctionName
234+
: Definition.ClassName + "." + Definition.FunctionName;
272235

273-
// Here we pack all var arguments in an array {y1, y2, ..., ym}
274-
// (skipping the first n == paramCount - 1 inputs)
275-
var argPack = AstFactory.BuildExprList(inputAstNodes.Skip(paramCount - 1).ToList());
276-
inputAstNodes = inputAstNodes.Take(paramCount - 1).ToList();
277-
inputAstNodes.Add(argPack);
236+
// Unique, deterministic per node + arity so repeated runs redefine the same
237+
// wrapper (cache-stable) and distinct nodes never collide.
238+
string wrapperName = "__vararg_" + model.GUID.ToString("N") + "_" + inputAstNodes.Count;
239+
240+
var paramNames = Enumerable.Range(0, inputAstNodes.Count).Select(i => "p" + i).ToList();
241+
var prefixArgs = paramNames.Take(variadicStart);
242+
var packedArg = "[" + string.Join(", ", paramNames.Skip(variadicStart)) + "]";
243+
var innerArgs = string.Join(", ", prefixArgs.Append(packedArg));
244+
245+
string wrapperSource =
246+
"def " + wrapperName + "(" + string.Join(", ", paramNames) + ")" +
247+
" { return = " + callName + "(" + innerArgs + "); }";
248+
249+
FunctionDefinitionNode wrapperDef;
250+
try
251+
{
252+
wrapperDef = ParserUtils.Parse(wrapperSource).Body
253+
.OfType<FunctionDefinitionNode>()
254+
.FirstOrDefault();
255+
}
256+
catch
257+
{
258+
return false;
278259
}
279260

280-
base.BuildOutputAst(model, inputAstNodes, resultAst);
261+
if (wrapperDef == null)
262+
return false;
263+
264+
resultAst.Add(wrapperDef);
265+
266+
// Per-port AtLevel / lacing replication guides land on the wrapper-call
267+
// arguments, where they are honored just as on a non-variadic node.
268+
model.UseLevelAndReplicationGuide(inputAstNodes);
269+
270+
var rhs = AstFactory.BuildFunctionCall(wrapperName, inputAstNodes);
271+
AssignIdentifiersForFunctionCall(model, rhs, resultAst);
272+
return true;
281273
}
282274
}
283275
}

src/DynamoCore/PublicAPI.Unshipped.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,3 @@ Dynamo.Models.DynamoModel.DefaultStartConfiguration.EnableUnTrustedLocationsNoti
66
Dynamo.Models.DynamoModel.IStartConfiguration.EnableUnTrustedLocationsNotifications.get -> bool
77
Dynamo.Models.DynamoModel.OpenFileCommand.OpenFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean isTemplate, System.Boolean forceBlockRun) -> void
88
Dynamo.Models.DynamoModel.InsertFileCommand.InsertFileCommand(System.String filePath, System.Boolean forceManualExecutionMode, System.Boolean forceBlockRun) -> void
9-
virtual Dynamo.Graph.Nodes.NodeModel.LevelAndReplicationGuideInputCount(int rawInputCount) -> int

test/DynamoCoreTests/Nodes/ListTests.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,17 @@ public void TestConcatenateListsNormalInput()
109109
public void TestListJoinUseLevelsOnVariadicPort()
110110
{
111111
// DYN-10572: List.Join is a pure-variadic DSVarArgFunction (no non-variadic
112-
// prefix). With [IsLacingDisabled] on the C# method the node's lacing is
113-
// Disabled, so per-port Use Levels must still be honored pre-pack even
114-
// though no replication guides are applied. This test confirms the
115-
// variadic Use Levels path runs without breaking the post-pack pass
116-
// (which is a no-op here because LevelAndReplicationGuideInputCount is 0).
112+
// prefix). Use Levels on a variadic port (list1 = [3,4] @L1) is now honored:
113+
// the per-port AtLevel replicates the join at level 1, exactly as it would on
114+
// a non-variadic node, independently of the other port. Before the fix the
115+
// level was silently dropped and the result was the flat [1,2,3,4]; honoring
116+
// it replicates element-wise to [[1,3],[2,4]].
117117
string testFilePath = Path.Combine(listTestFolder, "TestListJoinUseLevelsOnVariadicPort.dyn");
118118

119119
RunModel(testFilePath);
120120

121-
AssertPreviewValue("dddd11110000222200003333dddd4444", new object[] { 1, 2, 3, 4 });
121+
AssertPreviewValue("dddd11110000222200003333dddd4444",
122+
new object[] { new object[] { 1, 3 }, new object[] { 2, 4 } });
122123
}
123124

124125
#endregion

test/DynamoCoreTests/Nodes/StringTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ public void TestJoinStringNormalInput()
230230
public void TestJoinStringUseLevelsOnVariadicPort()
231231
{
232232
// DYN-10572: String.Join has both a non-variadic prefix port (separator)
233-
// and variadic ports. Set UseLevels on the separator port to exercise the
234-
// bounded post-pack pass (LevelAndReplicationGuideInputCount == 1) AND on
235-
// a variadic port to exercise the new pre-pack handling, confirming both
236-
// code paths cooperate correctly.
233+
// and variadic ports. The separator (prefix port) broadcasts unchanged while
234+
// Use Levels on a variadic port (string1 = ["a","b"] @L1) replicates the join,
235+
// confirming the prefix port passes through correctly and per-port Use Levels
236+
// is honored on the variadic ports.
237237
string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestJoinStringUseLevelsOnVariadicPort.dyn");
238238

239239
RunModel(testFilePath);

test/core/string/TestJoinStringUseLevelsOnVariadicPort.dyn

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
"Description": "String used to separate strings.\n\nstring",
8383
"UsingDefaultValue": false,
8484
"Level": 1,
85-
"UseLevels": true,
85+
"UseLevels": false,
8686
"KeepListStructure": false
8787
},
8888
{

0 commit comments

Comments
 (0)