Merge pull request #948 from Zetenypapp/fix/duplicate-shape-ids

# Conflicts:
#	crates/engine/src/tests.rs

ROADMAP.md

@@ -132,6 +132,7 @@ app's JSON control channel (`deckcraft --control PORT`).
 | Date | What landed |
 |---|---|
 | 2026-10-17 | Opening a file caps absurd effect sizes (glow, soft edges, shadows, reflections, on shapes, text and table text) at what Format Shape allows: a glow of 10^18 pt in a .pptx made rendering ask for 34 GB and abort |
+| 2026-10-17 | Opening a .deckcraft file whose shapes repeat an id gives the repeats fresh ids (groups, layouts and masters too), so the deck validates and edits by id find one shape |
 | 2026-10-15 | PPTX trigger sequences keep separate On Click steps, including paragraph builds; synthetic import/edit/save/reopen and headless show regressions |
 | 2026-10-15 | CLI: .txt and .md export includes table cell text, which it used to skip silently (#814) |
 | 2026-10-15 | A `.pptx` slide that can't be read is reported, not dropped in silence: `lostSlides` from `file.open`, a message and a repaired copy in the app, a warning from `deckcraft-cli` (#203) |

crates/engine/src/tests.rs

@@ -3779,3 +3779,49 @@ fn opening_a_file_caps_absurd_effect_sizes() {
         t.execute("file.render", &json!({"slide": i, "scale": 0.1})).unwrap();
     }
 }
+
+/// A .deckcraft file whose shapes repeat an id (groups included) opens with fresh ids for the
+/// repeats: the deck passes `validate`, the first shape keeps its id and none is lost.
+#[test]
+fn opening_a_file_gives_repeated_shape_ids_fresh_ones() {
+    let mut s = Session::with_new();
+    s.execute("slide.new", &json!({"layout": "blank"})).unwrap();
+    for x in [0, 100, 200, 300] {
+        s.execute("shape.insert", &json!({"preset": "rect", "rect": [x, 50, 80, 80]})).unwrap();
+    }
+    s.execute("edit.selectAll", &json!({})).unwrap();
+    s.execute("arrange.group", &json!({})).unwrap();
+    s.execute("shape.insert", &json!({"preset": "ellipse", "rect": [400, 300, 80, 80]})).unwrap();
+    let first = s.doc().unwrap().shapes()[0].id;
+    // Every shape, the group's children too, takes the group's id.
+    s.edit(|doc, sel| {
+        fn all(shapes: &mut [deckcraft_model::Shape], id: deckcraft_model::ShapeId) {
+            for sh in shapes {
+                sh.id = id;
+                if let deckcraft_model::ShapeKind::Group { children, .. } = &mut sh.kind {
+                    all(children, id);
+                }
+            }
+        }
+        all(crate::shapes_mut(doc, sel).unwrap(), first);
+        Ok(())
+    })
+    .unwrap();
+    assert!(!s.doc().unwrap().doc.validate().is_empty(), "the deck repeats ids");
+    let bytes = cmd::file::save_bytes(&s.doc().unwrap().doc, "deckcraft").unwrap();
+    let mut t = Session::new();
+    t.execute("file.openBytes", &json!({"name": "dup.deckcraft", "data": cmd::base64_encode(&bytes)})).unwrap();
+    let st = t.doc().unwrap();
+    assert_eq!(st.doc.validate(), Vec::<String>::new());
+    let slide = st.doc.slides.last().unwrap();
+    let mut ids = vec![];
+    deckcraft_model::walk(&slide.shapes, &mut |sh, _| ids.push(sh.id));
+    assert_eq!(ids.len(), 6, "the group, its four children and the ellipse");
+    assert_eq!(ids[0], first, "the first shape keeps its id");
+    assert!(ids.iter().all(|id| id.0 <= st.doc.next_id), "new ids come from the deck's counter");
+    // Each shape can be selected by its own id now.
+    let last = *ids.last().unwrap();
+    t.execute("slide.go", &json!({"index": st.doc.slides.len() - 1})).unwrap();
+    t.execute("selection.select", &json!({"id": last.0})).unwrap();
+    assert_eq!(t.doc().unwrap().selected_shapes()[0].id, last);
+}

crates/format/src/lib.rs

@@ -197,6 +197,7 @@ pub fn repair(p: &mut Presentation) {
             }
         }
     }
+    reid_duplicate_shapes(p);
     p.slide_size = sane_slide_size(p.slide_size);
     // The notes page only lays out notes, but its size divides too.
     if !(p.notes_size.width.is_finite() && p.notes_size.width > 0.0 && p.notes_size.height.is_finite() && p.notes_size.height > 0.0) {
@@ -205,6 +206,57 @@ pub fn repair(p: &mut Presentation) {
     sane_numbers(p);
 }
 
+/// Give a shape that repeats an id already used on its slide, layout or master (groups included)
+/// a fresh one, so edits, selection and the Selection Pane find one shape per id. The first keeps
+/// its id, and with it what animations and connectors point at. An exhausted id space leaves the
+/// rest as they are (#407).
+fn reid_duplicate_shapes(p: &mut Presentation) {
+    fn has_duplicates(shapes: &[deckcraft_model::Shape]) -> bool {
+        let mut seen = std::collections::HashSet::new();
+        let mut dup = false;
+        deckcraft_model::walk(shapes, &mut |sh, _| dup |= !seen.insert(sh.id));
+        dup
+    }
+    fn reid(shapes: &mut [deckcraft_model::Shape], seen: &mut std::collections::HashSet<deckcraft_model::ShapeId>, next: &mut u32, depth: usize) {
+        if depth > 64 {
+            return;
+        }
+        for sh in shapes {
+            if !seen.insert(sh.id)
+                && let Some(id) = (*next).max(1).checked_add(1)
+            {
+                *next = id;
+                sh.id = deckcraft_model::ShapeId(id);
+                seen.insert(sh.id);
+            }
+            if let deckcraft_model::ShapeKind::Group { children, .. } = &mut sh.kind {
+                reid(children, seen, next, depth + 1);
+            }
+        }
+    }
+    let mut next = p.next_id;
+    let mut fix = |shapes: &mut Vec<deckcraft_model::Shape>| {
+        if has_duplicates(shapes) {
+            reid(shapes, &mut std::collections::HashSet::new(), &mut next, 0);
+        }
+    };
+    for s in &mut p.slides {
+        if has_duplicates(&s.shapes) {
+            fix(&mut Arc::make_mut(s).shapes);
+        }
+    }
+    for m in p.masters.iter_mut().chain(p.notes_master.iter_mut()).chain(p.handout_master.iter_mut()) {
+        if has_duplicates(&m.shapes) || m.layouts.iter().any(|l| has_duplicates(&l.shapes)) {
+            let m = Arc::make_mut(m);
+            fix(&mut m.shapes);
+            for l in &mut m.layouts {
+                fix(&mut l.shapes);
+            }
+        }
+    }
+    p.next_id = next;
+}
+
 /// Smallest and largest slide side, in points: PowerPoint's 1–56 in, as Design › Slide Size allows.
 pub const SLIDE_SIDE: std::ops::RangeInclusive<f64> = 72.0..=4032.0;