From fe64834d1d8600cf31c0a89caa8342d74114bf36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Tejero=20Cantero?= Date: Sat, 3 Oct 2026 23:19:44 +0200 Subject: [PATCH] :books: Call the ids a comparison runs in an id space, not a frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch comments called the set of ids a document's references are written in (main's file id and media rows, or the branch's) a frame. Penpot's code uses frame for a board, through `:frame-id`, so a reader could not tell which sense a comment meant. The comments, the docstrings of `get-base-data`, `branch-base-data`, and `remap-changes`, and one test name now say id space. Every board-sense use of frame is unchanged. AI-assisted-by: mixed models Signed-off-by: Álvaro Tejero Cantero --- backend/src/app/rpc/commands/files_branch.clj | 50 +++++++++---------- .../rpc_file_branch_oplog_test.clj | 10 ++-- .../backend_tests/rpc_file_branch_test.clj | 4 +- common/src/app/common/files/branch_merge.cljc | 2 +- .../src/app/main/data/workspace/branches.cljs | 4 +- .../main/ui/workspace/sidebar/branches.cljs | 6 +-- 6 files changed, 38 insertions(+), 38 deletions(-) diff --git a/backend/src/app/rpc/commands/files_branch.clj b/backend/src/app/rpc/commands/files_branch.clj index 21ab610d03..a5b5286fbb 100644 --- a/backend/src/app/rpc/commands/files_branch.clj +++ b/backend/src/app/rpc/commands/files_branch.clj @@ -270,11 +270,11 @@ the op log over it, so this function would decode the same snapshot a second time. - The snapshot stores the branch's frame — its media references name the - branch's own paired rows, what the replay reproduces — while a - comparison runs in main's frame, so the media ids are normalized back - here through the pairs, the way `branch-id-map` normalizes the branch's - own data." + The snapshot stores the branch's id space — its media references name + the branch's own paired rows, what the replay reproduces — while a + comparison runs in main's id space, so the media ids are normalized + back here through the pairs, the way `branch-id-map` normalizes the + branch's own data." [cfg {:keys [source-file-id branch-file-id base-snapshot-id] :as branch}] (when-not base-snapshot-id (ex/raise :type :not-found @@ -297,7 +297,7 @@ nil. Like `get-base-data`, it comes back with the media ids normalized into - main's frame: the snapshot names the branch's own paired rows, the + main's id space: the snapshot names the branch's own paired rows, the comparison runs against main's." [cfg source-file-id {:keys [id] :as branch-file}] (-> (or (::bfc/base-data branch-file) @@ -1283,14 +1283,14 @@ ;; Two id maps, one per direction of the crossing. ;; - ;; The COMPARISON runs in main's frame, the frame the merge - ;; base was captured in, the frame the compare view uses - ;; (`files_branch.clj::branch-id-map` in - ;; `files_branch.clj::get-branch-diff`), and the only frame + ;; The COMPARISON runs in main's id space, the id space the + ;; merge base was captured in, the id space the compare view + ;; uses (`files_branch.clj::branch-id-map` in + ;; `files_branch.clj::get-branch-diff`), and the only id space ;; the branch's inherited content agrees with. A branch stores ;; no data, so `binfile.clj::branch-file-data` replays its op ;; log over that base and everything it inherited carries the - ;; base's ids. Comparing in the branch's frame instead + ;; base's ids. Comparing in the branch's id space instead ;; rewrites main's and the base's local references to the ;; branch file id, and a self reference in main is then ;; unequal to the branch's own copy of it, so every shape @@ -1301,8 +1301,8 @@ branch-cmp (bm/remap-refs (:data branch-file) (branch-id-map cfg branch-file-id main-id)) - ;; The WRITE runs in the branch's frame: a reference is only a - ;; reference in the file it names, so main's file id and + ;; The WRITE runs in the branch's id space: a reference is only + ;; a reference in the file it names, so main's file id and ;; main's media rows are re-pointed at the branch's ids before ;; the change reaches the branch. Media ADDED on main has no ;; branch row yet, so it gets a fresh id here and its row is @@ -1310,7 +1310,7 @@ pairs (media-pairs cfg main-id branch-file-id) new-media (unpaired-media-rows cfg main-id main-raw pairs) fresh-map (into {} (map (fn [row] [(:id row) (uuid/next)])) new-media) - ;; the branch's OWN frame: every media row the branch + ;; the branch's OWN id space: every media row the branch ;; references is one of its own — a paired copy or one of ;; the fresh copies made below. The write and the pinned ;; base name those; `apply-map` adds the file id on top for @@ -1327,13 +1327,13 @@ reposition-base! (fn [ts branch-revn base-data] - ;; the base is pinned in the branch's frame — its media - ;; references name the branch's own rows — because it is - ;; what the op log replays over: the derived document - ;; carries that frame, so a base naming main's rows would - ;; make every paired media read as deleted+added on the - ;; next squash. The comparisons normalize it back to - ;; main's frame on read (`get-base-data`/`branch-base-data`). + ;; the base is pinned in the branch's id space — its media + ;; references name the branch's own rows — because it is what + ;; the op log replays over: the derived document carries that + ;; id space, so a base naming main's rows would make every + ;; paired media read as deleted+added on the next squash. The + ;; comparisons normalize it back to main's id space on read + ;; (`get-base-data`/`branch-base-data`). (let [new-base (fsnap/create! cfg (assoc main-file :data base-data) {:label (str "branch-base/" (:name branch)) :created-by "system" @@ -1415,8 +1415,8 @@ ;; which re-derives the net against the new base. (db/delete! conn :file-branch-change {:branch-id branch-id}) ;; in sync, so no row is copied and the pairs are all - ;; the frame the base needs: unpaired rows stay named - ;; by the id main owns, which is what the branch + ;; the id space the base needs: unpaired rows stay + ;; named by the id main owns, which is what the branch ;; references while it holds no copy (reposition-base! ts (:revn branch-file) (bm/remap-refs main-raw pairs)) (audited {:status :updated :revn (:revn branch-file)} tpoint :update-from-main)) @@ -1439,8 +1439,8 @@ (copy-media-rows! conn branch-file-id fresh-map new-media) (let [applied (bm/remap-changes changes apply-map) - ;; the write lands in the branch's frame, so the - ;; document is moved there first — its paired + ;; the write lands in the branch's id space, so + ;; the document is moved there first — its paired ;; media refs re-pointed at the branch's own ;; rows — or the remapped changes would name ids ;; it does not hold and apply to nothing (a diff --git a/backend/test/backend_tests/rpc_file_branch_oplog_test.clj b/backend/test/backend_tests/rpc_file_branch_oplog_test.clj index f168976e4e..b111ddb732 100644 --- a/backend/test/backend_tests/rpc_file_branch_oplog_test.clj +++ b/backend/test/backend_tests/rpc_file_branch_oplog_test.clj @@ -210,13 +210,13 @@ (ct/is-after? deleted-at (ct/in-future {:days 400})))) rows))))))) -(t/deftest update-from-main-compares-in-main-frame +(t/deftest update-from-main-compares-in-main-id-space ;; A branch stores no data: its document is the base snapshot plus the op ;; log, so every reference it inherited names MAIN. A local reference is a ;; self reference in both files, because a branch keeps main's colours and ;; typographies under the same entity ids, so the comparison must run in - ;; main's frame and the change that lands in the branch must still name the - ;; branch. + ;; main's id space and the change that lands in the branch must still name + ;; the branch. (with-redefs [cf/flags (conj cf/flags :branching)] (let [profile (th/create-profile* 1 {:is-active true}) proj-id (:default-project-id profile) @@ -252,7 +252,7 @@ :parent-id uuid/zero :frame-id uuid/zero}) (assoc :fills (fills main-id "#ff0000")))}) - (let [create (create-branch* profile main-id "main-frame") + (let [create (create-branch* profile main-id "main-id-space") branch-id (:id create) branch-file-id (:branch-file-id create)] @@ -273,7 +273,7 @@ (t/is (= "#00ff00" (get-in out [:result :data :pages-index page-id :objects shape-id :fills 0 :fill-color])))) ;; main's own change arrives through the repositioned base, so the - ;; net the update writes is empty. A ref left in main's frame would + ;; net the update writes is empty. A ref left in main's id space would ;; make this shape differ from the new base and put it in the log. (t/is (empty? (oplog-rows branch-file-id)))) diff --git a/backend/test/backend_tests/rpc_file_branch_test.clj b/backend/test/backend_tests/rpc_file_branch_test.clj index eb33542258..a7de155282 100644 --- a/backend/test/backend_tests/rpc_file_branch_test.clj +++ b/backend/test/backend_tests/rpc_file_branch_test.clj @@ -1141,7 +1141,7 @@ (t/is (some? img-id)) ;; the colour's image keeps resolving to the right content (the ;; document names the branch's own paired copy: the base is pinned - ;; in the branch's frame and the op log replays over it) + ;; in the branch's id space and the op log replays over it) (let [row (th/db-get :file-media-object {:id img-id})] (t/is (= (:id sobj) (:media-id row))))) @@ -1154,7 +1154,7 @@ (t/testing "the update logs no phantom branch change" ;; the colour image id must be re-pointed at the copy like every - ;; other ref (`bm/remap-changes`); left in main's frame it makes + ;; other ref (`bm/remap-changes`); left in main's id space it makes ;; the colour differ from the new base and lands in the op log (t/is (empty? (th/db-query :file-branch-change {:branch-id branch-id}))))))) diff --git a/common/src/app/common/files/branch_merge.cljc b/common/src/app/common/files/branch_merge.cljc index 6d80f8b58e..cadfc8ec3b 100644 --- a/common/src/app/common/files/branch_merge.cljc +++ b/common/src/app/common/files/branch_merge.cljc @@ -1099,7 +1099,7 @@ (defn remap-changes "Rewrite the cross-file references of a change vector through `id-map`, the counterpart of `remap-refs` for the changes `compute-changes` emits. - A change computed in the source file's frame has to be applied in the + A change computed in the source file's id space has to be applied in the target's, because a reference is only a reference in the file it names: a fill whose `:fill-color-ref-file` names another file loses its link, and so does an instance whose `:component-file` does. diff --git a/frontend/src/app/main/data/workspace/branches.cljs b/frontend/src/app/main/data/workspace/branches.cljs index 47db7ea090..66488d0f46 100644 --- a/frontend/src/app/main/data/workspace/branches.cljs +++ b/frontend/src/app/main/data/workspace/branches.cljs @@ -439,7 +439,7 @@ Those conflicts are authoritative for the whole modal: they are the very ones the command's own `resolved?` matches against, while the diff the modal - can fetch on its own is computed in another id frame and may list a + can fetch on its own is computed in another id space and may list a different set, or none at all. Keeping them in the store also lets the bulk actions resolve the same set the modal shows." [{:keys [branch mode conflicts]}] @@ -491,7 +491,7 @@ ;; the returned conflicts are authoritative: they are the very ;; ones the command computes `resolved?` against, and the diff ;; the modal could fetch on its own is computed in another id - ;; frame, which can disagree with them (vcs:tp-update-conflict-frame) + ;; space, which can disagree with them (vcs:tp-update-conflict-frame) :conflicts (rx/of (open-conflict-resolutions {:branch branch :mode :update :conflicts conflicts})) diff --git a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs index 48ba65b26b..7bc7c5e76e 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/branches.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/branches.cljs @@ -1600,9 +1600,9 @@ (if ago (str ago " · " you) you)) ;; the command that opened this modal returned its own conflict set, - ;; computed in the frame it resolves against; the diff this dialog can - ;; fetch may have been computed in another frame and disagree, which - ;; left the dialog showing 0 of 0 with an Apply that could never + ;; computed in the id space it resolves against; the diff this dialog + ;; can fetch may have been computed in another id space and disagree, + ;; which left the dialog showing 0 of 0 with an Apply that could never ;; enable (vcs:tp-update-conflict-frame). Prefer what the command said. conflicts (if (seq conflicts) conflicts (:conflicts diff))