Undoing Remove/Disconnect in a statement or parameter list deletes the next sibling, and connecting onto a filled sequence pin silently drops a node

[matt-edmondson]

What's wrong

Three pieces of code combine to lose nodes when the target is a Many slot (a statement body, parameters, arguments or members):

• AstSchema.TryAttachAt (Coder.Graph/AstSchema.cs ~L202) always replaces in place when index < Count. It never inserts.
• AstGraph.MoveTo (Coder.Graph/AstGraph.cs ~L318–339) detaches the node first, which shifts every later sibling down by one, and then attaches at the original index.
• The editor undoes a move with Graph.MoveTo(node, from): RecordMove in AstGraphEditor.cs ~L1615, plus the Remove/Detach inverses ~L1542/L1601. Undo therefore overwrites whichever sibling has shifted into the old index.

The overwritten node is not added to detached. It drops out of the graph and the document with no warning. This contradicts MoveTo's own comment that "the node stays in the graph rather than vanishing".

Failure scenarios (reproduced headlessly through AstGraph / AstGraphEditor)

The body is [s0, s1, s2], where each sN is an ExpressionStatement(CallExpression).

Undo destroys a statement:

remove s1:      s0, s2
undo:           s0, s1   detached=0     <- s2 is gone
disconnect s1:  s0, s2
undo:           s0, s1   detached=0     <- s2 is gone

The same happens through the new pin context menu's Delete and Disconnect items (RemoveFromPin / DisconnectPin).

Connecting within a list or onto a filled pin:

editor.Connect(s0 -> Body[1]):          s1, s0          <- s2 deleted; s1 (the pin's actual occupant) kept
undo:                                   s0              <- s1 deleted too; s0, s1, s2 cannot be recovered
editor.Connect(loose x -> Body[0]) on [s0, s1]:  x, s1  <- s0 not detached, just gone
undo:                                   s1, detached=1  <- s0 cannot be recovered

Replacing the occupant when you drop onto a filled pin may be intended. Replacing the wrong sibling after the index shift is not. Neither is losing the displaced node permanently.

Undo is supposed to be the safe way back, and here it silently deletes user code. The existing tests only cover single-operand slots (e.g. Binary.Left) or TryAttachAt on its own, so this path has no coverage.

Suggested fix

• Separate "insert at" from "replace at": add AstSchema.TryInsertAt, and have MoveTo use insert semantics for Many slots. At minimum, the undo path restoring a node to its original location should insert.
• When the source and target are the same parent and slot, adjust the target index after the detach (decrement it if the source index was lower).
• If a drop onto a filled pin should keep replacing the occupant, move the displaced occupant into detached and record a composite step that re-attaches it on undo, as Replace already does for the children it moves.

Acceptance criteria

Regression tests, each ending with a body/list identical to the starting one after undo and nothing missing from the graph:

• remove or disconnect a middle statement, then undo
• connect Body[0] onto Body[1], then undo
• connect a loose node onto a filled sequence pin, then undo

[matt-edmondson]

Triage

• Category: Bug
• Priority: High. Undo, which users rely on as the safe way back, silently deletes user code, and there is no way to recover it (the displaced node is never added to detached).
• Area / suggested assignee: Coder.Graph (AstSchema.TryAttachAt, AstGraph.MoveTo) and the editor's undo recording (AstGraphEditor.RecordMove and the Remove/Detach inverses). Suggested for whoever owns the graph editor (@matt-edmondson).
• Duplicates: None found. The other open Coder bugs (Rust generator turns static/const fields inside a type into per-instance struct fields and silently drops their value #81–YAML round-trip drops Metadata and keyed children from every declaration node (functions, classes, fields, parameters, entry points…) #87) cover the generators and YAML, not graph editing.
• In progress: No open PR covers this. Open Coder PRs Spell a mutable static C++ field inline static inside a type #88–Write whole-number doubles with a fraction so they stay floating-point #90 address Whole-number double literals are emitted as integers: 1.0/2.0 becomes integer division (0) in C/C++/Go and fails to compile in Rust #78–C++ generator emits non-const static data members with in-class initializers (static int count = 0;), which doesn't compile #80.

Notes: The fix has two parts. First, give Many slots insert semantics, via TryInsertAt or by having MoveTo insert, and adjust the index when a node moves within the same list. Second, decide whether dropping onto a filled pin should replace its occupant. If it should, the displaced node has to go into detached and be restored on undo. The acceptance criteria list three regression tests, and all three should land with the fix.

---

Generated by Claude Code