From d9c2be1994c47ac436c9afff8b696b7e3b5d2e73 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Sat, 24 Oct 2026 01:07:22 +0200 Subject: [PATCH] :bug: Restore sibling order when undoing component child removals Bulk removal inverses move siblings back in original order. Component sync records each removal's current index; inverse sync falls back to the original main container when the builder lacks it (F158). A six-permutation deletion test and a main-swap sync test fail before the fix. Focused suites pass 70 tests and 242 assertions; two inverse-sync regressions also pass. Common lint and format pass. The controlled benchmark has no flags: mains 0.51/0.59/0.58; levels 0.54; chain 0.58; library 0.55; many shapes 0.51; switch 0.63. The first measurement under concurrent stress load flagged main-100 at 1.17 and was rerun in isolation. AI-assisted-by: GPT-6 --- .../memories/common/changes-architecture.md | 7 +++++ .../src/app/common/files/changes_builder.cljc | 5 +++- common/src/app/common/logic/libraries.cljc | 4 ++- .../common_tests/logic/swap_group_test.cljc | 29 +++++++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/.serena/memories/common/changes-architecture.md b/.serena/memories/common/changes-architecture.md index d5b182a33e..92be62f865 100644 --- a/.serena/memories/common/changes-architecture.md +++ b/.serena/memories/common/changes-architecture.md @@ -42,3 +42,10 @@ To inspect what a UI action emitted, use `mem:frontend/cljs-repl` with the snipp `:remote-synced` as well as `:touched`. Removing or adding a child clears the former even when touched groups do not change; restoring only the groups loses prior state on undo (F157). + +- Undo of bulk removals moves restored siblings in original sibling order. + Component sync records each removal's current index from builder objects; + indexes in the original container become stale after earlier removals. + When the builder lacks a main's container (inverse sync), use that main + container's original index. Both rules preserve order after several + siblings disappear (F158). diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index e920a49cf8..c0379dc3fd 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -731,7 +731,10 @@ (:remote-synced parent)) (conj (restore-parent-state-change page-id parent))))) $ (distinct (keep (fn [id] (:parent-id (get objects id))) ids))) - (reduce add-undo-change-parent $ ids) + ;; Moves undo in sibling order: each preceding + ;; sibling must be in place before its follower. + (reduce add-undo-change-parent $ + (reverse (sort-by (partial cfh/get-position-on-parent objects) (seq ids)))) (reduce add-undo-change-shape $ ids)))))) (defn remove-objects diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 6fbb120183..a60e083bbb 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -823,7 +823,9 @@ (pcb/add-changes changes container (conj (mapv del-obj-change ids) {:type :reg-objects :shapes (vec parents)}) - (map #(add-obj-change (get objects %) (cfh/get-position-on-parent objects %)) + (map #(add-obj-change (get objects %) + (or (cfh/get-position-on-parent (pcb/get-objects changes) %) + (cfh/get-position-on-parent objects %))) ids)))) (defn- longest-increasing diff --git a/common/test/common_tests/logic/swap_group_test.cljc b/common/test/common_tests/logic/swap_group_test.cljc index 46d38cad27..e7bc4676ec 100644 --- a/common/test/common_tests/logic/swap_group_test.cljc +++ b/common/test/common_tests/logic/swap_group_test.cljc @@ -848,3 +848,32 @@ undone (thf/apply-undo-changes changed changes)] (t/is (not (:remote-synced (ths/get-shape changed :copy-star))) (name operation)) (t/is (= (thn/normalize-file file) (thn/normalize-file undone)) (name operation))))) + +(t/deftest removing-several-siblings-restores-their-order-on-undo + (let [file (reduce (fn [file label] (tho/add-rect file label {:parent-label :parent})) + (tho/add-frame (thf/sample-file :file1) :parent) [:a :b :c :d :e]) + page (thf/current-page file)] + (doseq [labels [[:b :c :d] [:b :d :c] [:c :b :d] [:c :d :b] [:d :b :c] [:d :c :b]]] + (let [changes (-> (pcb/empty-changes nil (:id page)) + (pcb/with-objects (:objects page)) + (pcb/remove-objects (mapv thi/id labels))) + removed (thf/apply-changes file changes) + undone (thf/apply-undo-changes removed changes)] + (t/is (= (:shapes (ths/get-shape file :parent)) + (:shapes (ths/get-shape undone :parent))) (pr-str labels)))))) + +(t/deftest syncing-a-main-swap-restores-all-removed-siblings-in-order-on-undo + (let [file (-> (thf/sample-file :file1) + (tho/add-simple-component :star :star-main :star-rect) + (tho/add-rect :extra-a {:parent-label :star-main}) + (tho/add-rect :extra-b {:parent-label :star-main}) + (tho/add-rect :extra-c {:parent-label :star-main}) + (tho/add-simple-component :heart :heart-main :heart-rect) + (tho/add-frame :card-main) + (thc/instantiate-component :star :card-star :parent-label :card-main) + (thc/make-component :card :card-main) + (thc/instantiate-component :card :copy :children-labels [:copy-star])) + initial (tm/make-situation file) + swapped (-> initial (r/step (n/swap-shape :card-star :heart)) r/settle) + undone (r/step swapped (n/undo))] + (t/is (= (thn/normalize-file file) (thn/normalize-file (tm/file undone))))))