M1.5: commands that default to the current layer validate the remembered id (reused after undo it could name a compound path: Send to Current Layer moved art into it, Delete Layer could delete it)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/layer.rs

@@ -66,7 +66,7 @@ fn new_layer(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn new_sublayer(s: &mut Session, p: &Value) -> Result<Value> {
-    let parent = id_param(p, "parent").or(s.doc()?.active_layer).ok_or_else(|| bad("layer.newSublayer", "no parent layer"))?;
+    let parent = id_param(p, "parent").or(s.doc()?.current_layer()).ok_or_else(|| bad("layer.newSublayer", "no parent layer"))?;
     let name = str_param(p, "name").map(str::to_string);
     let id = s.edit("New Sublayer", |d, _| {
         let n = d.node(parent).and_then(|n| n.children()).map(|c| c.iter().filter(|c| c.is_layer()).count()).unwrap_or(0);
@@ -80,7 +80,7 @@ fn new_sublayer(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn delete_layer(s: &mut Session, p: &Value) -> Result<Value> {
-    let id = id_param(p, "id").or(s.doc()?.active_layer).ok_or_else(|| bad("layer.delete", "no layer"))?;
+    let id = id_param(p, "id").or(s.doc()?.current_layer()).ok_or_else(|| bad("layer.delete", "no layer"))?;
     let st = s.doc()?;
     if st.doc.layers.len() == 1 && st.doc.layers[0].id == id {
         return Err(EngineError::Other("a document needs at least one layer".into()));
@@ -98,7 +98,7 @@ fn delete_layer(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn duplicate_layer(s: &mut Session, p: &Value) -> Result<Value> {
-    let id = id_param(p, "id").or(s.doc()?.active_layer).ok_or_else(|| bad("layer.duplicate", "no layer"))?;
+    let id = id_param(p, "id").or(s.doc()?.current_layer()).ok_or_else(|| bad("layer.duplicate", "no layer"))?;
     let nid = s.edit("Duplicate Layer", |d, _| {
         let (par, idx, _) = d.position(id).ok_or(EngineError::NoNode(id))?;
         let n = d.node(id).cloned().ok_or(EngineError::NoNode(id))?;

crates/engine/src/cmd/object.rs

@@ -321,7 +321,9 @@ fn arrange(s: &mut Session, how: Arrange) -> Result<Value> {
 
 fn send_to_current_layer(s: &mut Session, _: &Value) -> Result<Value> {
     let ids = selected_roots(s)?;
-    let layer = s.doc()?.active_layer.ok_or_else(|| EngineError::Other("no current layer".into()))?;
+    // Ids are reused after undo: the remembered current layer must still be a layer.
+    let st = s.doc()?;
+    let layer = st.current_layer().or_else(|| st.doc.default_layer()).ok_or_else(|| EngineError::Other("no current layer".into()))?;
     s.edit("Send to Current Layer", |d, _| {
         for id in &ids {
             d.move_node(*id, Some(layer), usize::MAX)?;

crates/engine/src/lib.rs

@@ -138,6 +138,10 @@ impl DocState {
             .unwrap_or_else(|| self.doc.title.clone())
     }
     /// Where new art is inserted: the isolation container, else the active layer.
+    /// The current layer, if the remembered id still names a layer (ids are reused after undo).
+    pub fn current_layer(&self) -> Option<NodeId> {
+        self.active_layer.filter(|l| self.doc.node(*l).is_some_and(|n| n.is_layer()))
+    }
     pub fn insertion_parent(&self) -> Option<NodeId> {
         // Ids are reused after undo (the id counter is part of the document), so a remembered
         // layer or isolated group must still be one: a pasted group must never land in a path.

crates/engine/src/tests.rs

@@ -357,3 +357,42 @@ fn reshape_moves_the_grabbed_point_and_its_neighbourhood() {
     assert!(tl.y > -40.0);
     assert!(pts.iter().filter(|p| (p.y - 100.0).abs() < 1e-9).count() == 2);
 }
+
+#[test]
+fn graph_tool_drag_creates_a_graph_and_asks_for_data() {
+    let mut s = session();
+    let v = ViewInfo::default();
+    let undo_before = s.doc().unwrap().history.undo.len();
+    s.select_tool("pieGraph", v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Down, 100.0, 100.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 300.0, 250.0), v).unwrap();
+    let ui = s.pointer(&PointerEvent::new(PointerKind::Up, 300.0, 250.0), v).unwrap();
+    assert!(ui.iter().any(|r| matches!(r, crate::UiRequest::Dialog(k, _) if k == "graphData")), "{ui:?}");
+    let st = s.doc().unwrap();
+    assert_eq!(st.history.undo.len(), undo_before + 1);
+    let g = st.doc.node(st.selection.objects[0]).unwrap();
+    assert_eq!(g.graph.as_ref().unwrap().kind, vectorcraft_doc::GraphKind::Pie);
+    assert_eq!(g.graph.as_ref().unwrap().rect, vectorcraft_geom::Rect::new(100.0, 100.0, 300.0, 250.0));
+}
+
+#[test]
+fn stale_current_layer_id_never_targets_a_reused_id() {
+    let mut s = session();
+    let a = rect(&mut s, 0.0, 0.0, 10.0, 10.0);
+    let b = rect(&mut s, 5.0, 5.0, 10.0, 10.0);
+    let layer = NodeId(s.execute("layer.new", &json!({})).unwrap()["id"].as_u64().unwrap());
+    s.execute("edit.undo", &json!({})).unwrap();
+    // The undone layer's id is handed out again, here to a compound path.
+    s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap();
+    let c = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap());
+    assert_eq!(c, layer, "precondition: id reused");
+    let d = rect(&mut s, 50.0, 50.0, 10.0, 10.0);
+    s.execute("select.set", &json!({"ids": [d.0]})).unwrap();
+    s.execute("object.arrange.sendToCurrentLayer", &json!({})).unwrap();
+    let doc = &s.doc().unwrap().doc;
+    assert!(doc.node(c).unwrap().children().unwrap().iter().all(|n| n.path_data().is_some()));
+    assert!(doc.parent_of(d).is_some_and(|p| doc.node(p).unwrap().is_layer()));
+    // layer.delete without an id must not delete the compound path.
+    s.execute("layer.delete", &json!({})).ok();
+    assert!(s.doc().unwrap().doc.node(c).is_some());
+}