Fix Alt-drag duplicate cleanup on right-click abort

[jsjgdh]

discussion : https://discord.com/channels/@me/1461994742291234997/1473407462097948765 from DM with @Keavon

[Keavon]

/gemini review

[Copilot]

Pull request overview

This PR aims to ensure Alt-drag node duplication is cleaned up correctly when the drag is aborted (notably via right-click), by tracking whether duplication occurred during the drag and adjusting transaction handling.

Changes:

• Introduces a duplicated_in_drag flag to track Alt-drag duplication state across drag lifecycle.
• Splits duplication into a regular path vs. an Alt-drag path (DuplicateSelectedNodesForDrag) via a shared helper implementation.
• Adjusts abort/commit behavior on right-click/Escape/PointerUp to try to undo or finalize Alt-drag duplication.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs	Adds duplicated_in_drag, introduces DuplicateSelectedNodesForDrag, and changes duplication + drag abort/commit behavior
editor/src/messages/portfolio/document/node_graph/node_graph_message.rs	Adds the DuplicateSelectedNodesForDrag message variant
editor/src/messages/portfolio/document/document_message_handler.rs	Changes Escape handling to attempt undoing Alt-drag duplication on abort

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gemini-code-assist]

Code Review

This pull request addresses an issue with cleaning up duplicated nodes when an Alt-drag operation is aborted. The changes introduce a duplicated_in_drag flag to track when a duplication has occurred during a drag, and uses this flag to ensure proper cleanup via transaction abortion and undos. The logic has been updated for both Escape key and right-click aborts. A good refactoring was done to extract the node duplication logic into a shared duplicate_selected_nodes_impl function.

I have one suggestion to improve code clarity by removing a redundant if/else block. Overall, the changes look good and correctly solve the issue.

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs:1134">
P2: Alt-drag duplicates are now created inside the current drag transaction (no AddTransaction), but ShakeNode still aborts that transaction mid-drag to reset state. This rollback now deletes the duplicate itself, so Alt-drag followed by Shake loses the duplicated nodes and leaves shake logic operating on the pre-duplication selection. The new duplicated_in_drag flag is never used to guard this path.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}