Offer to create a node when a link is dropped on empty canvas

[matt-edmondson]

Fixes #37

Dragging a link end off a pin and releasing it over nothing used to do nothing at all: ImNodes threw the link away, and the user had to reach for the right-click palette, create a node, then drag the link a second time. Now the drop opens a menu of the nodes that would actually connect to the pin the drag began at, and picking one creates it at the drop position and wires it up.

Which way the pin faces decides what is offered

The new node takes the other end of the connection, so the two directions are not the same menu:

Drag began at	The user is asking for	Entries offered
an input pin (a slot)	something to fill that slot	the templates the slot accepts
an output pin (a node)	somewhere to put that node	the templates with a slot that accepts it

In the output-pin case the node the user dragged moves into the new one, rather than both being left detached — that is the direction the issue is really about.

Both are filtered through the same AstSchema.Accepts the manual drag is checked against, so nothing is offered that the connection would then refuse. Per the triage's suggestion, that check is reused rather than re-implemented.

The two UX questions the triage raised

"No compatible node type exists" — the menu says so, as disabled text naming what was dragged, instead of opening empty. Categories and submenus that filtered empty are hidden, so the menu is a shortlist rather than the whole catalogue with most of it refusing.

This is reachable when the drag began at a slot: an enumeration's members take an EnumMember and the palette has no entry for one, which is what the test for it uses. It is not currently reachable from an output pin, and the reason is worth recording — AstSlotKind.Statement accepts everything except a Parameter and an EntryPoint, so every node in the catalogue has somewhere to go. (I found this by writing a test that assumed an EnumMember would be refused everywhere; it isn't, because a body slot takes one. That may be worth its own look, but it is pre-existing and not this PR's to change.) That branch stays as a guard rather than being deleted.

Cancelling out of the menu — nothing is left dangling. ImNodes discards a dropped link itself, so there is no half-made connection to undo; dismissing the popup only forgets the remembered pin. There is a test for exactly this.

History

Creating the node and connecting it go on the stack as two steps, matching the documented intent on Add ("creating one and connecting it are two steps in the history rather than one — that matches what the user did"). The first undo takes the connection back and leaves the node the user asked for on the canvas, which is a state they can reach by hand too.

Where the code sits

The class docs say this type "only reads what the user did and asks the graph to do it, so the part that cannot be tested without a GPU holds no rules of its own". The rules therefore went into testable methods rather than the draw call:

• AstGraph.OwnerOfOutputPin / AstGraph.LocationOfInputPin — two pin lookups, so a caller holding one pin id can tell which end of a connection it is. LocationOfInputPin returns an AstLocation rather than the private pin record, because a location names nodes and outlives the rebuild that reassigns pin ids.
• AstGraphEditor.CreatableFrom(pinId) — the filtered entries. Sits beside ConversionsFor, which is the same shape of rule.
• AstGraphEditor.CreateFrom(template, pinId, position) — creates and connects. Refuses without recording anything if the pin is not in the graph.
• AstGraphEditor.RequestCreateFrom(pinId) — offers the menu. ApplyLinkChanges calls it when ImNodes reports a drop, but it is a real entry point in its own right: a host binding the same affordance to a key wants it, and it makes the menu reachable without a drag landing on a pin by luck. It records only the request, since ImGui.OpenPopup means nothing outside a frame.

Verification

Check	Result
Clean Release build	succeeded, 0 warnings, 0 errors
Full suite (Release)	885/885 passed — 867 before, plus 18 new
SonarCloud	Quality Gate passed, 0 new issues, 96.9% coverage on new code, 0 duplication

The rule tests were checked against two deliberately broken implementations, rather than only against the working one:

1. Resolve the pin but drop the acceptance filter (offer the whole catalogue) → 3 tests fail, on the per-entry Accepts assertion, the parameter menu growing from 1 entry to 49, and the empty-menu case.
2. Create the node but never connect it (leave an orphan) → 3 tests fail, on the slot not being filled and on the dragged node not ending up inside the new one.

Ten headless tests in AstGraphEditorLinkDropTests cover the rules. Seven harness tests in AstGraphEditorLinkDropMenuTests drive the popup itself through the CPU rasterizer — which categories it lists, clicking through to an entry and getting the node created and connected, the grouped operator submenu, the empty-slot message, dismissal recording nothing, and an unknown pin being refused. One further harness test performs a real drag, since ImNodes.IsLinkDropped is a native call that only a rendered frame exercises.

Corrections to an earlier version of this description. It claimed the popup's body was not rendered by any test and treated that as an acceptable limitation. It was not acceptable — SonarCloud failed this PR at 48.6% coverage on new code, correctly, since the repo sits at 92.6% overall. The cause was a design problem rather than a testing one: the popup could only open as a side effect of a drag landing on a pin, so no test could reach it. RequestCreateFrom fixes that, and the menu's items are now named for ImGuiProbes as the inspector's rows already are. New-code coverage went 48.6% → 96.9%. The same version also said the "nothing takes this node" message was reachable from an output pin; it is not, as explained above.

Seven lines remain uncovered, deliberately: the IsLinkDropped branch itself (still needs a drag to land on a pin), an empty group inside a non-empty category (no slot in the catalogue produces that combination), and the output-pin branch of the "nothing connects" message (unreachable, per above).

Note on the issue's triage comments

The triage on #37 (and on #35/#36) names NodeEditorRenderer, NodeEditorInputHandler, NodeEditorEngine and AttributeBasedNodeFactory as the places to change. Only NodeEditorEngine/NodeEditorRenderer exist, and they come from the ktsu.ImGui.NodeEditor package rather than this repo; there is no AttributeBasedNodeFactory or NodeEditorInputHandler anywhere, and the node types come from the curated AstNodeCatalog instead. The plan held up, but anyone picking up #35 or #36 should not trust those class names.

🤖 Generated with Claude Code

https://claude.ai/code/session_01U2TWMet7QsHnYUTZzRYXMQ

[matt-edmondson]

Fixed the quality gate — it was a fair call, and the PR body was wrong to shrug at it

SonarCloud failed on 48.6% Coverage on New Code (required ≥ 80%). I checked whether that was an artifact and it was not: this repo sits at 92.6% coverage overall, and the ~130 lines of menu-drawing code I added were exercised by nothing. The original PR body disclosed that the popup body was untested and treated it as an acceptable limitation. That was the wrong call — pushed in 5157af2.

The real problem was a design one, not a testing one

The popup could not be opened by any test, because it only opened as a side effect of ImNodes.IsLinkDropped, which needs a drag to land on a pin whose screen position depends on the force-directed layout. The tell: the existing DrawPalette is covered (16/27 lines), precisely because a right-click opens it unconditionally.

So offering the menu is now an operation:

public bool RequestCreateFrom(int pinId)

ApplyLinkChanges calls it when ImNodes reports the drop. It's a legitimate entry point in its own right — a host binding the same affordance to a key wants it — and it separates reading the gesture from deciding to offer the menu, which is what this class's own docs ask for ("this type only reads what the user did"). It records only the request; the popup opens on the next frame drawn, because ImGui.OpenPopup means nothing outside a frame.

I also named the menu's categories, groups and entries for the probes, the way the inspector's rows already are, so a test picks an entry by the label the user reads rather than by a coordinate. That was a convention I'd missed.

Coverage

	Before	After
New-code coverage (measured locally)	48.6%	95.2% (140/147)
Full suite (Release)	876/876	885/885

Seven new harness tests drive the popup for real through the CPU rasterizer: which categories it lists, clicking through to an entry and getting the node created and connected, the grouped operator submenu, the empty-slot message, dismissal recording nothing, and an unknown pin being refused. Two headless tests cover the refusal branches in CreateFrom that nothing reached before.

Seven lines still uncovered, deliberately

1. The IsLinkDropped branch itself — still needs a drag to land on a pin.
2. An empty group inside a non-empty category — no slot in the catalogue currently produces that combination.
3. NothingConnects' output-pin branch — unreachable today, and the reason is worth recording: AstSlotKind.Statement accepts everything except a Parameter and an EntryPoint, so every node has somewhere in the palette to go. I found this by writing a test that assumed an EnumMember would be refused everywhere; it isn't, because a body slot takes one. That may be worth its own look — an enumeration member being a legal statement reads like the denylist being broader than intended — but it is pre-existing and not this PR's to change. The branch stays as a guard rather than being deleted.

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
96.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud