docs: auto-update for PR #2969 — fix(lww-register): add value byte tie-break to fix merge-mode divergence#2977
docs: auto-update for PR #2969 — fix(lww-register): add value byte tie-break to fix merge-mode divergence#2977meroreviewer[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🤖 AI Code Reviewer
Reviewed by 1 agents | Quality score: 31% | Review time: 46.7s
💡 1 suggestions. See inline comments.
🤖 Generated by AI Code Reviewer | Review ID: review-956039fb
| /// Borsh-serialized bytes compare greater wins. | ||
| /// | ||
| /// This ordering is commutative, associative, and idempotent, satisfying | ||
| /// the CRDT convergence guarantee even for merge-mode registers that carry |
There was a problem hiding this comment.
💡 Doc states merge is 'commutative, associative, and idempotent' but only commutativity is tested
The doc-comment contract in the new section claims the three-level ordering is 'commutative, associative, and idempotent' and the regression test only asserts commutativity (merge(A,B) == merge(B,A)). The documentation would be more precise if it noted that associativity and idempotency are properties of the total order itself (not separately tested), or if it pointed to where those properties are verified. This is a minor documentation accuracy issue.
Suggested fix:
Either add a note that associativity and idempotency follow from the strict total order property (no separate test needed), or add a brief explanation of why those properties hold given the lexicographic byte comparison.
Automatic Documentation Update
Opened automatically after PR #2969 merged.
Each block shows the documentation change as a diff (added lines in green, removed in red); expand "Why this changed" for the source rationale.
Documentation changes
architecture/storage-schema.html— Add value-byte tie-break to make LwwRegister merge commutative under zero stamps; Tighten trait bound on LwwRegister merge operations to require BorshSerialize; Add regression test for commutativity of zero-stamp mergesWhy this changed (source: PR #2969)
The merge ordering logic previously had only two comparison levels: timestamp, then node_id. In merge-mode, both fields are zeroed, so two registers with different values had identical stamps, making merge(A,B) keep A while merge(B,A) kept B — a permanently diverging, non-commutative outcome. A new private helper
other_winsnow adds a third level: when both timestamp and node_id are equal, it borsh-serializes both values and compares the resulting byte slices lexicographically, producing a deterministic total order regardless of call order. Serialization failure panics withexpectrather than silently substituting an empty vec (which would corrupt the ordering).The
impl<T: Clone> LwwRegister<T>block providingmergeandwould_update, and theMergeableblanket impl, now requireT: Clone + borsh::BorshSerialize. This is a narrowing of the public API surface for those methods.A new test
merge_mode_equal_stamps_different_values_convergecreates two merge-mode registers with value 1 and 2 respectively (both having zero timestamps and zero node_ids), then asserts that merge(A,B).get() == merge(B,A).get(), directly catching any future regression to non-commutative behavior.architecture/migrations.html— Add value-byte tie-break to make LwwRegister merge commutative under zero stamps; Tighten trait bound on LwwRegister merge operations to require BorshSerialize; Add regression test for commutativity of zero-stamp mergesWhy this changed (source: PR #2969)
The merge ordering logic previously had only two comparison levels: timestamp, then node_id. In merge-mode, both fields are zeroed, so two registers with different values had identical stamps, making merge(A,B) keep A while merge(B,A) kept B — a permanently diverging, non-commutative outcome. A new private helper
other_winsnow adds a third level: when both timestamp and node_id are equal, it borsh-serializes both values and compares the resulting byte slices lexicographically, producing a deterministic total order regardless of call order. Serialization failure panics withexpectrather than silently substituting an empty vec (which would corrupt the ordering).The
impl<T: Clone> LwwRegister<T>block providingmergeandwould_update, and theMergeableblanket impl, now requireT: Clone + borsh::BorshSerialize. This is a narrowing of the public API surface for those methods.A new test
merge_mode_equal_stamps_different_values_convergecreates two merge-mode registers with value 1 and 2 respectively (both having zero timestamps and zero node_ids), then asserts that merge(A,B).get() == merge(B,A).get(), directly catching any future regression to non-commutative behavior.Generated by
ai-reviewer update-docs. Nothing was auto-merged.