From 9fbc7dd81838783006647b0faf97c447165b9b6b Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Thu, 22 Oct 2026 14:37:05 +0200 Subject: [PATCH] :bug: Keep component sync with each user state transition Emit changes-applied with the commit's prior file data. Sync each commit before a later edit can replace its state or separate its undo group. Allow later commits through while completion barriers finish (F110). Process layout batches after their buffer closes, so reentrant component sync cannot lose a pending layout task. Cover three user edits without an intermediate wait, then every undo and redo. Token and tiny-layout cases also pass. Re-record golden cases X and AJ for the settled timing. Frontend: 1495 tests, 7255 assertions pass; lint and format checks pass. The three-edit regression failed before the fix. Benchmark vs Phase 1e (lower quantile, ms): world baseline now ratio main-10 1.790 0.993 0.55 main-100 15.415 9.038 0.59 main-1000 153.086 90.145 0.59 three-levels 35.215 18.989 0.54 chain 68.931 35.045 0.51 library-500 83.083 85.830 1.03 many-shapes 4.837 3.344 0.69 variant-100 32.052 33.586 1.05 All worlds pass the 10% regression threshold. AI-assisted-by: GPT-6 --- .../workspace-state-persistence-subtleties.md | 6 ++ frontend/src/app/main/data/changes.cljs | 6 ++ .../app/main/data/workspace/libraries.cljs | 77 +++++++------------ .../app/main/data/workspace/shape_layout.cljs | 3 + .../composable_tests/golden_master.edn | 4 +- .../data/workspace_sync_barrier_test.cljs | 17 ++++ 6 files changed, 60 insertions(+), 53 deletions(-) diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index eee6de48b8..594df35e2d 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -20,6 +20,12 @@ - `commit-changes` refuses to create commits unless `:permissions :can-edit` is true. It captures file revn/vern, selected-before, features, tags, undo group, and translation flag into a `::commit` event. A refusal answers with an empty stream, never nil, so callers subscribe uniformly instead of branching on nil. - Applying a remote commit first rolls back pending local commits, applies the remote changes, then replays pending local redo changes. Index updates are emitted for undo, remote redo, and replayed redo paths. - Local commits are independently consumed by undo, persistence, WASM model updates, thumbnail/library watchers, and text position-data recalculation. +- `dch/changes-applied?` follows local application and carries the commit + with its prior file data. The component watcher syncs each transition + before the next user commit, keeping undo groups adjacent. Its barrier + remains open through the event turn. Foreign file cascades stay in common. +- Layout batches cross an `:asap` boundary before reflow so synchronous + component sync cannot emit into the buffer being closed and lose a task. - Persistence buffers local commits: status becomes pending after about 200ms, commits are flushed after about 3s or `::force-persist`, and buffered commits are merged per file before `:update-file`. - Persistence sends revn as the max of the commit revn and locally tracked latest revn; remote commits update that revn tracker. - Persistence is skipped in version preview/read-only mode or without edit permission. diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index ee8eaa33f3..5e9f35be91 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -35,6 +35,9 @@ (def commit? (ptk/type? ::commit)) +(def changes-applied? + (ptk/type? ::changes-applied)) + (defn- fix-page-id "For events that modifies the page, page-id does not comes as a property so we assign it from the `id` property." @@ -203,6 +206,9 @@ (get-pending-commits state))] (rx/concat (rx/of (apply-changes-localy commit pending)) + (rx/of (ptk/data-event ::changes-applied + {:commit commit + :old-data (get-in state [:files file-id :data])})) (if pending (rx/concat (->> (rx/from (reverse pending)) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 6f7cde8c23..0b973ccaa3 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -57,7 +57,6 @@ [app.main.features :as features] [app.main.features.pointer-map :as fpmap] [app.main.fonts :as fonts] - [app.main.refs :as refs] [app.main.repo :as rp] [app.main.router :as rt] [app.main.store :as st] @@ -1399,31 +1398,19 @@ (rx/filter (fn [event-type] (or (= ::dwpg/finalize-page event-type) (= ::watch-component-changes event-type))))) - workspace-data-s - (->> (rx/from-atom refs/workspace-data {:emit-current-value? true}) - (rx/share)) - - ;; Pair every commit with the file data right before it - ;; (so deleted shapes still exist when we inspect the change). - workspace-buffer-s - (->> (rx/concat - (rx/take 1 workspace-data-s) - workspace-data-s) - (rx/buffer 2 1) - (rx/map first)) - - ;; Barriers open before async inspection and close after detection. + ;; Barriers cover detection and the derived work of each commit. ;; Every local commit opens one, so a wait covers the watcher's ;; decision for any commit: sync, only touch, or drop. pending-sync-barriers* (atom #{}) start-sync-barrier - (fn [{:keys [file-id] :as event}] - (let [task (when (uuid? file-id) - (wrf/start! :sync-file [file-id]))] + (fn [{:keys [commit old-data]}] + (let [file-id (:file-id commit) + task (when (uuid? file-id) + (wrf/start! :sync-file [file-id]))] (when task (swap! pending-sync-barriers* conj task)) - [event task])) + [commit task old-data])) finish-sync-barrier! (fn [task] @@ -1431,17 +1418,11 @@ (wrf/finish! task) (swap! pending-sync-barriers* disj task))) - ;; Each batch entry is [[event task] old-data]. - finish-batch-barriers! - (fn [batch] - (doseq [[[_ task] _] batch] - (finish-sync-barrier! task))) - local-commits-s (->> stream - (rx/filter dch/commit?) + (rx/filter dch/changes-applied?) (rx/map deref) - (rx/filter #(= :local (:source %))) + (rx/filter #(= :local (get-in % [:commit :source]))) ;; Keep waits pending while component changes are checked. (rx/map start-sync-barrier) (rx/share)) @@ -1451,10 +1432,10 @@ ;; corrective commits (font-load selrect fix, position-data regen) ;; are not user component edits. dropped-commit? - (fn [[{:keys [translation? skip-component-sync? changes]} _]] + (fn [[{:keys [translation? skip-component-sync? changes]} _ old-data]] (or skip-component-sync? (and translation? - (moves-whole-mains? @refs/workspace-data changes)))) + (moves-whole-mains? old-data changes)))) dropped-commits-s (->> local-commits-s @@ -1465,22 +1446,13 @@ commits-s (->> local-commits-s (rx/filter (complement dropped-commit?)) - ;; Paired as it arrives, so later commits in the same batch - ;; cannot hide the shapes this one moved or deleted. - (rx/with-latest-from workspace-buffer-s) (rx/share)) - ;; Buffers commits until a timer turn passes with no new ones. The - ;; debounce timer (not a microtask) queues behind any timers already - ;; due, so a burst of commits is inspected as one batch in one task. - commit-batches-s - (rx/buffer-until (rx/debounce 0 commits-s) commits-s) - get-component-changes (fn [old-data {:keys [file-id changes save-undo? undo-group]}] (let [changed-components (when (and old-data - (or (nil? file-id) (= file-id (:id old-data)))) + (= file-id (:current-file-id state) (:id old-data))) (into #{} (mapcat (partial ch/components-changed old-data)) changes))] @@ -1500,14 +1472,11 @@ ;; Undos only bump :modified-at. (map #(vector ::touch-component %) changed-components)))) - ;; One event per distinct component change in the batch, each - ;; commit checked against the file data right before it + ;; Sync each transition before a later user commit can replace its + ;; state or separate its sync from its undo group. get-component-events - (fn [batch] - (->> batch - (mapcat (fn [[[event _] old-data]] - (get-component-changes old-data event))) - (distinct) + (fn [event old-data] + (->> (get-component-changes old-data event) (map (fn [[type component-id file-id undo-group]] (if (= type ::component-changed) (component-changed component-id file-id undo-group) @@ -1515,11 +1484,17 @@ (rx/from))) component-events-s - (->> commit-batches-s - (rx/mapcat - (fn [batch] - (->> (get-component-events batch) - (rx/finalize #(finish-batch-barriers! batch))))) + (->> commits-s + ;; Barrier completion is async; it must not queue later + ;; commits behind the previous transition. + (rx/merge-map + (fn [[event task old-data]] + (->> (rx/concat + (get-component-events event old-data) + ;; Keep the completion barrier through the current + ;; event turn, including derived layout work. + (->> (rx/of nil) (rx/observe-on :async) (rx/ignore))) + (rx/finalize #(finish-sync-barrier! task))))) ;; Close barriers left behind when the page shuts down. (rx/finalize #(wrf/finish-tasks! @pending-sync-barriers*)) (rx/share)) diff --git a/frontend/src/app/main/data/workspace/shape_layout.cljs b/frontend/src/app/main/data/workspace/shape_layout.cljs index 16d9bc027b..0983e2c523 100644 --- a/frontend/src/app/main/data/workspace/shape_layout.cljs +++ b/frontend/src/app/main/data/workspace/shape_layout.cljs @@ -116,6 +116,9 @@ (rx/map #(assoc % ::reflow-task (wrf/start! :layout (:ids %)))) (rx/buffer-time 100) (rx/filter #(d/not-empty? %)) + ;; Open the next buffer before a reflow's synchronous component + ;; sync can request more layout work. + (rx/observe-on :asap) ;; The updates of a page reflow together, in the undo group of ;; the step that asked for them: the one an update names, as ;; those with none come from the same step. diff --git a/frontend/test/frontend_tests/composable_tests/golden_master.edn b/frontend/test/frontend_tests/composable_tests/golden_master.edn index c2cadbeddf..cae54ad46f 100644 --- a/frontend/test/frontend_tests/composable_tests/golden_master.edn +++ b/frontend/test/frontend_tests/composable_tests/golden_master.edn @@ -30,7 +30,7 @@ {:transcript "one-of -> \n {:target :copy-root, :value 500}", :states ["000943ecb6c266ee" "209fe67d76a27a9b"]} {:transcript "one-of -> \n one-of -> ", :states ["94318a628db305b6"]}] :case-aj-reflow-after-nested-sync-is-its-own-undo-step - [{:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:times 1}", :states ["43ca8b2de80a51fc" "32b45a5ac7536b08" "7104bfaccd42cd3b"]}] + [{:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:times 1}", :states ["43ca8b2de80a51fc" "ce4ef0e73faa0894" "ef4de3d8d773582c"]}] :case-ak-update-main-pushes-grid-cells [{:transcript " {:target :copy-child, :to :other, :keep-touched? false}\n {:board :copy-root, :child :copy-child, :row 1, :column 3}\n {:target :copy-root}", :states ["5a6af6c0b8f86ac3" "278e4ff6bf0df746" "322e03c88f4a9dce" "432cfef7e9b05164"]} {:transcript "one-of -> \n {:board :copy-root, :child :copy-child, :row 1, :column 3}\n {:target :copy-root}", :states ["9900a7f71dc0cb5f" "de66ab1c3d4cc157" "6b5ff9fd86e2aa87"]}] @@ -205,7 +205,7 @@ {:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:target #fn, :property :fills, :value \"#00ff00\"}\n {:target #fn, :property :fills, :value \"#aaaaaa\"}\n {:times 3}", :states ["08a251a3907c6037" "3dff1bfab5a5b77d" "b5df4ace7209f759" "cd9d65b6e9e44dd4" "ddea124a9ab145f1"]} {:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:target #fn, :property :fills, :value \"#00ff00\"}\n {:target #fn, :property :fills, :value \"#aaaaaa\"}\n {:times 3}", :states ["4220f3cd46b6b642" "180cc6a41d003bb5" "9fa58862bf863db6" "3f2266fbef201c71" "32c65c945efec2cd"]}] :case-x-undo-before-sync-leaves-copies-unchanged - [{:transcript " {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["734ec524d88eb6bb" "ff1fa863b88027e5"]}] + [{:transcript " {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["c6423d10a4e5e278" "ac9279bfdfb5c7e5"]}] :case-y-text-override-merges-with-main-text-change [{:transcript "one-of -> \n {:target :copy-instance, :edit :text, :value \"copy text\"}\n {:target :main-instance, :edit :text, :value \"main text\"}", :states ["7ea87003598e5626" "f823f8147e941701" "3a6ecb19a9315cb8"]} {:transcript "one-of -> \n {:target :copy-instance, :edit :text, :value \"copy text\"}\n {:target :main-instance, :edit :font-size, :value \"24\"}", :states ["7ea87003598e5626" "bc06b54fa303bde9" "230d1c77384fc557"]} diff --git a/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs index 97b6a0001b..789d36da50 100644 --- a/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs @@ -15,6 +15,7 @@ [app.config :as cf] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.reflow :as wrf] + [app.main.data.workspace.undo :as dwu] [app.main.store :as st] [app.plugins.reflow :as pwrf] [beicon.v2.core :as rx] @@ -27,6 +28,22 @@ (def ^:private red "#ff0000") +(t/deftest ^:async consecutive-main-edits-undo-and-redo-with-their-copies + (ftm/install! (setup/simple-component-with-labeled-copy)) + (st/emit! (ftm/fill-commit :main-child red {})) + (st/emit! (ftm/fill-commit :main-child "#00ff00" {})) + (st/emit! (ftm/fill-commit :main-child "#0000ff" {})) + (t/is (= :settled (await (ftm/await-step [])))) + (doseq [[event color] [[dwu/undo "#00ff00"] + [dwu/undo red] + [dwu/undo setup/labeled-fill] + [dwu/redo red] + [dwu/redo "#00ff00"] + [dwu/redo "#0000ff"]]] + (t/is (= :settled (await (ftm/await-step [event])))) + (t/is (= color (ftm/fill-of :main-child))) + (t/is (= color (ftm/fill-of :copy-child))))) + (defn- sync-pending? [file-id] (contains? (get (wrf/pending) file-id) :sync-file))