You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Undoing Remove/Disconnect in a statement or parameter list deletes the next sibling, and connecting onto a filled sequence pin silently drops a node #91
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).
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
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).
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.
Activity
matt-edmondson commentedon Sep 26, 2026
Triage
detached).AstSchema.TryAttachAt,AstGraph.MoveTo) and the editor's undo recording (AstGraphEditor.RecordMoveand the Remove/Detach inverses). Suggested for whoever owns the graph editor (@matt-edmondson).static int count = 0;), which doesn't compile #80.Notes: The fix has two parts. First, give
Manyslots insert semantics, viaTryInsertAtor by havingMoveToinsert, 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 intodetachedand be restored on undo. The acceptance criteria list three regression tests, and all three should land with the fix.Generated by Claude Code