From a62a15206d2934a4b5ded6292615108d5002ddd4 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Tue, 27 Oct 2026 10:16:15 +0100 Subject: [PATCH] :bug: Keep delayed copy measurements with their undo group Route non-stacked derived commits to their applied group when a later edit or transaction intervenes. Copy remeasurement must undo with the font action rather than becoming a separate step after an unrelated edit. Keep adjacent non-stacked entries and exclude the redo tail from routing. Extend the font-boundary regression to component mains and copies, and cover both stacking modes during a later transaction. The regression fails before the fix; all 1,563 frontend tests and 10,880 assertions pass. Live WASM history passes exact undo/redo and full-sync consistency. The sync core is unchanged by this undo fix. Its preceding benchmark ratios remain 0.48-0.76 of the unchanged baseline, with no regression flag. AI-assisted-by: gpt-6 --- .../workspace-state-persistence-subtleties.md | 12 +- .../src/app/main/data/workspace/undo.cljs | 11 +- .../completion_contract_test.cljs | 108 ++++++++++-------- .../data/workspace_undo_test.cljs | 59 +++++----- 4 files changed, 101 insertions(+), 89 deletions(-) diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 9d0551783d..909ba9a8e2 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -55,12 +55,12 @@ inverse sync and all copy syncs. Deferred text reflow may commit after the transaction closes; its changes must still join the user action (F152). - Undo transactions can stay open only temporarily; timed-out pending transactions are force-committed after about 20s. Undo entries are capped at 50. -- Stacked derived changes first look for their own group among applied undo - entries (F214). A delayed font resize must not join a later unrelated - edit or transaction. Preserve an older entry's selection; never search - its redo tail. Without a matching group, stacking keeps its current-entry - behavior. Non-stacked delayed copy measurement is tracked separately as - F222 while its async regression and routing correction are pending. +- Derived changes first look for their own group among applied undo entries + (F214, F222). Delayed font and copy measurements extend that group across + later edits or transactions, even without `:stack-undo?`. Preserve an + older entry's selection; never search its redo tail. Adjacent non-stacked + changes keep separate entries; without a matching group, stacking keeps + its current-entry behavior. - Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path. - After local commits and when render-wasm is active, text shapes get derived `:position-data` recomputed in a separate commit tagged `#{:position-data}`; that tag is excluded from the position-data watcher to avoid loops. - WASM component thumbnails cancel the entire upload/render stream on diff --git a/frontend/src/app/main/data/workspace/undo.cljs b/frontend/src/app/main/data/workspace/undo.cljs index 60d1bd340a..72c0de3c6a 100644 --- a/frontend/src/app/main/data/workspace/undo.cljs +++ b/frontend/src/app/main/data/workspace/undo.cljs @@ -161,9 +161,9 @@ (assoc-in [:workspace-undo :transaction :tags] tags))) (defn append-undo - "Add changes to undo history. Stacked changes extend their applied undo - group when present, otherwise the current entry. An open transaction - accumulates changes unless they stack into another applied group." + "Add changes to undo history. Delayed changes extend their applied undo + group across later edits or transactions, even without stacking. + Stacked changes without a matching group extend the current entry." [entry stack?] (assert (check-undo-entry entry)) @@ -176,7 +176,7 @@ entry (assoc entry :selected-after selected-after) index (get-in state [:workspace-undo :index] -1) transaction (get-in state [:workspace-undo :transaction]) - group-index (when (and stack? (:undo-group entry) + group-index (when (and (:undo-group entry) (not= (:undo-group entry) (:undo-group transaction))) (last (keep-indexed (fn [index item] @@ -185,7 +185,8 @@ (cond ;; A delayed derived change still belongs to its primary action, ;; even after another edit or transaction has started. - (some? group-index) + (and (some? group-index) + (or stack? (< group-index index) transaction)) (stack-undo-entry state entry group-index) (and (get-in state [:workspace-undo :transaction]) diff --git a/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs b/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs index 7720cf63d3..ddec2fd30f 100644 --- a/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs +++ b/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs @@ -182,57 +182,67 @@ (st/emit! #(assoc % :features features)))))) (t/deftest ^:async a-delayed-font-resize-stays-with-its-font-edit - (let [features (:features @st/state) - ready (atom true) - face {:font-id "gfont-roboto" :weight 400 :style 0 :emoji? false}] - (thw/setup-wasm-mocks!) - (try - (await - (mock/with-mocks* - {wasm/text-editor-has-focus? (mock/stub (constantly false)) - wasm.fonts/get-content-fonts (mock/stub (constantly [face])) - wasm.fonts/make-font-data (mock/stub identity) - wasm.fonts/font-ready? (mock/stub (fn [_] @ready)) - wasm/get-text-dimensions (mock/stub (fn [] {:x 0 :y 0 :width 100 :max-width 100 - :height (if @ready 40 20)})) - dwthw/render-thumbnail (mock/stub (fn [& _] (ptk/data-event ::thumbnail-skipped)))} - (let [file (-> (thf/sample-file :file1) + (doseq [component? [false true]] + (let [features (:features @st/state) + ready (atom true) + face {:font-id "gfont-roboto" :weight 400 :style 0 :emoji? false} + measuring (atom nil)] + (thw/setup-wasm-mocks!) + (try + (await + (mock/with-mocks* + {wasm/text-editor-has-focus? (mock/stub (constantly false)) + wasm.fonts/get-content-fonts (mock/stub (constantly [face])) + wasm.fonts/make-font-data (mock/stub identity) + wasm.fonts/font-ready? (mock/stub (fn [_] @ready)) + wasm/set-shape-text-content (mock/stub (fn [id _] (reset! measuring id))) + wasm/get-text-dimensions (mock/stub (fn [] {:x 0 :y 0 :width 100 :max-width 100 + :height (if @ready + (if (= @measuring (thi/id :copy-child)) 50 40) + 20)})) + dwthw/render-thumbnail (mock/stub (fn [& _] (ptk/data-event ::thumbnail-skipped)))} + (let [file (cond-> (thf/sample-file :file1) + component? + (thtx/add-text-component-with-copy [[["Read the guide"]]]) + (not component?) (ths/add-sample-shape :text :type :text :width 100 :height 20 :grow-type :auto-height - :content (thtx/content [[["Read the guide"]]])) - (ths/add-sample-shape :rect :type :rect :opacity 1)) - _ (ftm/install! (tm/make-situation file)) - before (ftm/snapshot)] - (st/emit! #(update % :features (fnil conj #{}) "render-wasm/v1")) - (reset! ready false) - (st/emit! (dwt/update-all-attrs [(thi/id :text)] - {:font-id "gfont-roboto" :font-family "Roboto" - :font-variant-id "regular"})) - (await (ha/wait-for #(some (comp seq :font) (vals (wrf/pending))) - "font edit is pending")) - (st/emit! (dwsh/update-shapes [(thi/id :rect)] #(assoc % :opacity 0.4))) - (await (ha/wait-for #(= 0.4 (:opacity (ths/get-shape (ftm/current-file) :rect))) - "unrelated edit commits before the font arrives")) - (let [text (ths/get-shape (ftm/current-file) :text) - faces (map (comp wasm.fonts/font-data-key wasm.fonts/make-font-data) - (wasm.fonts/get-content-fonts (:content text)))] - (t/is (seq faces) "the font wait has a face to release") - (reset! ready true) - (doseq [face faces] (rx/push! wasm.fonts/font-stored-stream face))) - (t/is (= :settled (await (ftm/await-step [])))) - (t/is (mth/close? 40 (:height (ths/get-shape (ftm/current-file) :text)))) - (let [after (ftm/snapshot)] - (t/is (= :settled (await (ftm/await-step [dwu/undo])))) - (t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :rect)))) - (t/is (mth/close? 40 (:height (ths/get-shape (ftm/current-file) :text))) - "undoing the unrelated edit keeps the font's measured height") - (t/is (= :settled (await (ftm/await-step [dwu/undo])))) - (t/is (uc/same-state? before (ftm/snapshot))) - (t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo])))) - (t/is (uc/same-state? after (ftm/snapshot))))))) - (finally - (thw/teardown-wasm-mocks!) - (st/emit! #(assoc % :features features)))))) + :content (thtx/content [[["Read the guide"]]]))) + text-id (thi/id (if component? :main-child :text)) + file (-> file + (ths/add-sample-shape :rect :type :rect :opacity 1)) + _ (ftm/install! (tm/make-situation file)) + before (ftm/snapshot)] + (st/emit! #(update % :features (fnil conj #{}) "render-wasm/v1")) + (reset! ready false) + (st/emit! (dwt/update-all-attrs [text-id] + {:font-id "gfont-roboto" :font-family "Roboto" + :font-variant-id "regular"})) + (await (ha/wait-for #(some (comp seq :font) (vals (wrf/pending))) + "font edit is pending")) + (st/emit! (dwsh/update-shapes [(thi/id :rect)] #(assoc % :opacity 0.4))) + (await (ha/wait-for #(= 0.4 (:opacity (ths/get-shape (ftm/current-file) :rect))) + "unrelated edit commits before the font arrives")) + (let [text (ths/get-shape-by-id (ftm/current-file) text-id) + faces (map (comp wasm.fonts/font-data-key wasm.fonts/make-font-data) + (wasm.fonts/get-content-fonts (:content text)))] + (t/is (seq faces) "the font wait has a face to release") + (reset! ready true) + (doseq [face faces] (rx/push! wasm.fonts/font-stored-stream face))) + (t/is (= :settled (await (ftm/await-step [])))) + (t/is (mth/close? 40 (:height (ths/get-shape-by-id (ftm/current-file) text-id)))) + (let [after (ftm/snapshot)] + (t/is (= :settled (await (ftm/await-step [dwu/undo])))) + (t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :rect)))) + (t/is (mth/close? 40 (:height (ths/get-shape-by-id (ftm/current-file) text-id))) + "undoing the unrelated edit keeps the font's measured height") + (t/is (= :settled (await (ftm/await-step [dwu/undo])))) + (t/is (uc/same-state? before (ftm/snapshot))) + (t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo])))) + (t/is (uc/same-state? after (ftm/snapshot))))))) + (finally + (thw/teardown-wasm-mocks!) + (st/emit! #(assoc % :features features))))))) (t/deftest ^:async rotation-nodes-compose-successive-turns (let [initial (ftm/start-generated (setup/simple-component-with-labeled-copy)) diff --git a/frontend/test/frontend_tests/data/workspace_undo_test.cljs b/frontend/test/frontend_tests/data/workspace_undo_test.cljs index a687ba5ec2..5dd84b62ce 100644 --- a/frontend/test/frontend_tests/data/workspace_undo_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_undo_test.cljs @@ -60,35 +60,36 @@ (get-in @st/state [:workspace-undo :index])) (t/deftest ^:async delayed-derived-changes-do-not-join-an-unrelated-transaction - (let [file (-> (thf/sample-file :file) - (ths/add-sample-shape :first :type :rect :opacity 1) - (ths/add-sample-shape :second :type :rect :opacity 1)) - _ (ftm/install! (tm/make-situation file)) - before (ftm/snapshot) - first-group (uuid/next) - second-group (uuid/next) - transaction (js/Symbol) - edit (fn [label attr value opts] - (let [page (thf/current-page (ftm/current-file)) - changes (cls/generate-update-shapes - (pcb/empty-changes nil (:id page)) [(thi/id label)] - #(assoc % attr value) (:objects page) {})] - (dch/commit-changes (merge changes opts))))] - (t/is (= :settled (await (ftm/await-step [(edit :first :opacity 0.4 {:undo-group first-group})])))) - (t/is (= :settled - (await (ftm/await-step [(dwu/start-undo-transaction transaction) - (edit :second :opacity 0.5 {:undo-group second-group}) - (edit :first :r1 8 {:undo-group first-group :stack-undo? true}) - (dwu/commit-undo-transaction transaction)])))) - (let [after (ftm/snapshot)] - (t/is (= [first-group second-group] (mapv :undo-group (get-in @st/state [:workspace-undo :items])))) - (t/is (= :settled (await (ftm/await-step [dwu/undo])))) - (t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :second)))) - (t/is (= 8 (:r1 (ths/get-shape (ftm/current-file) :first)))) - (t/is (= :settled (await (ftm/await-step [dwu/undo])))) - (t/is (uc/same-state? before (ftm/snapshot))) - (t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo])))) - (t/is (uc/same-state? after (ftm/snapshot)))))) + (doseq [stack? [true false]] + (let [file (-> (thf/sample-file :file) + (ths/add-sample-shape :first :type :rect :opacity 1) + (ths/add-sample-shape :second :type :rect :opacity 1)) + _ (ftm/install! (tm/make-situation file)) + before (ftm/snapshot) + first-group (uuid/next) + second-group (uuid/next) + transaction (js/Symbol) + edit (fn [label attr value opts] + (let [page (thf/current-page (ftm/current-file)) + changes (cls/generate-update-shapes + (pcb/empty-changes nil (:id page)) [(thi/id label)] + #(assoc % attr value) (:objects page) {})] + (dch/commit-changes (merge changes opts))))] + (t/is (= :settled (await (ftm/await-step [(edit :first :opacity 0.4 {:undo-group first-group})])))) + (t/is (= :settled + (await (ftm/await-step [(dwu/start-undo-transaction transaction) + (edit :second :opacity 0.5 {:undo-group second-group}) + (edit :first :r1 8 {:undo-group first-group :stack-undo? stack?}) + (dwu/commit-undo-transaction transaction)])))) + (let [after (ftm/snapshot)] + (t/is (= [first-group second-group] (mapv :undo-group (get-in @st/state [:workspace-undo :items])))) + (t/is (= :settled (await (ftm/await-step [dwu/undo])))) + (t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :second)))) + (t/is (= 8 (:r1 (ths/get-shape (ftm/current-file) :first)))) + (t/is (= :settled (await (ftm/await-step [dwu/undo])))) + (t/is (uc/same-state? before (ftm/snapshot))) + (t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo])))) + (t/is (uc/same-state? after (ftm/snapshot))))))) (t/deftest ^:async undo-and-redo-an-entry-without-undo-group (ftm/install! (setup/simple-component-with-labeled-copy))