Merge pull request #1946 from glopesdev/issue-1792

Exclude build dependencies when counting inputs to new nested node
bonsai-rx · Aug 7, 2024 · 70520d9 · 70520d9
2 parents 5f8e06e + 7a875e3
commit 70520d9
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 5 deletions.
diff --git a/Bonsai.Core.Tests/TestWorkflow.cs b/Bonsai.Core.Tests/TestWorkflow.cs
@@ -12,16 +12,22 @@ public TestWorkflow()
         {
         }
 
-        private TestWorkflow(ExpressionBuilderGraph workflow, Node<ExpressionBuilder, ExpressionBuilderArgument> cursor)
+        private TestWorkflow(
+            ExpressionBuilderGraph workflow,
+            Node<ExpressionBuilder, ExpressionBuilderArgument> cursor,
+            int argumentIndex = 0)
         {
             Workflow = workflow ?? throw new ArgumentNullException(nameof(workflow));
+            ArgumentIndex = argumentIndex;
             Cursor = cursor;
         }
 
         public ExpressionBuilderGraph Workflow { get; }
 
         public Node<ExpressionBuilder, ExpressionBuilderArgument> Cursor { get; }
 
+        public int ArgumentIndex { get; }
+
         public TestWorkflow ResetCursor()
         {
             if (Cursor != null)
@@ -41,7 +47,17 @@ public TestWorkflow Append(ExpressionBuilder builder)
             var node = Workflow.Add(builder);
             if (Cursor != null)
                 Workflow.AddEdge(Cursor, node, new ExpressionBuilderArgument());
-            return new TestWorkflow(Workflow, node);
+            return new TestWorkflow(Workflow, node, argumentIndex: 1);
+        }
+
+        public TestWorkflow AddArguments(params TestWorkflow[] arguments)
+        {
+            var argumentIndex = ArgumentIndex;
+            for (int i = 0; i < arguments.Length; i++)
+            {
+                Workflow.AddEdge(arguments[i].Cursor, Cursor, new ExpressionBuilderArgument(argumentIndex++));
+            }
+            return new TestWorkflow(Workflow, Cursor, argumentIndex);
         }
 
         public TestWorkflow AppendCombinator<TCombinator>(TCombinator combinator) where TCombinator : new()
@@ -81,6 +97,11 @@ public TestWorkflow AppendPropertyMapping(params string[] propertyNames)
             return Append(mappingBuilder);
         }
 
+        public TestWorkflow AppendBranch(Func<TestWorkflow, TestWorkflow> selector)
+        {
+            return selector(this);
+        }
+
         public TestWorkflow AppendNested<TWorkflowExpressionBuilder>(
             Func<TestWorkflow, TestWorkflow> selector,
             Func<ExpressionBuilderGraph, TWorkflowExpressionBuilder> constructor)

diff --git a/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj b/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj
@@ -15,6 +15,7 @@
     </PackageReference>
   </ItemGroup>
   <ItemGroup>
+    <ProjectReference Include="..\Bonsai.Core.Tests\Bonsai.Core.Tests.csproj" />
     <ProjectReference Include="..\Bonsai.Editor\Bonsai.Editor.csproj" />
   </ItemGroup>
 </Project>
diff --git a/Bonsai.Editor.Tests/WorkflowEditorTests.cs b/Bonsai.Editor.Tests/WorkflowEditorTests.cs
@@ -3,6 +3,7 @@
 using System.IO;
 using System.Linq;
 using System.Xml;
+using Bonsai.Core.Tests;
 using Bonsai.Dag;
 using Bonsai.Editor.GraphModel;
 using Bonsai.Expressions;
@@ -47,6 +48,12 @@ static string ToString(IEnumerable<T> sequence)
             ExpressionBuilderGraph workflow = null,
             MockGraphView graphView = null)
         {
+            if (workflow != null)
+            {
+                // Workflows must be topologically sorted to ensure all editor operations are reversible
+                workflow.InsertRange(0, workflow.TopologicalSort());
+            }
+
             graphView ??= new MockGraphView(workflow);
             var editor = new WorkflowEditor(graphView.ServiceProvider, graphView);
             editor.UpdateLayout.Subscribe(graphView.UpdateGraphLayout);
@@ -219,6 +226,35 @@ public void CreateAnnotation_EmptySelection_InsertAfterClosestRoot()
             Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode(annotationBuilder).Index);
             assertIsReversible();
         }
+
+        [TestMethod]
+        public void ReplaceGraphNode_SingleInputWithVisualizerMapping_GroupWorkflowHasSingleSourceNode()
+        {
+            // related to https://github.com/bonsai-rx/bonsai/issues/1792
+            var workflow = new TestWorkflow()
+                .AppendValue(0)
+                .AppendBranch(source => source
+                    .AppendSubject<Reactive.PublishSubject>("P")
+                    .AddArguments(source.Append(new VisualizerMappingBuilder())))
+                .Workflow
+                .ToInspectableGraph();
+
+            var (editor, assertIsReversible) = CreateMockEditor(workflow);
+            var targetNode = editor.FindNode("P");
+            editor.ReplaceGraphNode(
+                targetNode,
+                typeof(GroupWorkflowBuilder).AssemblyQualifiedName,
+                ElementCategory.Nested,
+                arguments: "N");
+            Assert.AreEqual(expected: 3, workflow.Count);
+
+            var groupNode = editor.FindNode("N");
+            var groupBuilder = ExpressionBuilder.Unwrap(groupNode?.Value) as GroupWorkflowBuilder;
+            Assert.IsInstanceOfType(groupBuilder, typeof(GroupWorkflowBuilder));
+            Assert.AreEqual(expected: 2, groupBuilder.Workflow.Count);
+            Assert.IsInstanceOfType(ExpressionBuilder.Unwrap(groupBuilder.Workflow[1].Value), typeof(WorkflowOutputBuilder));
+            assertIsReversible();
+        }
     }
 
     static class WorkflowEditorHelper

diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs
@@ -1114,9 +1114,10 @@ void ConfigureWorkflowBuilder(
             CreateGraphNodeType nodeType)
         {
             // Estimate number of inputs to the nested node
-            var inputCount = workflowBuilder.ArgumentRange.LowerBound;
-            if (nodeType == CreateGraphNodeType.Successor) inputCount = Math.Max(inputCount, selectedNodes.Count());
-            else inputCount = Math.Max(inputCount, selectedNodes.Sum(node => workflow.PredecessorEdges(node).Count()));
+            var inputCount = nodeType == CreateGraphNodeType.Successor
+                ? selectedNodes.Count(node => !node.Value.IsBuildDependency())
+                : selectedNodes.Sum(node => workflow.PredecessorEdges(node).Count(edge => !edge.Item1.Value.IsBuildDependency()));
+            inputCount = Math.Max(workflowBuilder.ArgumentRange.LowerBound, inputCount);
 
             // Limit number of inputs depending on nested operator argument range
             if (!(workflowBuilder is GroupWorkflowBuilder || workflowBuilder.GetType() == typeof(Defer)))