From 7b12de321ccf31b61de9b15d34cee1f7271097d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 06:45:12 +0000 Subject: [PATCH 1/4] Offer a pin's own menu on right-click (closes #35) [minor] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-clicking a pin opened the create-node palette, so what could be done to the place a pin stands for was reachable only elsewhere: cutting a child loose meant hitting the link drawn to it, removing it meant selecting the node and pressing delete, and filling the pin meant dragging a link off it and dropping it on empty canvas. The gesture now decides which menu it means: over a slot's pin it opens that pin's own menu, anywhere else the palette as before. The menu offers what the pin's state allows — always the nodes that would fill it, and, when something is in it, cutting that loose or removing it outright. Each is the edit the editor already had, so each is undoable as it was. An output pin keeps the palette: it stands for the node itself rather than for a place in the document. Renaming is not offered. A pin has no name of its own to change — it is labelled from the slot it fills, which is part of the node's shape rather than of the document — and what the pin holds is renamed by its own Name in the inspector. AstGraph.ChildInPin answers what is in a pin, since a variadic slot draws a free pin past its last child; Disconnect and Remove grew a pin-shaped entry point each, sharing the body they already had. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011gZgXrkFA4Fy13yuxgmqKz --- Coder.Graph/AstGraph.cs | 21 ++ Coder.Graph/AstGraphEditor.cs | 189 ++++++++++- .../Graph/AstGraphEditorPinMenuTests.cs | 302 ++++++++++++++++++ Coder.Test/Graph/AstGraphTests.cs | 61 ++++ 4 files changed, 568 insertions(+), 5 deletions(-) create mode 100644 Coder.Test/Graph/AstGraphEditorPinMenuTests.cs diff --git a/Coder.Graph/AstGraph.cs b/Coder.Graph/AstGraph.cs index 4b0bcac..a425ca3 100644 --- a/Coder.Graph/AstGraph.cs +++ b/Coder.Graph/AstGraph.cs @@ -117,6 +117,27 @@ public AstGraph(AstNode root) ? new AstLocation(pin.Parent, pin.Slot, pin.Index) : null; + /// + /// Finds the child currently sitting in an input pin. + /// + /// The pin's id. + /// The child filling the pin, or null when the pin is empty or is not an input pin of this graph. + /// + /// A variadic slot draws a free pin past its last child, so a pin being in the graph does not mean + /// anything is in it. That is what separates this from : the + /// location says where the pin points, this says whether anything is there. + /// + public AstNode? ChildInPin(int inputPinId) + { + if (LocationOfInputPin(inputPinId) is not AstLocation place || place.Parent is null || place.Slot is null) + { + return null; + } + + IReadOnlyList children = AstSchema.ChildrenOf(place.Parent, place.Slot); + return place.Index < children.Count ? children[place.Index] : null; + } + /// /// Rebuilds the engine graph from the current AST, preserving on-screen positions. /// diff --git a/Coder.Graph/AstGraphEditor.cs b/Coder.Graph/AstGraphEditor.cs index 4109429..e4a7e27 100644 --- a/Coder.Graph/AstGraphEditor.cs +++ b/Coder.Graph/AstGraphEditor.cs @@ -67,6 +67,11 @@ public sealed class AstGraphEditor(AstNode root) /// private const string LinkDropPopup = "ast-graph-link-drop"; + /// + /// The popup offering a pin's own actions, opened by right-clicking it. + /// + private const string PinMenuPopup = "ast-graph-pin-menu"; + private string statusMessage = string.Empty; private string fieldBuffer = string.Empty; @@ -94,6 +99,22 @@ public sealed class AstGraphEditor(AstNode root) /// private bool linkDropOpening; + /// + /// The pin whose own menu is open, while it is. Null when none is. + /// + /// + /// Held across frames for the reason is: the right-click and the choice + /// happen in different ones. The pin is held by identifier rather than as the place it points at, + /// because every edit offered here closes the menu, so there is no frame in which the menu is open + /// and the rebuild that reassigns those identifiers has happened. + /// + private int? pinMenuPin; + + /// + /// Whether the menu for still has to be opened. + /// + private bool pinMenuOpening; + /// /// Gets the graph being edited. /// @@ -221,6 +242,7 @@ public void Draw(Vector2 size, float deltaTime) TrackSelection(); ApplyDeletions(); DrawPalette(); + DrawPinMenu(); DrawLinkDropMenu(); if (ShowDebugOverlays) @@ -843,9 +865,30 @@ public AstConnectResult Connect(int outputPinId, int inputPinId) /// /// The link to cut. /// True if the link was found and cut. - public bool Disconnect(int linkId) + public bool Disconnect(int linkId) => Detach(Graph.ChildOfLink(linkId)); + + /// + /// Cuts whatever fills a pin loose from it, as one undoable edit. + /// + /// The pin to empty. + /// True if the pin held a child, which was cut loose. + /// + /// The same edit as cutting the link, reached from the pin rather than from the line drawn to it: a + /// link is a few pixels wide, and the pin is what the user was already pointing at. + /// + public bool DisconnectPin(int pinId) => Detach(Graph.ChildInPin(pinId)); + + /// + /// Cuts a child loose from its parent, as one undoable edit. + /// + /// The node to detach, or null when there was nothing to detach. + /// True if there was a node, and it was detached. + /// + /// The node stays in the graph with no parent rather than leaving it: the user asked for it not to + /// be connected there, which is not the same as asking for it to be gone. + /// + private bool Detach(AstNode? child) { - AstNode? child = Graph.ChildOfLink(linkId); if (child is null) { return false; @@ -1113,6 +1156,118 @@ public bool RequestCreateFrom(int pinId) return true; } + /// + /// Offers a pin's own menu, the way right-clicking the pin does. + /// + /// The pin to offer the menu for. + /// True if the pin is one this graph offers a menu for, so the menu will open. + /// + /// Separate from the gesture that usually triggers it, for the reason + /// is: a host binding the menu to a key wants the same menu, and it can be driven in a test without + /// a pin happening to be under the pointer. + /// + /// Only a slot's pin has a menu. A node's output pin stands for the node itself rather than for a + /// place in the document, so its actions are the node's and the palette answers a click on it, as + /// it did before this menu existed. + /// + /// + public bool RequestPinMenu(int pinId) + { + if (Graph.LocationOfInputPin(pinId) is null) + { + statusMessage = Graph.OwnerOfOutputPin(pinId) is null + ? "That pin is not in this graph." + : "An output pin stands for the node itself, so it has no menu of its own."; + return false; + } + + pinMenuPin = pinId; + pinMenuOpening = true; + return true; + } + + /// + /// Draws a pin's own menu, and makes whichever edit the user picks. + /// + /// + /// What a pin offers is what can be done to the place it stands for: fill it, cut loose what fills + /// it, or remove that outright. The last two are offered only when something is there, since a + /// variadic slot draws a free pin past its last child and that one has nothing to act on. + /// + /// Renaming is not offered, because a pin has no name of its own to change: it is labelled from the + /// slot it fills, which is part of the node's shape rather than of the document. What the pin holds + /// is renamed by editing that node's own Name in the inspector, which is where the rest of a node's + /// values are edited. + /// + /// + private void DrawPinMenu() + { + if (pinMenuPin is not int pinId) + { + return; + } + + if (pinMenuOpening) + { + ImGui.OpenPopup(PinMenuPopup); + pinMenuOpening = false; + } + + if (!ImGui.BeginPopup(PinMenuPopup)) + { + // Remembered exactly as long as the popup is open: once it has gone the user either picked + // something or dismissed it, and either way the pin is spent. + pinMenuPin = null; + return; + } + + AstNode? child = Graph.ChildInPin(pinId); + + bool create = Picked("Create node here", "Pin create"); + bool disconnect = child is not null && Picked("Disconnect", "Pin disconnect"); + bool remove = child is not null && Picked("Delete", "Pin delete"); + + if (create) + { + // Offered rather than made here: what would fill the pin is a choice, and it is the one the + // menu a dropped link opens already asks. This reaches it without the drag. + RequestCreateFrom(pinId); + } + else if (disconnect) + { + DisconnectPin(pinId); + } + else if (remove) + { + RemoveFromPin(pinId); + } + + if (create || disconnect || remove) + { + pinMenuPin = null; + ImGui.CloseCurrentPopup(); + } + + ImGui.EndPopup(); + } + + /// + /// Draws one entry of a pin's menu, under a name a test can ask for. + /// + /// The entry as the user reads it. + /// The name the entry is recorded under. + /// True if the user picked it. + /// + /// Marked under a name of its own rather than the label it reads as, because a word as ordinary as + /// "Delete" will be drawn elsewhere too and a test asking for one should not find the other. + /// + private static bool Picked(string label, string probe) + { + bool picked = ImGui.MenuItem(label); + ImGuiProbes.MarkItem(probe); + return picked; + } + /// /// Creates a node from a template and connects it to the pin a link was dragged off. /// @@ -1297,9 +1452,26 @@ private void AttachAt(AstNode parent, AstSlot slot, int index, AstNode child) /// The removal is checked before it is recorded, so pressing delete over the root — which is /// never removed — leaves the history alone rather than adding a step that does nothing. /// - public bool Remove(int nodeId) + public bool Remove(int nodeId) => RemoveSubtree(Graph.AstNodeFor(nodeId)); + + /// + /// Removes whatever fills a pin, and everything under it, as one undoable edit. + /// + /// The pin to empty. + /// True if the pin held a child, which was removed. + /// + /// Told apart from , which leaves the child in the graph: this is the + /// edit pressing delete over the child makes, reached from the pin holding it. + /// + public bool RemoveFromPin(int pinId) => RemoveSubtree(Graph.ChildInPin(pinId)); + + /// + /// Removes a node and everything under it, as one undoable edit. + /// + /// The node to remove, or null when there was nothing to remove. + /// True if there was a node that could be removed, and it was. + private bool RemoveSubtree(AstNode? node) { - AstNode? node = Graph.AstNodeFor(nodeId); if (node is null || ReferenceEquals(node, Graph.Root)) { return false; @@ -1408,7 +1580,14 @@ private void DrawPalette() { if (ImGui.IsMouseClicked(ImGuiMouseButton.Right) && ImGui.IsWindowHovered(ImGuiHoveredFlags.RootAndChildWindows)) { - ImGui.OpenPopup("ast-graph-palette"); + // A right-click over a pin is about that pin, so the palette answers only one that lands + // elsewhere. Both gestures are the same click, and which menu it means is decided in one + // place rather than by two handlers each opening a popup. + int hovered = 0; + if (!ImNodes.IsPinHovered(ref hovered) || !RequestPinMenu(hovered)) + { + ImGui.OpenPopup("ast-graph-palette"); + } } if (!ImGui.BeginPopup("ast-graph-palette")) diff --git a/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs new file mode 100644 index 0000000..9f11c2a --- /dev/null +++ b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs @@ -0,0 +1,302 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Graph; + +using System.Linq; +using System.Numerics; +using Hexa.NET.ImGui; +using ktsu.Coder.Ast; +using ktsu.Coder.Graph; +using ktsu.ImGui.App; +using ktsu.ImGui.App.Testing; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers the menu a right-clicked pin opens as the user meets it: entries reached with the mouse, by +/// the label they read as, through the headless rasterizer. +/// +/// +/// What fills a pin is covered headlessly in . What is covered here is +/// that the popup draws the entries the pin's state calls for, and that picking one reaches the edit — +/// the half that only a rendered frame can establish, since ImGui menus fault inside native code +/// rather than throwing when misnested. +/// +/// The menu is opened through rather than by right-clicking +/// a pin, because where a pin lands on screen depends on the force-directed layout and ImNodes +/// publishes no position for one. That is the same reason +/// opens its menu through the editor. +/// +/// +/// ImGui contexts are process-global, so only one harness can be live at a time and this class must +/// not run its methods in parallel. +/// +/// +[TestClass] +[DoNotParallelize] +public sealed class AstGraphEditorPinMenuTests +{ + private static readonly HarnessOptions Options = new() { Width = 1280, Height = 800 }; + + private static FunctionDeclaration SampleFunction() + { + FunctionDeclaration function = new("total") { ReturnType = "int" }; + function.Parameters.Add(new Parameter("a", "int")); + function.Body.Add(new ReturnStatement( + new BinaryExpression(new VariableReference("a"), BinaryOperator.Add, Literal.Number(1)))); + return function; + } + + private static ImGuiAppConfig ConfigFor(AstGraphEditor editor) => new() + { + Title = "AST pin menu", + OnRender = delta => + { + ImGui.Begin("graph"); + editor.Draw(new Vector2(1000, 600), delta); + ImGui.End(); + }, + }; + + private static bool IsVisible(ImGuiAppHarness harness, string name) => + harness.Probe.WasSeenInFrame(name, harness.FrameCount - 1); + + /// + /// Starts a harness with a pin's own menu already open. + /// + /// The editor to draw. + /// The pin to open the menu for. + /// The running harness, for the caller to dispose. + private static ImGuiAppHarness Offering(AstGraphEditor editor, int pinId) + { + ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); + harness.Step(2); + + Assert.IsTrue(editor.RequestPinMenu(pinId), "the pin to offer a menu for should be one of this graph's slots"); + harness.Step(2); + return harness; + } + + /// + /// Tests that a pin holding a child offers what can be done to that child as well as what can fill + /// the pin. + /// + [TestMethod] + public void Menu_OffersDisconnectAndDeleteForAFilledPin() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 0)); + + Assert.IsTrue(IsVisible(harness, "Pin create"), "a pin can always be offered something to fill it"); + Assert.IsTrue(IsVisible(harness, "Pin disconnect"), "the pin holds a parameter, which can be cut loose"); + Assert.IsTrue(IsVisible(harness, "Pin delete"), "the pin holds a parameter, which can be removed"); + } + + /// + /// Tests that the free pin a variadic slot draws past its last child offers only what would fill + /// it, since there is nothing in it to act on. + /// + [TestMethod] + public void Menu_OffersOnlyCreateForAFreePin() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 1)); + + Assert.IsTrue(IsVisible(harness, "Pin create")); + Assert.IsFalse(IsVisible(harness, "Pin disconnect"), "nothing fills the pin, so nothing can be cut loose"); + Assert.IsFalse(IsVisible(harness, "Pin delete"), "nothing fills the pin, so nothing can be removed"); + } + + /// + /// Tests that disconnecting cuts the child loose from the slot and leaves it in the graph, rather + /// than removing it. + /// + [TestMethod] + public void Menu_DisconnectCutsTheChildLoose() + { + FunctionDeclaration function = SampleFunction(); + Parameter parameter = function.Parameters[0]; + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 0)); + + harness.Click("Pin disconnect"); + harness.Step(2); + + Assert.IsEmpty(function.Parameters); + CollectionAssert.Contains(editor.Graph.Detached.ToArray(), parameter, "a disconnected node stays in the graph"); + Assert.IsTrue(editor.History.CanUndo, "disconnecting through the menu should be undoable"); + + editor.Undo(); + Assert.AreSame(parameter, function.Parameters.Single()); + } + + /// + /// Tests that deleting takes the child out of the document altogether, which is what tells it apart + /// from disconnecting. + /// + [TestMethod] + public void Menu_DeleteRemovesTheChild() + { + FunctionDeclaration function = SampleFunction(); + Parameter parameter = function.Parameters[0]; + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 0)); + + harness.Click("Pin delete"); + harness.Step(2); + + Assert.IsEmpty(function.Parameters); + Assert.IsFalse( + editor.Graph.Nodes.Values.Any(node => ReferenceEquals(node, parameter)), + "a deleted node leaves the graph"); + Assert.IsTrue(editor.History.CanUndo, "deleting through the menu should be undoable"); + + editor.Undo(); + Assert.AreSame(parameter, function.Parameters.Single()); + } + + /// + /// Tests that asking to fill a pin opens the menu of nodes that would fill it, which is the one a + /// dropped link already opens. + /// + [TestMethod] + public void Menu_CreateOffersTheNodesThatWouldFillThePin() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 1)); + + harness.Click("Pin create"); + harness.Step(3); + + Assert.IsTrue(IsVisible(harness, "Create Declarations"), "a Parameters slot takes a parameter, which is a declaration"); + Assert.IsFalse(IsVisible(harness, "Pin create"), "the pin's own menu should have closed behind it"); + } + + /// + /// Tests that dismissing the menu without picking anything leaves the document alone and forgets the + /// pin, rather than offering the menu again on the next frame. + /// + [TestMethod] + public void Menu_DismissedWithoutPicking_ChangesNothing() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Offering(editor, InputPin(editor.Graph, function, "Parameters", 0)); + Assert.IsTrue(IsVisible(harness, "Pin delete"), "the menu should be open to begin with"); + + int before = editor.Graph.Nodes.Count; + + // Clicked away from the popup rather than dismissed with Escape: a click outside is what closes + // an ImGui popup, and it is also what a user who changed their mind actually does. + harness.Mouse.Click(1220, 60); + harness.Step(3); + + Assert.IsFalse(IsVisible(harness, "Pin delete"), "the menu should be gone once dismissed"); + Assert.HasCount(before, editor.Graph.Nodes); + Assert.IsFalse(editor.History.CanUndo, "dismissing should record nothing"); + Assert.HasCount(1, function.Parameters); + } + + /// + /// Tests that a node's output pin is refused, since it stands for the node rather than for a place + /// in the document, and that a pin that is not in the graph is refused too. + /// + [TestMethod] + public void RequestPinMenu_AnythingButASlotPin_IsRefused() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); + harness.Step(2); + + int outputPin = editor.Graph.Engine.Nodes + .Single(n => ReferenceEquals(editor.Graph.AstNodeFor(n.Id), function)) + .OutputPins[0].Id; + + Assert.IsFalse(editor.RequestPinMenu(outputPin)); + Assert.IsFalse(editor.RequestPinMenu(-1)); + harness.Step(2); + + Assert.IsFalse(IsVisible(harness, "Pin create")); + } + + /// + /// Tests that a right-click that lands on a node but not on one of its pins is not taken for a pin, + /// so the create-node palette still answers it. + /// + /// + /// The click lands on the node's title bar, which is the one part of a node with no pin in it. The + /// opposite case — a click that does land on a pin — is not covered here: ImNodes publishes no + /// screen position for a pin, so a test could only guess at one, and the menu it opens is covered + /// through above. + /// + [TestMethod] + public void RightClickOffAPin_DoesNotOpenThePinMenu() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function) { LayoutRunning = false }; + + Vector2 topLeft = Vector2.Zero; + Vector2 size = Vector2.Zero; + int nodeId = 0; + + ImGuiAppConfig config = new() + { + Title = "AST pin menu gesture", + OnRender = delta => + { + ImGui.Begin("graph"); + editor.Draw(new Vector2(1000, 600), delta); + + // Read inside the frame that drew it: the node's screen position is ImNodes' own, and it + // is only meaningful while its context is the current one. + if (nodeId != 0) + { + topLeft = Hexa.NET.ImNodes.ImNodes.GetNodeScreenSpacePos(nodeId); + size = Hexa.NET.ImNodes.ImNodes.GetNodeDimensions(nodeId); + } + + ImGui.End(); + }, + }; + + using ImGuiAppHarness harness = ImGuiAppHarness.Start(config, Options); + harness.Step(4); + + nodeId = editor.Graph.Engine.Nodes.Single(n => ReferenceEquals(editor.Graph.AstNodeFor(n.Id), function)).Id; + harness.Step(2); + + Assert.IsGreaterThan(0f, size.X, "the node should have been measured by now"); + + harness.Mouse.Click(topLeft.X + (size.X * 0.5f), topLeft.Y + 4f, 1); + harness.Step(2); + + Assert.IsFalse(IsVisible(harness, "Pin create"), "no pin is under the title bar, so no pin menu is offered"); + Assert.IsFalse(editor.History.CanUndo, "a right-click records nothing either way"); + } + + /// + /// Finds the pin standing for one place in a node's slot, the way the editor does. + /// + /// The graph under test. + /// The node whose slot to look at. + /// The slot's name. + /// The position within the slot. + /// The input pin's id. + private static int InputPin(AstGraph graph, AstNode parent, string slotName, int index) + { + ktsu.ImGui.NodeEditor.Node parentNode = + graph.Engine.Nodes.Single(n => ReferenceEquals(graph.AstNodeFor(n.Id), parent)); + AstSlot slot = AstSchema.SlotsOf(parent).Single(s => s.Name == slotName); + return parentNode.InputPins.Single(p => p.EffectiveDisplayName == AstGraph.PinLabel(slot, index)).Id; + } +} diff --git a/Coder.Test/Graph/AstGraphTests.cs b/Coder.Test/Graph/AstGraphTests.cs index de0309c..6a9a22b 100644 --- a/Coder.Test/Graph/AstGraphTests.cs +++ b/Coder.Test/Graph/AstGraphTests.cs @@ -405,6 +405,67 @@ public void Connect_PutsAMemberInitialiserIntoAConstruction() Assert.IsEmpty(graph.Detached); } + /// + /// Tests that a pin reads back the child sitting in it, for both a variadic slot and a + /// single-valued one. + /// + [TestMethod] + public void ChildInPin_ReadsWhatFillsThePin() + { + FunctionDeclaration function = SampleFunction(); + AstGraph graph = new(function); + + Assert.AreSame(function.Parameters[0], graph.ChildInPin(InputPin(graph, function, "Parameters", 0))); + Assert.AreSame(function.Body[0], graph.ChildInPin(InputPin(graph, function, "Body", 0))); + + ReturnStatement returnStmt = (ReturnStatement)function.Body[0]; + Assert.AreSame(returnStmt.Expression, graph.ChildInPin(InputPin(graph, returnStmt, "Expression", 0))); + } + + /// + /// Tests that the free pin a variadic slot draws past its last child reads back as empty, since + /// nothing is in it yet. + /// + [TestMethod] + public void ChildInPin_IsNullForAFreePin() + { + FunctionDeclaration function = SampleFunction(); + AstGraph graph = new(function); + + Assert.IsNull(graph.ChildInPin(InputPin(graph, function, "Parameters", function.Parameters.Count))); + } + + /// + /// Tests that a pin this graph does not have, and a pin that is an output rather than a slot, both + /// read back as empty rather than throwing. + /// + [TestMethod] + public void ChildInPin_IsNullForAPinThatIsNotASlotPin() + { + FunctionDeclaration function = SampleFunction(); + AstGraph graph = new(function); + + Node functionNode = graph.Engine.Nodes.Single(n => ReferenceEquals(graph.AstNodeFor(n.Id), function)); + + Assert.IsNull(graph.ChildInPin(-1)); + Assert.IsNull(graph.ChildInPin(functionNode.OutputPins[0].Id)); + } + + /// + /// Finds the pin standing for one place in a node's slot, the way the editor does. + /// + /// The graph under test. + /// The node whose slot to look at. + /// The slot's name. + /// The position within the slot. + /// The input pin's id. + private static int InputPin(AstGraph graph, AstNode parent, string slotName, int index) + { + Node parentNode = graph.Engine.Nodes.Single(n => ReferenceEquals(graph.AstNodeFor(n.Id), parent)); + AstSlot slot = AstSchema.SlotsOf(parent).Single(s => s.Name == slotName); + return parentNode.InputPins.Single(p => p.EffectiveDisplayName == AstGraph.PinLabel(slot, index)).Id; + } + /// /// Connects a node to a named slot of a parent, looking the pins up the way the editor does. /// From fd57dea785aad159e8314547d840e52140788436 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 06:48:36 +0000 Subject: [PATCH 2/4] style(test): take the assertion Sonar names for the new test [patch] The same MSTEST0068 family Sonar raised on #76: Assert.Contains rather than CollectionAssert.Contains, on the line this PR adds. The two pre-existing uses in AstGraphTests keep theirs, since Sonar reads only new code and rewriting them is not this PR's business. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011gZgXrkFA4Fy13yuxgmqKz --- Coder.Test/Graph/AstGraphEditorPinMenuTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs index 9f11c2a..ae155c2 100644 --- a/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs +++ b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs @@ -127,7 +127,7 @@ public void Menu_DisconnectCutsTheChildLoose() harness.Step(2); Assert.IsEmpty(function.Parameters); - CollectionAssert.Contains(editor.Graph.Detached.ToArray(), parameter, "a disconnected node stays in the graph"); + Assert.Contains(parameter, editor.Graph.Detached, "a disconnected node stays in the graph"); Assert.IsTrue(editor.History.CanUndo, "disconnecting through the menu should be undoable"); editor.Undo(); From 6d5f44e0d5d524bcfcdcae5ef5be54ee583dcd9e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 07:00:52 +0000 Subject: [PATCH 3/4] style(test): assert the absence with DoesNotContain [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSTEST0037 on the delete test: Assert.DoesNotContain rather than Assert.IsFalse over a hand-written Any. Equivalent here — an AstNode has no value equality, so containment is reference identity, which is what the predicate asked. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011gZgXrkFA4Fy13yuxgmqKz --- Coder.Test/Graph/AstGraphEditorPinMenuTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs index ae155c2..28588bb 100644 --- a/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs +++ b/Coder.Test/Graph/AstGraphEditorPinMenuTests.cs @@ -151,9 +151,7 @@ public void Menu_DeleteRemovesTheChild() harness.Step(2); Assert.IsEmpty(function.Parameters); - Assert.IsFalse( - editor.Graph.Nodes.Values.Any(node => ReferenceEquals(node, parameter)), - "a deleted node leaves the graph"); + Assert.DoesNotContain(parameter, editor.Graph.Nodes.Values, "a deleted node leaves the graph"); Assert.IsTrue(editor.History.CanUndo, "deleting through the menu should be undoable"); editor.Undo(); From 94a7071c1b835eaa464c6ae0b06968936ab0f01f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 08:07:08 +0000 Subject: [PATCH 4/4] Ask the three-way menu choice as part of the click's own condition [patch] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S1066 on the merge resolution: the nested if was the only statement inside the right-click guard, so the two read as one condition and now are one. Same short-circuit as before — OfferOwnMenu is asked last, and only for a click that reached this canvas. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011gZgXrkFA4Fy13yuxgmqKz --- Coder.Graph/AstGraphEditor.cs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Coder.Graph/AstGraphEditor.cs b/Coder.Graph/AstGraphEditor.cs index 4ef19e0..00eccff 100644 --- a/Coder.Graph/AstGraphEditor.cs +++ b/Coder.Graph/AstGraphEditor.cs @@ -1710,16 +1710,16 @@ private bool OfferOwnMenu() /// private void DrawPalette() { - if (ImGui.IsMouseClicked(ImGuiMouseButton.Right) && ImGui.IsWindowHovered(ImGuiHoveredFlags.RootAndChildWindows)) - { - // A right-click over a pin is about that pin and one over a node is about that node, so the - // palette answers only a click that lands on neither. All three are the same click, and - // which menu it means is decided in one place rather than by three handlers racing to open - // a popup. - if (!OfferOwnMenu()) - { - ImGui.OpenPopup("ast-graph-palette"); - } + // A right-click over a pin is about that pin and one over a node is about that node, so the + // palette answers only a click that lands on neither. All three are the same click, and which + // menu it means is decided in one place rather than by three handlers racing to open a popup. + // OfferOwnMenu is asked last, and only where the click reaches this canvas at all, so a menu is + // never offered for a click somewhere else in the window. + if (ImGui.IsMouseClicked(ImGuiMouseButton.Right) + && ImGui.IsWindowHovered(ImGuiHoveredFlags.RootAndChildWindows) + && !OfferOwnMenu()) + { + ImGui.OpenPopup("ast-graph-palette"); } if (!ImGui.BeginPopup("ast-graph-palette"))