⚡ Stop resending deleted component objects on every touch

`pcb/update-component` attached the whole `:objects` map of a deleted
component to every `mod-component` change, so a single component swap
re-sent thousands of shapes that the same batch already carried as
shape changes. In one measured swap the 49 maps with objects were 94%
of the request (6.3 MB), and only 27% of the 4.275 shapes they carried
differed from what the server had.

Now `mod-component` carries component metadata only, and shapes inside
a component travel as `:mod-obj` changes scoped with `:component-id`,
which is what the component sync already emits. An absent `:objects`
key means "leave the stored shapes alone", only an explicit `nil`
drops them, so metadata-only changes cannot wipe a deleted component.

The two repairs that fix shapes inside a component (shape-ref cycle
and duplicated swap slot) now emit one `:mod-obj` per shape; the two
that move or clear the `:objects` key keep the whole-map write behind
the new `:objects? true` option.

Same swap on the measured file: 6.328.307 to 382.571 bytes.

AI-assisted-by: space-bunny

.serena/memories/common/changes-architecture.md

@@ -8,6 +8,7 @@ Each change is a map such as `{:type ... :id ... :page-id ...}`. Common families
 
 - `:add-obj`, `:mod-obj`, `:del-obj`: shape lifecycle. `:mod-obj` contains `:operations`, commonly `{:type :set :attr ... :val ... :ignore-geometry ... :ignore-touched ...}` or `{:type :set-touched ...}`.
 - `:add-component`, `:mod-component`, `:del-component`: component/library metadata.
+- Shapes stored inside the component record (deleted components keep them in `:objects`) are never written as a whole map: `mod-component` carries only component metadata by default, and shapes travel as `:mod-obj`/`:add-obj`/`:del-obj` changes scoped with `:component-id`. `pcb/update-component` has an `:objects? true` option for the two repairs that move or clear that key, and `ctkl/mod-component` reads an absent `:objects` as "leave the stored shapes alone" (only an explicit `nil` drops them). Before this, one component swap resent the full object map of every deleted component it touched (~6 MB in the request); do not reintroduce it.
 - `:add-children`, `:remove-children`, `:reg-objects`: tree and object-map edits.
 - `:set-option`, `:add-page`, `:mov-page`, and related file/page metadata changes.
 
@@ -21,7 +22,7 @@ High-value builder operations:
 - `pcb/with-page-id`, `pcb/with-objects`, `pcb/with-library-data`: set context for following operations.
 - `pcb/update-shapes ids update-fn`: emits `:mod-obj` with diff-derived `:set` ops. Options include `{:with-objects? true}`, `{:ignore-touched true}`, and `{:attrs #{...}}`.
 - `pcb/add-objects`, `pcb/change-parent`, `pcb/remove-objects`, `pcb/resize-parents`: shape/tree edits.
-- `pcb/add-component`, `pcb/update-component`, `pcb/mod-component`: component/library edits.
+- `pcb/add-component`, `pcb/update-component`, `pcb/mod-component`: component/library edits. Repairs that fix shapes inside a component use `pcb/with-container` + `pcb/with-objects` + `pcb/update-shapes` so the changes come out scoped with `:component-id`.
 - `pcb/set-translation? true`: marks the whole change set as translation-only, which lets component sync skip expensive work.
 
 ## Applying changes in tests

common/src/app/common/files/changes_builder.cljc

@@ -924,7 +924,14 @@
          (apply-changes-local {:apply-to-library? apply-changes-local-library?})))))
 
 (defn update-component
-  [changes id update-fn & {:keys [apply-changes-local-library?]}]
+  "Update a component record.
+
+   By default the change carries only component metadata. Shapes stored inside
+   the component (deleted components keep them in `:objects`) travel as
+   `:mod-obj` changes scoped with `:component-id`, which is what the component
+   sync emits. Pass `:objects? true` to rewrite the whole `:objects` map, only
+   needed by the repairs that move or clear that key."
+  [changes id update-fn & {:keys [apply-changes-local-library? objects?]}]
   (assert-library! changes)
   (let [library-data   (::library-data (meta changes))
         prev-component (get-in library-data [:components id])
@@ -938,8 +945,10 @@
                                               :main-instance-id (:main-instance-id new-component)
                                               :main-instance-page (:main-instance-page new-component)
                                               :annotation (:annotation new-component)
-                                              :objects (:objects new-component) ;; for deleted components
                                               :modified-at (:modified-at new-component)}
+
+                                       objects? (assoc :objects (:objects new-component))
+
                                        (some? (:variant-id new-component))
                                        (assoc :variant-id (:variant-id new-component))
                                        (nil? (:variant-id new-component))
@@ -954,8 +963,10 @@
                                               :path (:path prev-component)
                                               :main-instance-id (:main-instance-id prev-component)
                                               :main-instance-page (:main-instance-page prev-component)
-                                              :annotation (:annotation prev-component)
-                                              :objects (:objects prev-component)}
+                                              :annotation (:annotation prev-component)}
+
+                                       objects? (assoc :objects (:objects prev-component))
+
                                        (some? (:variant-id prev-component))
                                        (assoc :variant-id (:variant-id prev-component))
                                        (nil? (:variant-id prev-component))

common/src/app/common/files/repair.cljc

@@ -26,6 +26,20 @@
 (defmulti repair-error
   (fn [code _error _file-data _libraries] code))
 
+(defn- update-component-shapes
+  "Update shapes stored inside a component `:objects` map.
+
+   Emits one `:mod-obj` change per shape, scoped with `:component-id`, instead
+   of rewriting the whole map through a `:mod-component` change. `:ignore-touched`
+   keeps the value the caller computed: no touched group is merged on top."
+  [component objects ids update-fn]
+  (-> (pcb/empty-changes nil)
+      ;; A made container, so `update-shapes` scopes the changes to the
+      ;; component instead of to a page.
+      (pcb/with-container {:id (:id component) :type :component})
+      (pcb/with-objects objects)
+      (pcb/update-shapes ids update-fn {:ignore-touched true})))
+
 (defmethod repair-error :invalid-geometry
   [_ {:keys [shape page-id] :as error} file-data _]
   (let [repair-shape
@@ -372,33 +386,26 @@
         (pcb/update-shapes [(:id shape)] repair-shape))))
 
 (defmethod repair-error :shape-ref-cycle
-  [_ {:keys [shape args] :as error} file-data _]
-  (let [repair-component
-        (fn [component]
-          (let [objects   (:objects component)
-                ;; we only have encounter this on deleted components,
-                ;; so the relevant objects are inside the component
-                to-detach (->> (:cycles-ids args)
-                               (map #(get objects %))
-                               (map #(ctn/get-head-shape objects %))
-                               (map :id)
-                               distinct
-                               (mapcat #(ctn/get-children-in-instance objects %))
-                               (map :id)
-                               set)]
+  [_ {:keys [shape args] :as error} _file-data _]
+  (let [objects   (:objects shape)
+        ;; we only have encounter this on deleted components,
+        ;; so the relevant objects are inside the component
+        to-detach (->> (:cycles-ids args)
+                       (map #(get objects %))
+                       (map #(ctn/get-head-shape objects %))
+                       (map :id)
+                       distinct
+                       (mapcat #(ctn/get-children-in-instance objects %))
+                       (map :id)
+                       set)
+
+        detach-shape
+        (fn [shape]
+          (log/debug :hint "  -> detach shape" :shape-id (:id shape))
+          (ctk/detach-shape shape))]
 
-            (update component :objects
-                    (fn [objects]
-                      (reduce-kv (fn [acc k v]
-                                   (if (contains? to-detach k)
-                                     (assoc acc k (ctk/detach-shape v))
-                                     (assoc acc k v)))
-                                 {}
-                                 objects)))))]
     (log/debug :hint "repairing component :shape-ref-cycle" :id (:id shape) :name (:name shape))
-    (-> (pcb/empty-changes nil nil)
-        (pcb/with-library-data file-data)
-        (pcb/update-component (:id shape) repair-component))))
+    (update-component-shapes shape objects to-detach detach-shape)))
 
 (defmethod repair-error :shape-ref-in-main
   [_ {:keys [shape page-id] :as error} file-data _]
@@ -540,7 +547,7 @@
     (log/debug :hint "repairing component :component-nil-objects-not-allowed" :id (:id component) :name (:name component))
     (-> (pcb/empty-changes nil)
         (pcb/with-library-data file-data)
-        (pcb/update-component (:id component) repair-component))))
+        (pcb/update-component (:id component) repair-component {:objects? true}))))
 
 (defmethod repair-error :non-deleted-component-cannot-have-objects
   [_ {component :shape} file-data _]   ; in this error the :shape argument is the component
@@ -556,7 +563,7 @@
     (log/debug :hint "repairing component :non-deleted-component-cannot-have-objects" :id (:id component) :name (:name component))
     (-> (pcb/empty-changes nil)
         (pcb/with-library-data file-data)
-        (pcb/update-component (:id component) repair-component))))
+        (pcb/update-component (:id component) repair-component {:objects? true}))))
 
 (defmethod repair-error :invalid-text-touched
   [_ {:keys [shape page-id] :as error} file-data _]
@@ -608,7 +615,7 @@
         (pcb/update-shapes (map :id child-with-duplicate) repair-shape))))
 
 (defmethod repair-error :component-duplicate-slot
-  [_ {:keys [shape] :as error} file-data _]
+  [_ {:keys [shape] :as error} _file-data _]
   (let [main-shape            (get-in shape [:objects (:main-instance-id shape)])
         childs                (map #(get (:objects shape) %) (:shapes main-shape))
         childs-with-duplicate (let [result (reduce (fn [[seen duplicates] item]
@@ -620,20 +627,14 @@
                                                    childs)]
                                 (second result))
         duplicated-ids        (set (mapv :id childs-with-duplicate))
-        repair-component
-        (fn [component]
-          (let [objects (reduce-kv (fn [acc k v]
-                                     (if (contains? duplicated-ids k)
-                                       (assoc acc k (ctk/remove-swap-slot v))
-                                       (assoc acc k v)))
-                                   {}
-                                   (:objects component))]
-            (assoc component :objects objects)))]
+        repair-shape
+        (fn [shape]
+          ;; Remove the swap slot
+          (log/debug :hint "  -> remove swap-slot" :child-id (:id shape))
+          (ctk/remove-swap-slot shape))]
 
     (log/debug :hint "repairing component :component-duplicated-slot" :id (:id shape) :name (:name shape))
-    (-> (pcb/empty-changes nil)
-        (pcb/with-library-data file-data)
-        (pcb/update-component (:id shape) repair-component))))
+    (update-component-shapes shape (:objects shape) duplicated-ids repair-shape)))
 
 (defmethod repair-error :missing-slot
   [_ {:keys [shape page-id args] :as error} file-data _]

common/src/app/common/types/components_list.cljc

@@ -41,7 +41,7 @@
       variant-properties (update-in [:components id] assoc :variant-properties variant-properties))))
 
 (defn mod-component
-  [file-data {:keys [id name path main-instance-id main-instance-page objects annotation variant-id variant-properties modified-at]}]
+  [file-data {:keys [id name path main-instance-id main-instance-page objects annotation variant-id variant-properties modified-at] :as change}]
   (d/update-in-when file-data [:components id]
                     (fn [component]
                       (let [new-comp (cond-> component
@@ -57,10 +57,14 @@
                                        (some? main-instance-page)
                                        (assoc :main-instance-page main-instance-page)
 
-                                       (some? objects)
+                                       ;; An absent `:objects` key means "leave the stored
+                                       ;; shapes alone", only an explicit `nil` drops them.
+                                       ;; Without this distinction a metadata-only change
+                                       ;; would wipe the shapes of a deleted component.
+                                       (and (contains? change :objects) (some? objects))
                                        (assoc :objects objects)
 
-                                       (nil? objects)
+                                       (and (contains? change :objects) (nil? objects))
                                        (dissoc :objects)
 
                                        (some? modified-at)

common/test/common_tests/files/repair_test.cljc

@@ -12,6 +12,7 @@
    generate accurate errors, and that the repair functions return the file to
    a stable state."
   (:require
+   [app.common.files.changes :as ch]
    [app.common.files.repair :as cfr]
    [app.common.files.validate :as cfv]
    [app.common.test-helpers.components :as thc]
@@ -19,6 +20,8 @@
    [app.common.test-helpers.ids-map :as thi]
    [app.common.test-helpers.shapes :as ths]
    [app.common.test-helpers.variants :as thv]
+   [app.common.types.component :as ctk]
+   [app.common.types.shape :as cts]
    [app.common.uuid :as uuid]
    [clojure.test :as t]))
 
@@ -229,6 +232,55 @@
       (t/is (= "Group / Subgroup" (:path comp1')))
       (t/is (= "Component" (:name comp1'))))))
 
+(t/deftest repair-component-duplicate-slot
+  (t/testing "repair a deleted component whose children share a swap slot, with changes scoped to the component"
+    (let [component-id (uuid/next)
+          main-id      (uuid/next)
+          child1       (uuid/next)
+          child2       (uuid/next)
+          slot         (uuid/next)
+          child        (fn [id]
+                         (ctk/set-swap-slot
+                          (cts/setup-shape
+                           {:id id :type :rect :name "Child"
+                            :frame-id main-id :parent-id main-id
+                            :x 0 :y 0 :width 10 :height 10
+                            :touched #{:geometry-group}})
+                          slot))
+          component    {:id component-id
+                        :name "Component" :path ""
+                        :main-instance-id main-id
+                        :deleted true
+                        :objects {main-id (cts/setup-shape
+                                           {:id main-id :type :frame :name "Main"
+                                            :frame-id uuid/zero :parent-id uuid/zero
+                                            :shapes [child1 child2]})
+                                  child1 (child child1)
+                                  child2 (child child2)}}
+          changes (cfr/repair-error :component-duplicate-slot
+                                    {:code :component-duplicate-slot :shape component}
+                                    {:components {component-id component}}
+                                    {})
+          change (first (:redo-changes changes))
+          data'  (ch/process-changes {:components {component-id component}} (:redo-changes changes))
+          objects' (:objects (get-in data' [:components component-id]))]
+
+      (t/is (= 1 (count (:redo-changes changes))))
+      (t/is (= :mod-obj (:type change))
+            "the repair updates one shape instead of rewriting the whole objects map")
+      (t/is (= component-id (:component-id change)))
+      (t/is (= child2 (:id change))
+            "the second child is the one carrying the duplicated slot")
+      (t/is (= [{:type :set :attr :touched :val #{:geometry-group}
+                 :ignore-geometry false :ignore-touched true}]
+               (:operations change)))
+
+      (t/testing "applying the repair drops the slot and leaves the rest alone"
+        (t/is (= 3 (count objects')))
+        (t/is (nil? (ctk/get-swap-slot (get objects' child2))))
+        (t/is (= #{:geometry-group} (:touched (get objects' child2))))
+        (t/is (= slot (ctk/get-swap-slot (get objects' child1))))))))
+
 (t/deftest repair-child-not-in-parent
   (t/testing "detect and repair shapes whose parent exists but does not list them"
     (let [file    (-> (thf/sample-file :file1 :page-label :page1)

common/test/common_tests/files_changes_test.cljc

@@ -119,6 +119,93 @@
       (t/testing label
         (assert-idempotent data [change])))))
 
+(defn- make-deleted-component
+  "File data with a deleted component, so it keeps the shapes of its main
+   instance inside its own `:objects` map. Returns the file data and the ids
+   needed to address those shapes."
+  []
+  (let [file-id      (uuid/custom 20 1)
+        page-id      (uuid/custom 20 2)
+        main-id      (uuid/custom 20 3)
+        child-id     (uuid/custom 20 4)
+        component-id (uuid/custom 20 5)
+        data         (-> (make-file-data file-id page-id)
+                         (ch/process-changes
+                          [{:type :add-obj
+                            :id main-id :page-id page-id :frame-id uuid/zero
+                            :obj (cts/setup-shape
+                                  {:id main-id :type :frame :name "Main"
+                                   :frame-id uuid/zero :parent-id uuid/zero
+                                   :component-id component-id :component-file file-id
+                                   :main-instance true :component-root true})}
+                           {:type :add-obj
+                            :id child-id :page-id page-id :frame-id main-id :parent-id main-id
+                            :obj (cts/setup-shape
+                                  {:id child-id :type :rect :name "Child"
+                                   :frame-id main-id :parent-id main-id
+                                   :x 0 :y 0 :width 10 :height 10})}
+                           {:type :add-component
+                            :id component-id :name "Component" :path ""
+                            :main-instance-id main-id :main-instance-page page-id}
+                           {:type :del-component :id component-id}]))]
+    {:data data
+     :page-id page-id
+     :main-id main-id
+     :child-id child-id
+     :component-id component-id}))
+
+(t/deftest update-component-does-not-carry-the-objects-map
+  (let [{:keys [data component-id]} (make-deleted-component)
+        changes (pcb/update-component (pcb/with-library-data (pcb/empty-changes nil) data)
+                                      component-id
+                                      #(assoc % :name "Renamed"))
+        change  (first (:redo-changes changes))]
+
+    (t/testing "the change carries component metadata only"
+      (t/is (= :mod-component (:type change)))
+      (t/is (= "Renamed" (:name change)))
+      (t/is (not (contains? change :objects))
+            "resending the whole objects map on every touch is what bloated the changes")
+      (t/is (not (contains? (first (:undo-changes changes)) :objects))))
+
+    (t/testing "applying the change keeps the component shapes"
+      (let [data' (ch/process-changes data (:redo-changes changes))]
+        (t/is (= 2 (count (get-in data' [:components component-id :objects]))))))))
+
+(t/deftest mod-component-touches-objects-only-when-it-carries-them
+  (let [{:keys [data component-id]} (make-deleted-component)
+        objects (:objects (get-in data [:components component-id]))]
+
+    (t/testing "a change without :objects leaves the stored shapes alone"
+      (let [data' (ch/process-changes data [{:type :mod-component
+                                             :id component-id :name "Renamed"}])]
+        (t/is (= objects (get-in data' [:components component-id :objects])))))
+
+    (t/testing "an explicit nil :objects drops them"
+      (let [data' (ch/process-changes data [{:type :mod-component
+                                             :id component-id :objects nil}])]
+        (t/is (not (contains? (get-in data' [:components component-id]) :objects)))))
+
+    (t/testing "a change with :objects replaces them"
+      (let [data' (ch/process-changes data [{:type :mod-component
+                                             :id component-id :objects {}}])]
+        (t/is (= {} (get-in data' [:components component-id :objects])))))))
+
+(t/deftest shape-change-scoped-to-a-component-matches-the-whole-map-change
+  (let [{:keys [data child-id component-id]} (make-deleted-component)
+        objects (:objects (get-in data [:components component-id]))
+        scoped-change {:type :mod-obj
+                       :component-id component-id :id child-id
+                       :operations [{:type :set :attr :name :val "Renamed"}]}
+        ;; what `update-component` used to emit: the whole objects map rewritten
+        whole-map-change {:type :mod-component
+                          :id component-id
+                          :objects (assoc objects child-id
+                                          (assoc (get objects child-id) :name "Renamed"))}]
+
+    (t/is (= (without-modified-at (ch/process-changes data [whole-map-change]))
+             (without-modified-at (ch/process-changes data [scoped-change]))))))
+
 (t/deftest token-changes-are-idempotent
   (let [set-id      (uuid/custom 20 1)
         other-id    (uuid/custom 20 2)