📚 Bring comments and docstrings in line with the op-log engine

Comments written for the duplicate-file prototype still described a
branch as a copy of its file, a merge as clean-only, token diffing as
missing, a review as a read-only snapshot, and file_change rows as
living for the team deletion delay. Each now states what the code at
this revision does: a branch is a file row derived from a pinned base
plus its op log, every change kind except :page-attrs is translated,
merges apply resolutions, a review opens the live editable branch, an
update squashes the log and repositions the base, and file_change rows
expire after one hour. The audited docstring names the props it sets,
the step 6 comment says each revn counter starts at its own side, and
handle-file-merged names both topics it arrives on.

Comment, docstring, and SQL-comment lines only. clj-kondo reports no
errors on the touched Clojure files. Translations are untouched.

AI-assisted-by: claude-opus-5-5

backend/src/app/migrations/sql/0157-add-file-branch-change-table.sql

@@ -1,14 +1,19 @@
 -- The durable op log of a branch: one row per branch save (the change
--- vector the editor produced, blob-encoded) plus the ops a
--- update-branch-from-main integration produced. The branch file's
--- :data is DERIVED by replaying these rows over the merge-base snapshot
--- (`file_branch.base_snapshot_id`), so the branch file itself stores no
--- data.
+-- vector the editor produced, blob-encoded). An update-branch-from-main
+-- replaces the whole log with one row holding the branch's squashed net
+-- changes against the repositioned merge base (no row when that net is
+-- empty); the integration vector itself goes to `file_change`. The
+-- branch file's :data is DERIVED by replaying these rows over the
+-- merge-base snapshot (`file_branch.base_snapshot_id`), so the branch
+-- file itself stores no data.
 --
 -- `file_change` rows remain the transient source for the lagged-changes
 -- machinery; this table is what keeps the branch reconstructible after
--- those rows are GC'd (team deletion delay). Rows live until the branch
--- is deleted or the op log is squashed (update-branch-from-main).
+-- those rows are GC'd (each is GC-eligible one hour after its insert).
+-- Rows live until the op log is squashed (update-branch-from-main) or
+-- dropped (materialize-file-branch), or until the objects GC hard-deletes
+-- the branch file row and the foreign keys cascade; a logical branch
+-- deletion leaves them in place.
 
 CREATE TABLE IF NOT EXISTS file_branch_change (
   id          uuid PRIMARY KEY,

backend/src/app/rpc/commands/files_branch.clj

@@ -5,10 +5,12 @@
 ;; Copyright (c) KALEIDOS INC Sucursal en España SL
 
 (ns app.rpc.commands.files-branch
-  "RPC commands for file branching: create an isolated copy of a file
-  (a \"branch\") linked to its source file (\"main\"), list branches,
-  diff a branch against main, merge a branch into main (with conflict
-  resolutions), and update a branch from main. See
+  "RPC commands for file branching: create a branch of a file linked to
+  its source file (\"main\"), list branches, diff a branch against main,
+  merge a branch into main (with conflict resolutions), and update a
+  branch from main. A branch is a file row with no data of its own: its
+  state is derived from a pinned base snapshot of main plus the branch's
+  op log (`app.binfile.common/branch-file-data`). See
   `app.rpc.commands.files-snapshot` for the patterns this namespace
   mirrors."
   (:require
@@ -152,10 +154,11 @@
 ;; operation attaches it to its own event.
 
 (defn- audited
-  "Attach the `:duration-ms` and `:outcome` props to the audit event of a
-  branch operation, and print the matching log line. `result` is returned
-  unchanged to the caller, and `extra` lets an operation add numbers of
-  its own to the same record.
+  "Attach the `:branch-operation`, `:branch-outcome` and
+  `:branch-duration-ms` props to the audit event of a branch operation,
+  and print the matching log line. `result` is returned unchanged to the
+  caller, and `extra` lets an operation add numbers of its own to the
+  same record.
 
   A listing returns a vector rather than a map, and a vector carries
   metadata exactly like the maps the other commands return, so the props
@@ -178,9 +181,12 @@
 
 ;; --- Helpers: media pairing
 ;;
-;; `duplicate-file` (branch creation) gives the branch's file_media_object
-;; rows FRESH ids and relinks the branch `:data` to them, so the same image
-;; carries a different id on each side. Before diffing or merging, the
+;; A branch copies no media at creation: the content it inherits from its
+;; base references main's file_media_object rows. Rows cross between the
+;; two files only as copies under FRESH ids (`copy-media-rows!`): an update
+;; from main copies the media main added into the branch, and a merge
+;; copies the media the branch added into main. So the same image can
+;; carry a different id on each side. Before diffing or merging, the
 ;; branch-side media ids must be normalized back to the target's ids
 ;; (together with the file id, via `bm/remap-refs`); otherwise every image
 ;; looks changed, and merged shapes would reference media rows owned by the
@@ -195,9 +201,10 @@
 (defn- media-pairs
   "{from-media-id -> to-media-id} for the file_media_object rows present on
   both files with the same content identity (storage object + name +
-  dimensions + mtype) — i.e. the rows `duplicate-file` copied at branch
-  creation. Rows with several identical copies are paired positionally
-  (they are interchangeable). Media added on one side stays unpaired."
+  dimensions + mtype), such as the rows one file received as a copy of
+  the other's (`copy-media-rows!`, run by an update from main or a merge).
+  Rows with several identical copies are paired positionally (they are
+  interchangeable). Media added on one side stays unpaired."
   [cfg from-file-id to-file-id]
   (let [ident   (juxt :media-id :name :width :height :mtype)
         from-gs (group-by ident (db/exec! cfg [sql:file-media-identity from-file-id]))
@@ -470,9 +477,10 @@
 
                     ;; 6. Persist the branch metadata. `base-revn`
                     ;; tracks MAIN's revision counter and
-                    ;; `base-branch-revn` the BRANCH file's one (they
-                    ;; coincide at creation but drift apart: they are
-                    ;; independent counters).
+                    ;; `base-branch-revn` the BRANCH file's one. Each
+                    ;; starts at its own side's revn (main's current
+                    ;; revn, the new branch file's 0) and is compared
+                    ;; against that side alone (see `revn-deltas`).
                     (db/insert! conn :file-branch
                                 {:id meta-id
                                  :branch-file-id branch-id
@@ -759,7 +767,9 @@
 
 (sv/defmethod ::get-branch-diff
   "Read-only three-way diff between a branch and its source (main),
-  using the merge base captured at branch creation. Returns the summary
+  using the branch's current merge base: the snapshot taken at branch
+  creation, or the one the latest update from main repositioned it to
+  (`reposition-base!` in `update-branch-from-main`). Returns the summary
   produced by `branch-merge/compute-merge` (stats, changes, conflicts).
 
   NOTE (Phase 2): base/main/branch are assumed to share the same file
@@ -830,13 +840,14 @@
 (sv/defmethod ::merge-file-branch
   "Merge a branch into its source file (main).
 
-  Phase 3 scope: clean merges only. If the three-way diff has conflicts
-  the command returns `{:status :conflicts}` (resolution UI lands in a
-  later phase); if it contains change kinds not yet translatable
-  (components, pages, tokens) it returns `{:status :unsupported}` so no
-  change is silently dropped. Otherwise it applies the merge to main
-  through the production change pipeline, takes a safety snapshot, marks
-  the branch merged and notifies open clients via msgbus."
+  Conflicts that `resolutions` leaves unresolved make the command return
+  `{:status :conflicts}`; a resolved conflict merges the side its
+  resolution picks, per entity or per attr. Change kinds the engine
+  cannot translate (`bm/unsupported-kinds`) make it return
+  `{:status :unsupported}` so no change is silently dropped. Otherwise it
+  takes a safety snapshot of main, applies the merge to main through the
+  production change pipeline, marks the branch merged and notifies open
+  clients via msgbus."
   {::doc/added "2.16"
    ::webhooks/event? true
    ::sm/params schema:merge-file-branch
@@ -1411,9 +1422,13 @@
       AND fb.deleted_at IS NULL")
 
 (sv/defmethod ::get-file-branch-info
-  "If the given file is a branch, return its branch metadata (with cheap
-  revn-based ahead/behind); otherwise nil. Returns nil when branching is
-  disabled, so it is safe to call on every file open."
+  "If the given file is a branch, return its branch metadata with
+  entity-level `ahead`/`behind`/`conflicts` counts read through the
+  summary cache (`cached-diff-counts`); otherwise nil. A cache miss on an
+  open branch whose revn deltas are not both zero runs the three-way diff
+  (`branch-diff-counts!`), which loads main, the branch and its base.
+  Returns nil when branching is disabled, so it is safe to call on every
+  file open."
   {::doc/added "2.16"
    ::sm/params schema:get-file-branch-info}
   [cfg {:keys [::rpc/profile-id file-id]}]

backend/src/app/rpc/commands/files_pull_request.clj

@@ -7,11 +7,15 @@
 (ns app.rpc.commands.files-pull-request
   "RPC commands for pull requests on top of file branching: a review
   request over a pinned snapshot of a branch. A pull request does NOT
-  create a new file; the review sandbox is a virtual read-only view of
-  the branch served from the pinned snapshot (`get-pull-request-bundle`),
-  so it disappears when the pull request is closed and the snapshot pin
-  is released. Merging stays on `app.rpc.commands.files-branch`; approving
-  a pull request is informative and never a gate for the merge."
+  create a new file. Reviewers open the live branch file, which stays
+  editable, so their edits change the branch and mark the pull request
+  `outdated` (the branch revn moved past the snapshot's). The pinned
+  snapshot serves only the read-only views: the interactions viewer
+  (`viewer/get-view-only-bundle` with `:pr-id`) and
+  `get-pull-request-bundle`. Both refuse once the pull request is no
+  longer open, and closing it releases the snapshot pin. Merging stays on
+  `app.rpc.commands.files-branch`; approving a pull request is
+  informative and never a gate for the merge."
   (:require
    [app.binfile.common :as bfc]
    [app.common.data :as d]

common/src/app/common/files/branch_merge.cljc

@@ -7,12 +7,14 @@
 (ns app.common.files.branch-merge
   "Three-way, entity-level merge/diff engine for file branching.
 
-  Phase 2 scope: read-only diff. `compute-merge` takes the file `:data`
-  of the merge base, main and branch and returns a serializable summary
-  of what the branch changes relative to main, plus the set of
-  conflicting entities. It never mutates the blob; translating the
-  result into `changes` (for an actual merge) and token diffing arrive
-  in later phases.
+  `compute-merge` takes the file `:data` of the merge base, main and
+  branch and returns a serializable summary of what the branch changes
+  relative to main, plus the set of conflicting entities. It never
+  mutates the blob. `compute-changes` translates the same comparison
+  into `changes` for an actual merge or update, resolved conflicts
+  included. It covers every kind `compute-merge` reports, token kinds
+  included, except the residual page attrs (`:page-attrs`), which
+  `unsupported-kinds` reports so the caller refuses them.
 
   Direction `:branch->main` (merge/compare) treats main as `theirs` and
   branch as `ours`; `:main->branch` (update from main) swaps them."
@@ -127,16 +129,18 @@
   "Drop keys whose value is `nil`, deeply, so that a key present with a
   `nil` value compares equal to an absent key.
 
-  A branch copy re-points its local references through the duplication
-  pipeline, which leaves keys like `:fill-color-ref-file` and
-  `:typography-ref-file` present with `nil` where the source omitted them;
-  the same asymmetry appears inside a text `:content`. Comparing the two
-  forms as different makes an untouched entity look edited on one side, and
-  when the other side really moved it the engine reports a false
-  `:modify-modify` conflict — 368 of them on the design-system file's
-  screenshots of the shape trees, before this existed. Absent and nil mean
-  the same thing for a shape attribute, so the comparison should not see
-  the difference."
+  The three sides reach the comparison by different routes: the base is a
+  stored snapshot, main is its own saved data, and the branch is that base
+  with its op log replayed over it. They do not always spell an unset
+  attribute the same way: one side can carry a key like
+  `:fill-color-ref-file` or `:typography-ref-file` with `nil` where another
+  omits it, and the same asymmetry appears inside a text `:content`.
+  Comparing the two forms as different makes an untouched entity look
+  edited on one side, and when the other side really moved it the engine
+  reports a false `:modify-modify` conflict — 368 of them on the
+  design-system file's screenshots of the shape trees, before this
+  existed. Absent and nil mean the same thing for a shape attribute, so
+  the comparison should not see the difference."
   [v]
   (cond
     (map? v)
@@ -428,12 +432,12 @@
 
 ;; --- Tokens ---
 ;;
-;; Token *values* (tokens within an existing set) are mergeable (kind
-;; :token -> :set-token). Structural token changes (adding/renaming sets,
-;; themes, active-theme/active-set toggles) are surfaced with
-;; non-mergeable kinds (:token-set, :token-theme, :token-active-themes)
-;; so the merge refuses them rather than dropping them silently — a full
-;; structural token merge is a later step.
+;; Every token change is mergeable. Token *values* (tokens within an
+;; existing set) carry kind :token -> :set-token. Structural changes
+;; carry their own kinds, each translated by `compute-changes`: set
+;; add/delete :token-set, rename/description :token-set-rename, set order
+;; :token-set-order, themes :token-theme, active themes
+;; :token-active-themes and active-set toggles :token-active-sets.
 
 (defn- lib-set-ids
   [lib]
@@ -560,7 +564,7 @@
                                          {:kind :token-set})
                      (presence-only)
                      (update :conflicts #(slim-conflict-sides slim-set %)))
-        ;; set rename/description on common sets (NOT yet mergeable)
+        ;; set rename/description on common sets (mergeable: :token-set-rename)
         rename   (three-way-entities (select-keys (lib-set-meta bl) common)
                                      (select-keys (lib-set-meta tl) common)
                                      (select-keys (lib-set-meta ol) common)
@@ -578,7 +582,7 @@
                                          {:active-themes (lib-active-paths tl)}
                                          {:active-themes (lib-active-paths ol)}
                                          {:kind :token-active-themes})
-        ;; active-set toggles (hidden theme) — NOT yet mergeable
+        ;; active-set toggles (hidden theme) (mergeable: :token-active-sets)
         active-sets (three-way-entities {:active-sets (lib-hidden-sets bl)}
                                         {:active-sets (lib-hidden-sets tl)}
                                         {:active-sets (lib-hidden-sets ol)}
@@ -602,21 +606,24 @@
 
 (defn remap-refs
   "Rewrite cross-file references in `data` through `id-map`
-  ({old-id -> new-id}): the file refs relinked by `duplicate-file`
+  ({old-id -> new-id}): the file refs `cfh/relink-refs` rewrites
   (`:component-file`, `:fill-color-ref-file`, `:stroke-color-ref-file`,
   `:typography-ref-file`, shadow/grid `:file-id`) plus the media refs
   (shape `:metadata`/`:fill-image`/`:stroke-image` ids, the `:media`
-  collection keys and library-color `:image` ids) — the exact same
-  surface `binfile.common/process-file` remaps when the branch copy is
-  created.
+  collection keys and library-color `:image` ids). It is the same surface
+  `binfile.common/process-file` remaps when it copies a file.
 
-  A branch is a file copy whose LOCAL references were re-pointed to its
-  own ids by the duplication pipeline. Before diffing or merging against
-  another file those references must be normalized to the target's ids;
-  otherwise (a) every affected entity looks modified, inflating the
-  diff/conflicts, and (b) merged references cannot be resolved in the
-  target file — repair would detach components and images would break.
-  Ids not present in `id-map` (external libraries) are left untouched."
+  A branch stores no data of its own: its state is its source's base
+  snapshot with the branch's op log replayed over it
+  (`binfile.common/branch-file-data`). What it inherited from the base
+  keeps the source's ids, but the references the branch writes itself
+  name the branch file and the media rows it owns. Before diffing or
+  merging against another file those references must be normalized to
+  the target's ids; otherwise (a) every affected entity looks modified,
+  inflating the diff/conflicts, and (b) merged references cannot be
+  resolved in the target file — repair would detach components and
+  images would break. Ids not present in `id-map` (external libraries)
+  are left untouched."
   [data id-map]
   (if (empty? id-map)
     data
@@ -760,19 +767,19 @@
   NOT sound for the direction that reports main's changes, and passing it
   there would hide them.
 
-  NOTE: token-lib diffing is not yet implemented (handled in a later
-  phase); `:tokens-lib` changes are not reported here."
+  The `:tokens-lib` is diffed as well (`diff-tokens`), so token changes
+  are reported here with their own kinds."
   ([base main branch dir] (compute-merge* base main branch dir nil))
   ([base main branch dir {:keys [only-pages]}]
    (compute-merge* base main branch dir only-pages)))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; MERGE -> CHANGES (Phase 3, no-conflict path)
+;; MERGE -> CHANGES (clean changes plus resolved conflicts)
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-;; Kinds `compute-changes` can translate into change ops. Components,
-;; pages (add/remove/rename) and tokens are not yet supported and cause
-;; the merge to refuse rather than silently drop changes.
+;; Kinds `compute-changes` can translate into change ops: every kind
+;; `compute-merge` reports except the residual page attrs (`:page-attrs`),
+;; which make the merge refuse rather than silently drop changes.
 (def ^:private mergeable-kinds
   #{:color :typography :media :shape :token :token-set :token-set-rename :token-set-order
     :token-theme :token-active-themes :token-active-sets
@@ -1041,8 +1048,8 @@
   no conflict remains unresolved before applying).
 
   Returns `{:changes [..] :unsupported #{kinds..}}`. When `:unsupported`
-  is non-empty the caller must refuse the merge (translation for those
-  kinds — components, pages, tokens — is not implemented yet).
+  is non-empty the caller must refuse the merge (the only kind it cannot
+  translate is `:page-attrs`, the residual page attrs no pass handles).
 
   The 5-arity accepts the `compute-merge` summary the caller usually
   already computed (for the conflict gate), so the full three-way diff is

frontend/src/app/main/data/workspace/notifications.cljs

@@ -329,8 +329,11 @@
           (rx/of (ptk/event ::dw/reload-current-file)))))))
 
 (defn handle-file-merged
-  "A branch was merged into this file (main). Reload the current file so
-  the merged changes appear for everyone with main open."
+  "Another session integrated changes into this file: a merge of a branch
+  into main (sent on main's topic) or an update of a branch from main
+  (sent on the branch file's topic). Reload the current file so the
+  integrated changes appear for everyone with that file open. The
+  websocket drops the message for the session that made the change."
   [{:keys [file-id] :as _msg}]
   (ptk/reify ::handle-file-merged
     ptk/WatchEvent

frontend/src/app/main/data/workspace/pull_requests.cljs

@@ -6,10 +6,12 @@
 
 (ns app.main.data.workspace.pull-requests
   "Data layer for pull requests over file branches. A pull request points
-  to a pinned review snapshot of its branch; the review sandbox is that
-  snapshot loaded read-only into the workspace (same mechanics as the
-  version-history preview in `app.main.data.workspace.versions`), so it
-  is never an editable copy of anything. Mirrors the patterns of
+  to a pinned review snapshot of its branch. Reviewing opens the live
+  branch file as a normal, editable workspace with a review context
+  attached (`:workspace-pr-preview`, which drives the review banner and
+  header), so a reviewer's edits change the branch and mark the pull
+  request outdated. The pinned snapshot backs the interactions viewer and
+  the outdated flag, not the canvas. Mirrors the patterns of
   `app.main.data.workspace.branches`."
   (:require
    [app.common.data :as d]
@@ -253,13 +255,14 @@
                 (rx/of (ntf/error (tr "workspace.pull-requests.lifecycle.error"))))))))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; REVIEW SANDBOX (read-only preview of the pinned snapshot)
+;; REVIEW CONTEXT (the live, editable branch file plus the review banner)
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (defn open-pull-request
-  "Navigate into the review sandbox of `pr`: the branch file's workspace
-  with the `pr-id` query param, which loads the pinned snapshot
-  read-only once the file is initialized."
+  "Navigate into the review of `pr`: the branch file's workspace with the
+  `pr-id` query param, which attaches the review context once the file is
+  initialized (`initialize-pull-request-preview`). The canvas is the live
+  branch, editable as always."
   [pr]
   (assert (uuid? (:id pr)) "expected a pull request row with a valid `:id`")
   (ptk/reify ::open-pull-request

frontend/src/app/main/ui/workspace.cljs

@@ -94,9 +94,9 @@
         pr-preview (mf/deref refs/pull-request-preview)]
     [:*
      (when (not ^boolean hide-ui?)
-       ;; inside a review sandbox the pull request banner replaces the
-       ;; branch banner: the user is reviewing a pinned snapshot, not
-       ;; editing the branch
+       ;; during a pull request review the review banner replaces the
+       ;; branch banner. The canvas is still the live, editable branch:
+       ;; edits change it and mark the pull request outdated
        (if (some? pr-preview)
          [:> pr-review-banner* {}]
          [:> branch-context-banner* {:file-id (get file :id)}]))

frontend/src/app/main/ui/workspace/left_header.cljs

@@ -41,9 +41,9 @@
         branch-ctx  (mf/deref refs/branch-context)
         branch?     (and (some? branch-ctx) (some? (:source-name branch-ctx)))
 
-        ;; When a pull request review sandbox is active the header gets a
-        ;; distinct badge and title so the snapshot is never mistaken for
-        ;; the editable branch.
+        ;; When a pull request review is active the header gets a
+        ;; distinct badge and the pull request title, so the user knows a
+        ;; review is open. The file is still the live, editable branch.
         pr-preview  (mf/deref refs/pull-request-preview)
         review?     (some? pr-preview)
         pr-title    (get-in pr-preview [:info :title] "")

frontend/src/app/main/ui/workspace/left_header.scss

@@ -94,8 +94,8 @@
   color: var(--button-secondary-foreground-color-rest);
 }
 
-// pull request review sandbox: accent-colored so the read-only snapshot
-// is never mistaken for the editable branch
+// pull request review: accent-colored so an active review stands out from
+// the plain branch badge (the file is still the live, editable branch)
 .review-badge {
   @include deprecated.flex-center;
 

frontend/src/app/main/ui/workspace/sidebar/pull_requests.cljs

@@ -484,9 +484,9 @@
         team     (mf/deref refs/team)
         author?  (= (:created-by pr) (:id profile))
         reviewer? (some #(= (:profile-id %) (:id profile)) (:reviews pr))
-        ;; merging keeps requiring edition permissions on main: the
-        ;; sandbox file permissions are stripped, but the team role tells
-        ;; whether this user could integrate the branch
+        ;; merging requires edition permissions on main, not on the open
+        ;; branch file, so the team role tells whether this user could
+        ;; integrate the branch
         can-merge? (boolean (get-in team [:permissions :can-edit]))
         [approvals total] (current-approvals pr)
 

frontend/src/app/main/ui/workspace/sidebar/pull_requests.scss

@@ -588,8 +588,9 @@
   max-inline-size: 90vw;
   border-radius: deprecated.$s-8;
 
-  // the review sandbox must never be confused with the editable branch:
-  // accent-colored border instead of the branch banner's neutral one
+  // the review banner must stand out from the branch banner it replaces
+  // (the canvas is still the live branch): accent-colored border instead
+  // of the branch banner's neutral one
   border: deprecated.$s-2 solid var(--color-accent-primary);
   background-color: var(--color-background-primary);