From 6e3b8f7da6645b1e3573c7bcf98ed7d59284ac0c Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 09:30:39 -0400 Subject: [PATCH 1/7] feat: begin implementation for #8473 From eca1b5b8b58900f5d452893a266a6d4612aab7cd Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 09:46:08 -0400 Subject: [PATCH 2/7] test: DYN-8473 add failing test reproducing String.Concat nested-list coupling Adds TestConcatStringNestedListInputIsIndependentOfScalarInput to capture the bug where a nested list connected to one input of String.Concat dictates the structure of every subsequent input. The test is [Ignore]'d for now and will be re-enabled once the AST fix lands in TASK 2. --- test/DynamoCoreTests/Nodes/StringTests.cs | 54 +++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/DynamoCoreTests/Nodes/StringTests.cs b/test/DynamoCoreTests/Nodes/StringTests.cs index 701604f6c0b..32a8eaa3390 100644 --- a/test/DynamoCoreTests/Nodes/StringTests.cs +++ b/test/DynamoCoreTests/Nodes/StringTests.cs @@ -7,6 +7,7 @@ using Dynamo.Graph.Nodes.ZeroTouch; using Dynamo.Models; using NUnit.Framework; +using DynCmd = Dynamo.Models.DynamoModel; namespace Dynamo.Tests { @@ -99,6 +100,59 @@ public void TestConcatStringInListMap() AssertPreviewValue("a105ad39-9b1c-44aa-a2cb-37866ea48dd0", new string[] { "0a", "10a", "20a", "30a", "40a", "50a" }); } + // DYN-8473: When a nested list is connected to one input of a String.Concat, + // each subsequent input should remain independent — a scalar second input + // should replicate against the nested first input rather than being packed + // into the same params array. The current implementation packs all ports + // into one string[] argument, so the nested structure of port 0 controls + // how ports 1..N are interpreted. Enable once TASK 2 lands the fix. + [Test] + [Ignore("DYN-8473: pending list-level fix for String.Concat (enabled by TASK 2)")] + public void TestConcatStringNestedListInputIsIndependentOfScalarInput() + { + // Build the graph programmatically: a String.Concat with two ports, a code block + // feeding a nested list of strings into port 0, and a code block feeding a single + // scalar string into port 1. + var stringConcat = new DSVarArgFunction( + CurrentDynamoModel.LibraryServices.GetFunctionDescriptor("DSCore.String.Concat@string[]")); + CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(stringConcat, 0, 0, true, false)); + // String.Concat defaults to a single input port; add a second to mimic the repro graph. + CurrentDynamoModel.ExecuteCommand(new DynCmd.ModelEventCommand(stringConcat.GUID, "AddInPort", 1)); + Assert.AreEqual(2, stringConcat.InPorts.Count); + + var nestedCbn = new CodeBlockNodeModel(CurrentDynamoModel.LibraryServices); + CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(nestedCbn, 0, 0, true, false)); + CurrentDynamoModel.ExecuteCommand( + new DynCmd.UpdateModelValueCommand(System.Guid.Empty, nestedCbn.GUID, "Code", "{{\"a\",\"b\"},{\"c\",\"d\"}};")); + + var scalarCbn = new CodeBlockNodeModel(CurrentDynamoModel.LibraryServices); + CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(scalarCbn, 0, 0, true, false)); + CurrentDynamoModel.ExecuteCommand( + new DynCmd.UpdateModelValueCommand(System.Guid.Empty, scalarCbn.GUID, "Code", "\"X\";")); + + CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( + nestedCbn.GUID, 0, PortType.Output, DynCmd.MakeConnectionCommand.Mode.Begin)); + CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( + stringConcat.GUID, 0, PortType.Input, DynCmd.MakeConnectionCommand.Mode.End)); + + CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( + scalarCbn.GUID, 0, PortType.Output, DynCmd.MakeConnectionCommand.Mode.Begin)); + CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( + stringConcat.GUID, 1, PortType.Input, DynCmd.MakeConnectionCommand.Mode.End)); + + RunCurrentModel(); + + // Expected: scalar input replicates against the nested-list input so each leaf is + // concatenated with "X", preserving the nested-list shape and keeping ports + // independent of each other. + AssertPreviewValue(stringConcat.GUID.ToString(), + new object[] + { + new[] { "aX", "bX" }, + new[] { "cX", "dX" } + }); + } + #endregion #region substring test cases From b72b8e5f498753396483d1c613ab72da4f0d20cb Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 10:15:13 -0400 Subject: [PATCH 3/7] feat: begin implementation for #8473 From 78c168e6d9e8d7206e51c715204ae4c44c636892 Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 10:50:11 -0400 Subject: [PATCH 4/7] feat: begin implementation for #8473 From 0ac48b8d9fe1c92c9d6ca46bd864cd08916ceb85 Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 11:37:05 -0400 Subject: [PATCH 5/7] feat: begin implementation for #8473 From 11d037f4d0362d1908bc8f00b23f63d8ef8b3431 Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 11:43:05 -0400 Subject: [PATCH 6/7] feat: DYN-8473 lower String.Concat to chain of binary + for per-input replication Special-cases DSCore.String.Concat@string[] in ZeroTouchVarArgNodeController.BuildOutputAst so that, when the node has >=2 inputs and is fully applied, the AST is lowered to a left-associative chain of binary `+` operator calls (s0 + s1 + s2 + ...). Each operand participates in Dynamo's normal per-argument replication, so a nested- list input no longer dictates the structure of subsequent inputs and list-level overrides work as users expect (DYN-8473). Tests updated to reflect the new, correct behaviour: - TestConcatStringMultipleInput now pairs parallel flat lists element-wise ({"abef","cdgh"}) instead of packing them into a single string[] argument. - TestConcatStringInvalidInput now asserts the coerced "a1" preview that DesignScript's `+` operator produces from (string, int) rather than the old node-level warning. - TestConcatStringNestedListInputIsIndependentOfScalarInput is re-enabled and now loads test/core/string/TestConcatString_nestedList.dyn instead of building the graph programmatically. --- .../Graph/Nodes/ZeroTouch/DSVarArgFunction.cs | 30 ++++++++ test/DynamoCoreTests/Nodes/StringTests.cs | 75 ++++++------------- .../string/TestConcatString_nestedList.dyn | 15 ++++ 3 files changed, 69 insertions(+), 51 deletions(-) create mode 100644 test/core/string/TestConcatString_nestedList.dyn diff --git a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs index 9e7cf288be4..3a27f4dc446 100644 --- a/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs +++ b/src/DynamoCore/Graph/Nodes/ZeroTouch/DSVarArgFunction.cs @@ -7,6 +7,7 @@ using Dynamo.Library; using Newtonsoft.Json; using ProtoCore.AST.AssociativeAST; +using ProtoCore.DSASM; namespace Dynamo.Graph.Nodes.ZeroTouch { @@ -164,8 +165,37 @@ protected override void InitializeFunctionParameters(NodeModel model, IEnumerabl } } + // Mangled name of DSCore.String.Concat. Treated specially so that each input + // port participates in Dynamo's natural per-input replication rather than being + // packed into a single string[] whose first element dictates how the rest of the + // inputs are interpreted. See DYN-8473. + private const string StringConcatMangledName = "DSCore.String.Concat@string[]"; + protected override void BuildOutputAst(NodeModel model, List inputAstNodes, List resultAst) { + if (!model.IsPartiallyApplied + && Definition.MangledName == StringConcatMangledName + && inputAstNodes.Count >= 2) + { + // For String.Concat with multiple ports, build a chain of binary + // string-concatenation operators (s0 + s1 + s2 + ...) so each port + // participates in Dynamo's normal replication independently. The + // operator is emitted as a call to its internal function (matching + // the way the DesignScript parser lowers infix `+`), so the engine + // routes through the usual op-function dispatcher. + var addOpFunction = Op.GetOpFunction(Operator.add); + AssociativeNode chain = inputAstNodes[0]; + for (int i = 1; i < inputAstNodes.Count; i++) + { + chain = AstFactory.BuildFunctionCall( + addOpFunction, + new List { chain, inputAstNodes[i] }); + } + + AssignIdentifiersForFunctionCall(model, chain, resultAst); + return; + } + // All inputs are provided, then we should pack all inputs that // belong to variable input parameter into a single array. if (!model.IsPartiallyApplied) diff --git a/test/DynamoCoreTests/Nodes/StringTests.cs b/test/DynamoCoreTests/Nodes/StringTests.cs index 32a8eaa3390..389aec8c744 100644 --- a/test/DynamoCoreTests/Nodes/StringTests.cs +++ b/test/DynamoCoreTests/Nodes/StringTests.cs @@ -2,12 +2,9 @@ using System.IO; using System.Linq; using CoreNodeModels; -using Dynamo.Configuration; using Dynamo.Graph.Nodes; using Dynamo.Graph.Nodes.ZeroTouch; -using Dynamo.Models; using NUnit.Framework; -using DynCmd = Dynamo.Models.DynamoModel; namespace Dynamo.Tests { @@ -66,20 +63,26 @@ public void TestConcatStringFunctionInput() AssertPreviewValue("8c7c1a80-021b-4064-b9d1-873a0538bb0b", "yesterday today.tomorrow"); } + // DYN-8473: String.Concat now lowers to a chain of binary `+` operations. + // DesignScript's overloaded `+` accepts a (string, int) pair and coerces the + // integer, so the node concatenates "a" and 1 into "a1" rather than raising a + // node-level warning as the old `params string[]` path used to. [Test] public void TestConcatStringInvalidInput() { - string testFilePath = Path.Combine(localDynamoStringTestFolder, + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_invalidInput.dyn"); RunModel(testFilePath); - var stringConcat = CurrentDynamoModel.CurrentWorkspace.NodeFromWorkspace - ("eb4d8a34-5437-4064-ad52-db5c58a95451"); - Assert.AreEqual(ElementState.Warning, stringConcat.State); - + AssertPreviewValue("eb4d8a34-5437-4064-ad52-db5c58a95451", "a1"); } + // DYN-8473: Each input port participates in Dynamo's per-input replication + // independently. Two parallel flat lists ({"ab","cd"} and {"ef","gh"}) now + // pair element-wise via the chain-of-`+` AST, producing {"abef","cdgh"}. + // Prior to the fix, the inputs were packed into a single string[] which + // caused per-port replication ({"abcd","efgh"}) — the bug behaviour. [Test] public void TestConcatStringMultipleInput() { @@ -87,7 +90,7 @@ public void TestConcatStringMultipleInput() RunModel(testFilePath); - AssertPreviewValue("fbc947fb-460b-49b9-8460-b223bffb63d5", new string[] { "abcd", "efgh" }); + AssertPreviewValue("fbc947fb-460b-49b9-8460-b223bffb63d5", new string[] { "abef", "cdgh" }); } [Test] @@ -101,51 +104,21 @@ public void TestConcatStringInListMap() } // DYN-8473: When a nested list is connected to one input of a String.Concat, - // each subsequent input should remain independent — a scalar second input - // should replicate against the nested first input rather than being packed - // into the same params array. The current implementation packs all ports - // into one string[] argument, so the nested structure of port 0 controls - // how ports 1..N are interpreted. Enable once TASK 2 lands the fix. + // each subsequent input remains independent — a scalar second input replicates + // against the nested first input rather than being packed into the same params + // array. [Test] - [Ignore("DYN-8473: pending list-level fix for String.Concat (enabled by TASK 2)")] public void TestConcatStringNestedListInputIsIndependentOfScalarInput() { - // Build the graph programmatically: a String.Concat with two ports, a code block - // feeding a nested list of strings into port 0, and a code block feeding a single - // scalar string into port 1. - var stringConcat = new DSVarArgFunction( - CurrentDynamoModel.LibraryServices.GetFunctionDescriptor("DSCore.String.Concat@string[]")); - CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(stringConcat, 0, 0, true, false)); - // String.Concat defaults to a single input port; add a second to mimic the repro graph. - CurrentDynamoModel.ExecuteCommand(new DynCmd.ModelEventCommand(stringConcat.GUID, "AddInPort", 1)); - Assert.AreEqual(2, stringConcat.InPorts.Count); - - var nestedCbn = new CodeBlockNodeModel(CurrentDynamoModel.LibraryServices); - CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(nestedCbn, 0, 0, true, false)); - CurrentDynamoModel.ExecuteCommand( - new DynCmd.UpdateModelValueCommand(System.Guid.Empty, nestedCbn.GUID, "Code", "{{\"a\",\"b\"},{\"c\",\"d\"}};")); - - var scalarCbn = new CodeBlockNodeModel(CurrentDynamoModel.LibraryServices); - CurrentDynamoModel.ExecuteCommand(new DynCmd.CreateNodeCommand(scalarCbn, 0, 0, true, false)); - CurrentDynamoModel.ExecuteCommand( - new DynCmd.UpdateModelValueCommand(System.Guid.Empty, scalarCbn.GUID, "Code", "\"X\";")); - - CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( - nestedCbn.GUID, 0, PortType.Output, DynCmd.MakeConnectionCommand.Mode.Begin)); - CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( - stringConcat.GUID, 0, PortType.Input, DynCmd.MakeConnectionCommand.Mode.End)); - - CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( - scalarCbn.GUID, 0, PortType.Output, DynCmd.MakeConnectionCommand.Mode.Begin)); - CurrentDynamoModel.ExecuteCommand(new DynCmd.MakeConnectionCommand( - stringConcat.GUID, 1, PortType.Input, DynCmd.MakeConnectionCommand.Mode.End)); - - RunCurrentModel(); - - // Expected: scalar input replicates against the nested-list input so each leaf is - // concatenated with "X", preserving the nested-list shape and keeping ports - // independent of each other. - AssertPreviewValue(stringConcat.GUID.ToString(), + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_nestedList.dyn"); + + RunModel(testFilePath); + + // Port 0: {{"a","b"},{"c","d"}} (nested 2D list of strings) + // Port 1: "X" (scalar string) + // Expected: the scalar replicates against every leaf of the nested list, + // preserving the nested shape and keeping ports independent of each other. + AssertPreviewValue("3a9b8f01-1111-2222-3333-444455556666", new object[] { new[] { "aX", "bX" }, diff --git a/test/core/string/TestConcatString_nestedList.dyn b/test/core/string/TestConcatString_nestedList.dyn new file mode 100644 index 00000000000..e91975fd2f6 --- /dev/null +++ b/test/core/string/TestConcatString_nestedList.dyn @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + From 4839cc8f2857ab1cbc7e459a8ccbcfc749f58e20 Mon Sep 17 00:00:00 2001 From: Neal Burnham Date: Tue, 9 Jun 2026 11:59:35 -0400 Subject: [PATCH 7/7] feat: DYN-8473 rename String.Concat variadic ports to list0/list1/... MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renames the `params string[]` parameter on `DSCore.String.Concat` from `strings` to `lists` so the variadic input ports surface as `list0`, `list1`, ... — matching the user request in DYN-8473 that the labels reflect that each port can accept either a string or a list of strings (replication happens per-port). The label derivation in ZeroTouchVarInputController/ZeroTouchVarArgNodeController is parameter- name-based, so the rename is the only code change required. The mangled name (`DSCore.String.Concat@string[]`) is type-based and unchanged, so the binary-`+` lowering special case from TASK 2 still matches and existing graphs resolve the same function. VariableInputNode serialization only persists `inputcount` / port GUIDs+state — port Name is not restored from disk — so old `.dyn` graphs serialized with `string0/string1` labels load cleanly with fresh `list0/list1` labels and all wires remain connected. Adds `TestConcatStringPortsAreNamedListN` asserting the first two ports of a loaded String.Concat node are `list0` and `list1`. Co-Authored-By: Claude Opus 4.7 --- src/Libraries/CoreNodes/String.cs | 12 +++++++----- test/DynamoCoreTests/Nodes/StringTests.cs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/Libraries/CoreNodes/String.cs b/src/Libraries/CoreNodes/String.cs index 42a10df5b33..54f98d99eb0 100644 --- a/src/Libraries/CoreNodes/String.cs +++ b/src/Libraries/CoreNodes/String.cs @@ -50,14 +50,16 @@ public static string GetNumber(string @string) return sb.ToString(); } /// - /// Concatenates multiple strings into a single string. + /// Concatenates multiple strings into a single string. Each input port participates + /// in Dynamo's normal per-input replication, so list levels are honoured and the + /// shape of one input does not constrain the others. /// - /// List of strings to concatenate. - /// String made from list of strings. + /// A string or list of strings to concatenate. + /// String made from the joined inputs. /// concatenate,join,combine strings - public static string Concat(params string[] strings) + public static string Concat(params string[] lists) { - return string.Concat(strings); + return string.Concat(lists); } /// diff --git a/test/DynamoCoreTests/Nodes/StringTests.cs b/test/DynamoCoreTests/Nodes/StringTests.cs index 389aec8c744..4c599f4dac9 100644 --- a/test/DynamoCoreTests/Nodes/StringTests.cs +++ b/test/DynamoCoreTests/Nodes/StringTests.cs @@ -126,6 +126,25 @@ public void TestConcatStringNestedListInputIsIndependentOfScalarInput() }); } + // DYN-8473: Variadic String.Concat ports should be labelled list0, list1, ... + // (derived from the renamed `lists` parameter) instead of the older + // string0/string1 scheme that users reported as confusing. + [Test] + public void TestConcatStringPortsAreNamedListN() + { + string testFilePath = Path.Combine(localDynamoStringTestFolder, "TestConcatString_nestedList.dyn"); + + OpenModel(testFilePath); + + var concatNode = CurrentDynamoModel.CurrentWorkspace.NodeFromWorkspace( + "3a9b8f01-1111-2222-3333-444455556666"); + + Assert.IsNotNull(concatNode, "Expected String.Concat node in fixture."); + Assert.GreaterOrEqual(concatNode.InPorts.Count, 2, "Fixture has at least two variadic inputs."); + Assert.AreEqual("list0", concatNode.InPorts[0].Name); + Assert.AreEqual("list1", concatNode.InPorts[1].Name); + } + #endregion #region substring test cases