From 79170faa601ede70d68095075655313683c811b8 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 27 May 2026 12:47:43 +0200 Subject: [PATCH 1/4] Skip spurious CFC move when between has no anchors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orderable diff in EntrySync hands BetweenPosition(null, null) to Add when no stable neighbours exist (e.g. a singleton component). During sync the same logical CFC is reached twice — once via SyncComplexForms on the component-side entry (no between) and once via SyncComplexFormComponents on the complex-form entry (between with both neighbours null). The second call found the just-created CFC and, because between was non-null, fell through to MoveComplexFormComponent → PickOrder → max+1 every time, emitting a spurious SetOrderChange that bumped Order from 1 → 2 (and further on repeated syncs). CreateComplexFormComponent now treats `(null, null)` as no positional intent — same boundary normalisation OrderPicker.PickOrder and FwDataMiniLcmApi.InsertComplexFormComponent already do. New test in ComplexFormComponentTestsBase exercises both API implementations. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 7 ++++++- .../ComplexFormComponentTestsBase.cs | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 184fa0fc52..9660435e16 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -313,7 +313,12 @@ public async Task CreateComplexFormComponent(ComplexFormCo return await repo.FindComplexFormComponent(addEntryComponentChange.EntityId); } - if (between is not null) + // BetweenPosition(null, null) carries no positional intent — it's what the + // orderable diff hands in for a lone or unanchored item. Treat it as a no-op + // here; otherwise MoveComplexFormComponent walks PickOrder and bumps Order + // to max+1 every time the same CFC is revisited (e.g. via SyncComplexForms + // and SyncComplexFormComponents in one sync). Matches FwDataMiniLcmApi. + if (between is { Previous: not null } or { Next: not null }) { await MoveComplexFormComponent(existing, between); } diff --git a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs index dd123d0f34..e767376ed4 100644 --- a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs @@ -95,6 +95,26 @@ public async Task CreateComplexFormComponent_ReplayingReturnedObject_IsIdempoten again.Should().BeEquivalentTo(created); } + [Fact] + public async Task CreateComplexFormComponent_BetweenWithNoAnchors_DoesNotReorderExisting() + { + // The orderable diff in EntrySync hands BetweenPosition(null, null) to Add when + // no stable neighbours exist (e.g. a singleton component). The same CFC can be + // reached twice in one sync — once via SyncComplexForms on the component-side + // entry (no between) and once via SyncComplexFormComponents on the complex-form + // entry (between with both neighbours null). The second call must be a no-op, + // not a spurious reorder that bumps Order from 1 → 2. + var first = await Api.CreateComplexFormComponent( + ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); + var second = await Api.CreateComplexFormComponent( + ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry), + new BetweenPosition(null, null)); + + second.Order.Should().Be(first.Order); + var entry = await Api.GetEntry(_complexFormEntryId); + entry!.Components.Should().ContainSingle().Which.Order.Should().Be(first.Order); + } + [Fact] public async Task CreateComplexFormComponent_UsingTheSameComponentWithSenseDoesNothing() { From 78233aa39a5f7e40a61f55ee536ac04092e27bd0 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 27 May 2026 12:56:43 +0200 Subject: [PATCH 2/4] Extract BetweenPosition.HasAnchor; replace inline pattern matches Names the predicate shared by OrderPicker.PickOrder (the create branch's "no positional intent" shortcut) and the new CreateComplexFormComponent guard. The CRDT site's previous inverted pattern read awkwardly; both callsites now use the same null-tolerant helper. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 10 ++++------ backend/FwLite/LcmCrdt/OrderPicker.cs | 4 ++-- .../MiniLcm.Tests/ComplexFormComponentTestsBase.cs | 10 ++++------ backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs | 9 +++++++++ 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 9660435e16..376ac8d456 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -313,12 +313,10 @@ public async Task CreateComplexFormComponent(ComplexFormCo return await repo.FindComplexFormComponent(addEntryComponentChange.EntityId); } - // BetweenPosition(null, null) carries no positional intent — it's what the - // orderable diff hands in for a lone or unanchored item. Treat it as a no-op - // here; otherwise MoveComplexFormComponent walks PickOrder and bumps Order - // to max+1 every time the same CFC is revisited (e.g. via SyncComplexForms - // and SyncComplexFormComponents in one sync). Matches FwDataMiniLcmApi. - if (between is { Previous: not null } or { Next: not null }) + // Without this guard, the same CFC revisited in one sync (via SyncComplexForms + // then SyncComplexFormComponents with a no-anchor between) gets bumped to + // Order = max + 1 by MoveComplexFormComponent → PickOrder. + if (between.HasAnchor()) { await MoveComplexFormComponent(existing, between); } diff --git a/backend/FwLite/LcmCrdt/OrderPicker.cs b/backend/FwLite/LcmCrdt/OrderPicker.cs index 14eb7746e7..45e42bff3f 100644 --- a/backend/FwLite/LcmCrdt/OrderPicker.cs +++ b/backend/FwLite/LcmCrdt/OrderPicker.cs @@ -8,8 +8,8 @@ public static class OrderPicker public static async Task PickOrder(IQueryable siblings, BetweenPosition? between = null) where T : class, IOrderableNoId, IObjectWithId//this is weird, but WritingSystems should not be IOrderable, because that won't work with FW data, but they have Ids when working with CRDTs { - // a common case that we can optimize by not querying whole objects - if (between is null or { Previous: null, Next: null }) + // common case — skip the full materialisation + if (!between.HasAnchor()) { var currMaxOrder = await siblings.Select(s => s.Order).DefaultIfEmpty().MaxAsync(); return currMaxOrder + 1; diff --git a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs index e767376ed4..4f1d4724f3 100644 --- a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs @@ -98,12 +98,10 @@ public async Task CreateComplexFormComponent_ReplayingReturnedObject_IsIdempoten [Fact] public async Task CreateComplexFormComponent_BetweenWithNoAnchors_DoesNotReorderExisting() { - // The orderable diff in EntrySync hands BetweenPosition(null, null) to Add when - // no stable neighbours exist (e.g. a singleton component). The same CFC can be - // reached twice in one sync — once via SyncComplexForms on the component-side - // entry (no between) and once via SyncComplexFormComponents on the complex-form - // entry (between with both neighbours null). The second call must be a no-op, - // not a spurious reorder that bumps Order from 1 → 2. + // EntrySync's orderable diff produces a no-anchor BetweenPosition for singletons, + // and the same CFC can be reached twice in one sync (via SyncComplexForms then + // SyncComplexFormComponents). The second call must be a no-op, not a Move that + // bumps Order to max + 1 via PickOrder. var first = await Api.CreateComplexFormComponent( ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); var second = await Api.CreateComplexFormComponent( diff --git a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs index f49fa529e7..fdb6975330 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs @@ -268,3 +268,12 @@ public async Task MapAsync(Func> map) } } public record BetweenPosition(Guid? Previous, Guid? Next) : BetweenPosition(Previous, Next); + +public static class BetweenPositionExtensions +{ + /// True when at least one of Previous/Next is non-null. A null receiver — or one + /// with both fields null — has no anchor; the orderable diff produces the latter for + /// singleton or fully-unstable items, and callers should treat it as "no positional intent". + public static bool HasAnchor(this BetweenPosition? between) => + between is { Previous: not null } or { Next: not null }; +} From 2e466d70cb9a2df35ee03577bb67fea764a94ee7 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 27 May 2026 13:06:22 +0200 Subject: [PATCH 3/4] Revert "Extract BetweenPosition.HasAnchor; replace inline pattern matches" This reverts commit 78233aa39a5f7e40a61f55ee536ac04092e27bd0. --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 10 ++++++---- backend/FwLite/LcmCrdt/OrderPicker.cs | 4 ++-- .../MiniLcm.Tests/ComplexFormComponentTestsBase.cs | 10 ++++++---- backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs | 9 --------- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 376ac8d456..9660435e16 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -313,10 +313,12 @@ public async Task CreateComplexFormComponent(ComplexFormCo return await repo.FindComplexFormComponent(addEntryComponentChange.EntityId); } - // Without this guard, the same CFC revisited in one sync (via SyncComplexForms - // then SyncComplexFormComponents with a no-anchor between) gets bumped to - // Order = max + 1 by MoveComplexFormComponent → PickOrder. - if (between.HasAnchor()) + // BetweenPosition(null, null) carries no positional intent — it's what the + // orderable diff hands in for a lone or unanchored item. Treat it as a no-op + // here; otherwise MoveComplexFormComponent walks PickOrder and bumps Order + // to max+1 every time the same CFC is revisited (e.g. via SyncComplexForms + // and SyncComplexFormComponents in one sync). Matches FwDataMiniLcmApi. + if (between is { Previous: not null } or { Next: not null }) { await MoveComplexFormComponent(existing, between); } diff --git a/backend/FwLite/LcmCrdt/OrderPicker.cs b/backend/FwLite/LcmCrdt/OrderPicker.cs index 45e42bff3f..14eb7746e7 100644 --- a/backend/FwLite/LcmCrdt/OrderPicker.cs +++ b/backend/FwLite/LcmCrdt/OrderPicker.cs @@ -8,8 +8,8 @@ public static class OrderPicker public static async Task PickOrder(IQueryable siblings, BetweenPosition? between = null) where T : class, IOrderableNoId, IObjectWithId//this is weird, but WritingSystems should not be IOrderable, because that won't work with FW data, but they have Ids when working with CRDTs { - // common case — skip the full materialisation - if (!between.HasAnchor()) + // a common case that we can optimize by not querying whole objects + if (between is null or { Previous: null, Next: null }) { var currMaxOrder = await siblings.Select(s => s.Order).DefaultIfEmpty().MaxAsync(); return currMaxOrder + 1; diff --git a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs index 4f1d4724f3..e767376ed4 100644 --- a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs @@ -98,10 +98,12 @@ public async Task CreateComplexFormComponent_ReplayingReturnedObject_IsIdempoten [Fact] public async Task CreateComplexFormComponent_BetweenWithNoAnchors_DoesNotReorderExisting() { - // EntrySync's orderable diff produces a no-anchor BetweenPosition for singletons, - // and the same CFC can be reached twice in one sync (via SyncComplexForms then - // SyncComplexFormComponents). The second call must be a no-op, not a Move that - // bumps Order to max + 1 via PickOrder. + // The orderable diff in EntrySync hands BetweenPosition(null, null) to Add when + // no stable neighbours exist (e.g. a singleton component). The same CFC can be + // reached twice in one sync — once via SyncComplexForms on the component-side + // entry (no between) and once via SyncComplexFormComponents on the complex-form + // entry (between with both neighbours null). The second call must be a no-op, + // not a spurious reorder that bumps Order from 1 → 2. var first = await Api.CreateComplexFormComponent( ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); var second = await Api.CreateComplexFormComponent( diff --git a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs index fdb6975330..f49fa529e7 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/DiffCollection.cs @@ -268,12 +268,3 @@ public async Task MapAsync(Func> map) } } public record BetweenPosition(Guid? Previous, Guid? Next) : BetweenPosition(Previous, Next); - -public static class BetweenPositionExtensions -{ - /// True when at least one of Previous/Next is non-null. A null receiver — or one - /// with both fields null — has no anchor; the orderable diff produces the latter for - /// singleton or fully-unstable items, and callers should treat it as "no positional intent". - public static bool HasAnchor(this BetweenPosition? between) => - between is { Previous: not null } or { Next: not null }; -} From c54c002a16022aa09e1b5fec247979a3a7695e8a Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Thu, 28 May 2026 10:29:53 +0200 Subject: [PATCH 4/4] Trim verbose comments on CFC no-anchor guard (#2307) Keep just the WHY: where (null, null) comes from and what skipping the move prevents. The full sync trace belongs in the commit history, not the source. Co-authored-by: Claude --- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 7 ++----- .../FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs | 8 ++------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 9660435e16..5a4053ec35 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -313,11 +313,8 @@ public async Task CreateComplexFormComponent(ComplexFormCo return await repo.FindComplexFormComponent(addEntryComponentChange.EntityId); } - // BetweenPosition(null, null) carries no positional intent — it's what the - // orderable diff hands in for a lone or unanchored item. Treat it as a no-op - // here; otherwise MoveComplexFormComponent walks PickOrder and bumps Order - // to max+1 every time the same CFC is revisited (e.g. via SyncComplexForms - // and SyncComplexFormComponents in one sync). Matches FwDataMiniLcmApi. + // The orderable diff sends (null, null) for singletons; skip the move so + // revisits in one sync don't bump Order via PickOrder. if (between is { Previous: not null } or { Next: not null }) { await MoveComplexFormComponent(existing, between); diff --git a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs index e767376ed4..8c32f57a7f 100644 --- a/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs +++ b/backend/FwLite/MiniLcm.Tests/ComplexFormComponentTestsBase.cs @@ -98,12 +98,8 @@ public async Task CreateComplexFormComponent_ReplayingReturnedObject_IsIdempoten [Fact] public async Task CreateComplexFormComponent_BetweenWithNoAnchors_DoesNotReorderExisting() { - // The orderable diff in EntrySync hands BetweenPosition(null, null) to Add when - // no stable neighbours exist (e.g. a singleton component). The same CFC can be - // reached twice in one sync — once via SyncComplexForms on the component-side - // entry (no between) and once via SyncComplexFormComponents on the complex-form - // entry (between with both neighbours null). The second call must be a no-op, - // not a spurious reorder that bumps Order from 1 → 2. + // Sync can revisit the same CFC with (null, null) — once via SyncComplexForms, + // again via SyncComplexFormComponents. The second call must not reorder. var first = await Api.CreateComplexFormComponent( ComplexFormComponent.FromEntries(_complexFormEntry, _componentEntry)); var second = await Api.CreateComplexFormComponent(