Skip to content

Merge commits record their merged value; legacy merges keep their values - #16

Merged
williamstein merged 4 commits into
mainfrom
feat/self-contained-merges
Oct 2, 2026
Merged

williamstein merged 4 commits into
mainfrom
feat/self-contained-merges

Conversation

@williamstein

Copy link
Copy Markdown
Contributor

A patch with several parents (a merge commit) is a diff from the merged value
of its parents, so computing its value reran merge3: changing merge3 changed
the values of patches already in a history, and their hashes stopped
matching. Now:

  • Session records the merged value of a merge commit's parents, as its author
    computed it: mergePatch, a diff from the exact value of mergeParent (the
    parent with the smallest diff). PatchGraph computes the value of such a
    patch from that parent alone, without merging, like a git merge commit
    records its tree. A later merge3 change only changes how new concurrent
    edits merge. patch keeps its meaning, so clients that do not know the new
    fields, and the apply-all fallback, read it as before.
  • A merge commit without them and without a hash was written by a version
    that applied every patch in time order (patchflow 0.8, what CoCalc runs in
    production) or with that fallback's value. Its value is now computed the
    same way, from the patches its parents descend from (legacyValueOfSet),
    instead of with merge3, so existing histories keep the values their authors
    saw. It starts from the newest snapshot that contains every older patch it
    needs; a snapshot made after a concurrent patch does not.
  • Undo of a merge commit reverts from the same recorded value.

Tests (merge-commit.test.ts): merge commits record their value; a different
merge3 gives the same values and matching hashes (and without the record
does not); readers without the fields agree; random 3-writer histories keep
every value under either merge3; 20 random histories written with apply-all
keep every value under merge3 and a different merge; a snapshot taken after
a concurrent patch is not a starting point; undo of a merge commit.

A patch with several parents (a merge commit) is a diff from the merged value
of its parents, so computing its value reran merge3: changing merge3 changed
the values of patches already in a history, and their hashes stopped
matching. Now:

- Session records the merged value of a merge commit's parents, as its author
  computed it: mergePatch, a diff from the exact value of mergeParent (the
  parent with the smallest diff). PatchGraph computes the value of such a
  patch from that parent alone, without merging, like a git merge commit
  records its tree. A later merge3 change only changes how new concurrent
  edits merge. `patch` keeps its meaning, so clients that do not know the new
  fields, and the apply-all fallback, read it as before.
- A merge commit without them and without a hash was written by a version
  that applied every patch in time order (patchflow 0.8, what CoCalc runs in
  production) or with that fallback's value. Its value is now computed the
  same way, from the patches its parents descend from (legacyValueOfSet),
  instead of with merge3, so existing histories keep the values their authors
  saw. It starts from the newest snapshot that contains every older patch it
  needs; a snapshot made after a concurrent patch does not.
- Undo of a merge commit reverts from the same recorded value.

Tests (merge-commit.test.ts): merge commits record their value; a different
merge3 gives the same values and matching hashes (and without the record
does not); readers without the fields agree; random 3-writer histories keep
every value under either merge3; 20 random histories written with apply-all
keep every value under merge3 and a different merge; a snapshot taken after
a concurrent patch is not a starting point; undo of a merge commit.
@williamstein

Copy link
Copy Markdown
Contributor Author

Replay on real histories (the patch databases of a cocalc.ai project, written by patchflow 0.8 clients, no hashes): every patch value, or for histories over 2,000 patches every merge commit, the patch after it and every 25th patch, compared with the value 0.8 computes (apply-all), which is what its author saw.

text files (113 files, 907 patches, 1 merge) chats + notebooks (14 files, 4,342 checked, 496 merges)
patchflow 0.10.0 0 differ 306 differ (agent chats 186 and 41, lite2.ipynb 10, lite2.chat 69)
this PR 0 differ 32 differ, all in lite2.chat

All 32 are patches whose 0.8 value starts from a snapshot taken after concurrent edits (29 of that file's 115 snapshots), which drops those edits; for each, this PR's value equals applying the full history in time order. So 0.10.0 would change the values of existing production histories at deploy; this PR keeps them, apart from not repeating that snapshot loss.

…eplays continue from checkpoints

Review of #16:
- P1: a merge commit without a hash or recorded merged value was not
  necessarily written by apply-all: patchflow 0.9 wrote them with exact
  merges too. Which one is known only to the application, so it says so:
  PatchGraphOptions/SessionOptions.unmarkedMerges, "merge3" (default, as
  0.9/0.10 read them) or "apply-all" (patchflow 0.8 histories; CoCalc).
  Test: a history written by the v0.9.3 Session (the reviewer's repro) reads
  as its author saw it by default.
- P2: successive legacy merges replayed every patch back to the start (or
  the snapshot) for each merge, quadratic over a snapshot-free history.
  legacyValueOfSet now keeps checkpoints of its replays, keyed by a hash of
  the replayed patch times from the same start, and continues from the
  longest one. Test: 150 rounds of fork/fork/merge read merge by merge,
  every value equal to its author's, apply < 4 per patch (34,125 without the
  checkpoints, < 1,804 with).
- Contract: a patch is still held back until all its parents arrive, even
  when its merged value is recorded (causal delivery, as for every patch).
  Test added.
@williamstein

Copy link
Copy Markdown
Contributor Author

Addressed the review in 1e62282:

  • P1 (unmarked ≠ apply-all): agreed, a missing hash doesn't identify the writer (0.9.x wrote exact-merge commits without one). The application now declares it: unmarkedMerges: "merge3" | "apply-all" on PatchGraphOptions/SessionOptions. The default "merge3" reads unmarked merges as 0.9/0.10 did. CoCalc, whose pre-hash history is patchflow 0.8, passes "apply-all". New test: the reviewer's v0.9.3 history (generated with the actual v0.9.3 Session) reads as helloA\nhelloB\ntail\n by default.
  • P2 (quadratic legacy replay): legacyValueOfSet keeps checkpoints keyed by a hash of the replayed patch times from the same start, and continues from the longest match. New test: 150 fork/fork/merge rounds read merge by merge, every value equal to the author's. Patch applications: < 1,804 with checkpoints, 34,125 with them disabled.
  • Contract question: intentional. A patch is held back until all its parents arrive, even when its merged value is recorded (causal delivery). New test documents it.

Replay of the real 0.8 histories with "apply-all": unchanged, 0 differing values except the 32 non-cut-snapshot cases in lite2.chat. 156 tests pass; lint and build are clean.

…chflow 0.8

patchflow 0.9 was never used in production, so a merge commit with no hash
and no recorded merged value comes from 0.8 (apply-all), as before 1e62282.
Keeps the replay checkpoints and the causal-delivery test.
@williamstein

Copy link
Copy Markdown
Contributor Author

Follow-up (e80ff64): per the maintainer, patchflow 0.9.x was never used in production (only test servers), so a merge commit without a hash or recorded merged value was written by 0.8. Dropped the unmarkedMerges option and its 0.9.3 fixture tests; such merges are read as 0.8 wrote them, as before. Kept the replay checkpoints (P2) and the causal-delivery test. 154 tests pass.

…apart

Review of #16 (P2): the checkpoint cache was bounded only by count (64), so
a large document kept up to 64 full versions (64M characters at 1M). Bound
it by size like the exact caches (exactCacheMaxSize, docSize), and keep the
newest checkpoint apart, so reading merge by merge continues from the last
replay even when one version does not fit the cache. Test: a 200,000
character document with a 150,000 cache keeps no more than that and still
applies under 4 patches per patch (fails with a count-only cache, and
without the newest checkpoint: 1,425 applications).
@williamstein

Copy link
Copy Markdown
Contributor Author

Re-review P2 (checkpoint retained size) fixed in bcd25e0: the checkpoint LRU is now bounded by size like the exact caches (exactCacheMaxSize, docSize), and the newest checkpoint is kept apart so merge-by-merge reads still continue when one version doesn't fit. Regression test: a 200,000-character document with a 150,000 cache keeps ≤ 150,000 characters of checkpoints and applies < 4 patches per patch. It fails with the count-only cache (200,006 kept) and without the newest-checkpoint fallback (1,425 applications). 155 tests pass; lint and build are clean.

@williamstein
williamstein merged commit 4304de8 into main Oct 2, 2026
1 check passed
@williamstein williamstein mentioned this pull request Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant