[claude] Roll snapshots back to a checkpoint when a commit arrives out of order

src/SIL.Harmony.Tests/DataModelSimpleChanges.WriteMultipleCommits.verified.txt

@@ -19,6 +19,7 @@
     ],
     Hash: Hash_1,
     ParentHash: Hash_Empty,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -68,6 +69,7 @@
     ],
     Hash: Hash_2,
     ParentHash: Hash_1,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,

src/SIL.Harmony.Tests/DataModelSimpleChanges.Writing2ChangesAtOnceWithMergedHistory.verified.txt

@@ -19,6 +19,7 @@
     ],
     Hash: Hash_1,
     ParentHash: Hash_Empty,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -69,6 +70,7 @@
     ],
     Hash: Hash_2,
     ParentHash: Hash_1,
+    IsSnapshotCheckpoint: false,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -102,6 +104,7 @@
     $type: Commit,
     Hash: Hash_3,
     ParentHash: Hash_2,
+    IsSnapshotCheckpoint: false,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -152,6 +155,7 @@
     ],
     Hash: Hash_4,
     ParentHash: Hash_3,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,

src/SIL.Harmony.Tests/DataModelSimpleChanges.WritingA2ndChangeDoesNotEffectTheFirstSnapshot.verified.txt

@@ -19,6 +19,7 @@
     ],
     Hash: Hash_1,
     ParentHash: Hash_Empty,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -68,6 +69,7 @@
     ],
     Hash: Hash_2,
     ParentHash: Hash_1,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,

src/SIL.Harmony.Tests/DataModelSimpleChanges.WritingAChangeMakesASnapshot.verified.txt

@@ -19,6 +19,7 @@
     ],
     Hash: Hash_1,
     ParentHash: Hash_Empty,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,

src/SIL.Harmony.Tests/DataModelSimpleChanges.WritingACommitWithMultipleChangesWorks.verified.txt

@@ -33,6 +33,7 @@
     ],
     Hash: Hash_1,
     ParentHash: Hash_Empty,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,

src/SIL.Harmony.Tests/DataModelTestBase.cs

@@ -56,11 +56,11 @@ public DataModelTestBase ForkDatabase(bool alwaysValidate = true)
         if (DbContext.Database.GetDbConnection() is not SqliteConnection existingConnection) throw new InvalidOperationException("Database is not SQLite");
         existingConnection.BackupDatabase(connection);
         var newTestBase = new DataModelTestBase(connection, alwaysValidate, performanceTest: _performanceTest);
-        newTestBase.SetCurrentDate(currentDate.DateTime);
+        newTestBase.SetCurrentDate(currentDate);
         return newTestBase;
     }
 
-    public void SetCurrentDate(DateTime dateTime)
+    public void SetCurrentDate(DateTimeOffset dateTime)
     {
         currentDate = dateTime;
     }

src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt

@@ -4,6 +4,7 @@
       Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
       ClientId (Guid) Required
       Hash (string) Required
+      IsSnapshotCheckpoint (bool) Required
       Metadata (CommitMetadata) Required
         Annotations: 
           Relational:ColumnType: jsonb

src/SIL.Harmony.Tests/DbContextTests.cs

@@ -53,6 +53,7 @@ await DbContext.Set<Commit>().ToLinqToDBTable().AsValueInsertable()
             .Value(c => c.Metadata, new CommitMetadata())
             .Value(c => c.Hash, "")
             .Value(c => c.ParentHash, "")
+            .Value(c => c.IsSnapshotCheckpoint, false)
             .InsertAsync(TestContext.Current.CancellationToken);
         var actualCommit = await DbContext.Commits.SingleOrDefaultAsyncEF(c => c.Id == commitId, TestContext.Current.CancellationToken);
         actualCommit!.HybridDateTime.DateTime.Should().Be(expectedDateTime, "EF");

src/SIL.Harmony.Tests/LateCommitTests.cs

@@ -0,0 +1,92 @@
+using SIL.Harmony.Sample.Changes;
+using SIL.Harmony.Sample.Models;
+
+namespace SIL.Harmony.Tests;
+
+/// <summary>
+/// A commit dated before commits a replica already holds makes it roll its snapshots back and replay history.
+/// These tests sync edits to the replica in one batch, which prunes some of their snapshots, and then land a late commit in the gap.
+/// </summary>
+public class LateCommitTests : IAsyncLifetime
+{
+    private readonly DataModelTestBase _author = new();
+    private readonly DataModelTestBase _replica = new();
+    private readonly DataModelTestBase _offlineClient = new();
+
+    public async ValueTask InitializeAsync()
+    {
+        await _author.InitializeAsync();
+        await _replica.InitializeAsync();
+        await _offlineClient.InitializeAsync();
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        await _author.DisposeAsync();
+        await _replica.DisposeAsync();
+        await _offlineClient.DisposeAsync();
+    }
+
+    // which of the edits' snapshots the batch prunes depends on their position in it, so land the late commit after each edit
+    [Theory]
+    [InlineData(0)]
+    [InlineData(1)]
+    public async Task EditsSyncedInOneBatchSurviveALateCommitBetweenThem(int lateCommitAfterEdit)
+    {
+        var wordId = Guid.NewGuid();
+        var antonymId = Guid.NewGuid();
+        await _author.WriteNextChange([_author.SetWord(wordId, "word"), _author.SetWord(antonymId, "antonym")]);
+        Commit[] edits =
+        [
+            await _author.WriteNextChange(new SetWordNoteChange(wordId, "a note")),
+            await _author.WriteNextChange(new SetAntonymReferenceChange(wordId, antonymId)),
+            await _author.WriteNextChange(new SetWordTextChange(wordId, "renamed word")),
+        ];
+        await _replica.DataModel.SyncWith(_author.DataModel);
+
+        // another client wrote an unrelated word while offline, dated between two of the edits
+        await _offlineClient.WriteChangeAfter(edits[lateCommitAfterEdit], _offlineClient.SetWord(Guid.NewGuid(), "written offline"));
+        await _replica.DataModel.SyncWith(_offlineClient.DataModel);
+
+        var replicaWord = await _replica.DataModel.GetLatest<Word>(wordId);
+        var authorWord = await _author.DataModel.GetLatest<Word>(wordId);
+        replicaWord.Should().BeEquivalentTo(authorWord);
+    }
+
+    // whether the replay keeps the definition's cascade-delete snapshot depends on the edit's position in the batch,
+    // which the size of the backlog before the delete shifts by one
+    [Theory]
+    [InlineData(1)]
+    [InlineData(2)]
+    public async Task CascadeDeleteSurvivesALateCommitAfterIt(int backlogSize)
+    {
+        var wordId = Guid.NewGuid();
+        var definitionId = Guid.NewGuid();
+        await _author.WriteNextChange(_author.SetWord(wordId, "word"));
+        await _author.WriteNextChange(_author.NewDefinition(wordId, "a definition", "noun", definitionId: definitionId));
+        await _replica.DataModel.SyncWith(_author.DataModel);
+        await _offlineClient.DataModel.SyncWith(_author.DataModel);
+
+        // deleting the word deletes its definition too, but no commit records that: it only exists as a snapshot
+        var delete = await _author.WriteNextChange(_author.DeleteWord(wordId));
+        await _replica.DataModel.SyncWith(_author.DataModel);
+
+        // meanwhile the offline client wrote some words before the delete happened...
+        var backlog = delete;
+        for (var i = 0; i < backlogSize; i++)
+        {
+            backlog = await _offlineClient.WriteChangeBefore(backlog, _offlineClient.SetWord(Guid.NewGuid(), $"offline word {i}"));
+        }
+        // ...and, never having received the delete, edits the definition after it
+        _offlineClient.SetCurrentDate(delete.DateTime);
+        var edit = await _offlineClient.WriteNextChange(new SetDefinitionPartOfSpeechChange(definitionId, "verb"));
+        await _replica.DataModel.SyncWith(_offlineClient.DataModel);
+        (await _replica.DataModel.GetLatest<Definition>(definitionId))!.DeletedAt.Should().NotBeNull();
+
+        // the author writes another word, dated between the delete and the edit it doesn't know about yet
+        await _author.WriteChangeBefore(edit, _author.SetWord(Guid.NewGuid(), "another word"));
+        await _replica.DataModel.SyncWith(_author.DataModel);
+
+        (await _replica.DataModel.GetLatest<Definition>(definitionId))!.DeletedAt.Should().NotBeNull();
+    }
+}

src/SIL.Harmony.Tests/RepositoryTests.cs

@@ -245,56 +245,77 @@ await _repository.AddSnapshots([
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_WithNoSnapshots_DoesNothing()
+    public async Task DeleteSnapshotsAfter_WithNoSnapshots_DoesNothing()
     {
-        //the empty-repository branch: nothing to delete, must not throw
-        await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(1, 0)));
+        await _repository.DeleteSnapshotsAfter(Commit(Guid.NewGuid(), Time(1, 0)));
 
         _crdtDbContext.Snapshots.Should().BeEmpty();
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_KeepsSnapshotsOlderThanTheCommit()
+    public async Task DeleteSnapshotsAfter_Null_DeletesEverySnapshot()
+    {
+        await _repository.AddSnapshots([
+            Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 0)),
+            Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(2, 0)),
+        ]);
+
+        await _repository.DeleteSnapshotsAfter(null);
+
+        _crdtDbContext.Snapshots.Should().BeEmpty();
+    }
+
+    [Fact]
+    public async Task HasSnapshotsAfter_ComparesTheFullCommitOrder()
+    {
+        await _repository.AddSnapshots([Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 1))]);
+
+        (await _repository.HasSnapshotsAfter(Commit(Guid.NewGuid(), Time(1, 0)))).Should().BeTrue();
+        (await _repository.HasSnapshotsAfter(Commit(Guid.NewGuid(), Time(1, 2)))).Should().BeFalse();
+    }
+
+    [Fact]
+    public async Task DeleteSnapshotsAfter_KeepsSnapshotsOlderThanTheCommit()
     {
         await _repository.AddSnapshots([
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 0)),
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(2, 0)),
         ]);
 
         //the new commit is newer than every existing snapshot, so none are stale
-        await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(3, 0)));
+        await _repository.DeleteSnapshotsAfter(Commit(Guid.NewGuid(), Time(3, 0)));
 
         _crdtDbContext.Snapshots.Should().HaveCount(2);
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_DeletesSnapshotsAfterCommitByTime()
+    public async Task DeleteSnapshotsAfter_DeletesSnapshotsAfterCommitByTime()
     {
         await _repository.AddSnapshots([
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 0)),
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(3, 0)),
         ]);
-        await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(2, 0)));
+        await _repository.DeleteSnapshotsAfter(Commit(Guid.NewGuid(), Time(2, 0)));
 
         _crdtDbContext.Snapshots.Include(s => s.Commit).Should().ContainSingle()
             .Which.Commit.HybridDateTime.DateTime.Hour.Should().Be(1);
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_DeletesSnapshotsAfterCommitByCount()
+    public async Task DeleteSnapshotsAfter_DeletesSnapshotsAfterCommitByCount()
     {
         await _repository.AddSnapshots([
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 0)),
             Snapshot(Guid.NewGuid(), Guid.NewGuid(), Time(1, 2)),
         ]);
-        await _repository.DeleteStaleSnapshots(Commit(Guid.NewGuid(), Time(1, 1)));
+        await _repository.DeleteSnapshotsAfter(Commit(Guid.NewGuid(), Time(1, 1)));
 
         _crdtDbContext.Snapshots.Include(s => s.Commit).Should().ContainSingle()
             .Which.Commit.HybridDateTime.Counter.Should().Be(0);
     }
 
     [Fact]
-    public async Task DeleteStaleSnapshots_DeletesSnapshotsAfterCommitByCommitId()
+    public async Task DeleteSnapshotsAfter_DeletesSnapshotsAfterCommitByCommitId()
     {
         var time = Time(1, 1);
         var entityId = Guid.NewGuid();
@@ -303,7 +324,7 @@ await _repository.AddSnapshots([
             Snapshot(entityId, ids[0], time),
             Snapshot(entityId, ids[2], time),
         ]);
-        await _repository.DeleteStaleSnapshots(Commit(ids[1], time));
+        await _repository.DeleteSnapshotsAfter(Commit(ids[1], time));
 
         _crdtDbContext.Snapshots.Should().ContainSingle()
             .Which.CommitId.Should().Be(ids[0]);

src/SIL.Harmony.Tests/SnapshotCheckpointTests.cs

@@ -0,0 +1,67 @@
+using Microsoft.EntityFrameworkCore;
+
+namespace SIL.Harmony.Tests;
+
+public class SnapshotCheckpointTests : DataModelTestBase
+{
+    private async Task<Guid[]> CheckpointIds()
+    {
+        return await DbContext.Commits.AsNoTracking()
+            .Where(c => c.IsSnapshotCheckpoint)
+            .Select(c => c.Id)
+            .ToArrayAsync(TestContext.Current.CancellationToken);
+    }
+
+    [Fact]
+    public async Task EveryLocallyAuthoredCommitIsACheckpoint()
+    {
+        var entityId = Guid.NewGuid();
+        var commit1 = await WriteNextChange(SetWord(entityId, "first"));
+        var commit2 = await WriteNextChange(SetWord(entityId, "second"));
+
+        (await CheckpointIds()).Should().BeEquivalentTo([commit1.Id, commit2.Id]);
+    }
+
+    [Fact]
+    public async Task ASyncedBatchOnlyMakesItsLastCommitACheckpoint()
+    {
+        var entityId = Guid.NewGuid();
+        var commits = new[]
+        {
+            await WriteNextChange(SetWord(entityId, "first"), add: false),
+            await WriteNextChange(SetWord(entityId, "second"), add: false),
+            await WriteNextChange(SetWord(entityId, "third"), add: false),
+        };
+
+        await AddCommitsViaSync(commits);
+
+        (await CheckpointIds()).Should().BeEquivalentTo([commits[2].Id]);
+    }
+
+    [Fact]
+    public async Task ALateCommitClearsTheCheckpointsItReplays()
+    {
+        var entityId = Guid.NewGuid();
+        var commit1 = await WriteNextChange(SetWord(entityId, "first"));
+        var commit2 = await WriteNextChange(SetWord(entityId, "second"));
+        var commit3 = await WriteNextChange(SetWord(entityId, "third"));
+
+        var lateCommit = await WriteChangeBefore(commit2, SetWord(Guid.NewGuid(), "late"));
+
+        //the replay resumed from commit1 and ran through commit3, so only its last commit is a checkpoint again
+        (await CheckpointIds()).Should().BeEquivalentTo([commit1.Id, commit3.Id]);
+        lateCommit.IsSnapshotCheckpoint.Should().BeFalse();
+    }
+
+    [Fact]
+    public async Task RegeneratingSnapshotsLeavesOnlyTheLastCommitAsACheckpoint()
+    {
+        var entityId = Guid.NewGuid();
+        await WriteNextChange(SetWord(entityId, "first"));
+        var lastCommit = await WriteNextChange(SetWord(entityId, "second"));
+
+        await DataModel.RegenerateSnapshots();
+
+        (await CheckpointIds()).Should().BeEquivalentTo([lastCommit.Id]);
+    }
+}

src/SIL.Harmony/Commit.cs

@@ -40,4 +40,13 @@ internal Commit() : this(Guid.NewGuid())
 
     [JsonIgnore]
     public string ParentHash { get; private set; }
+
+    /// <summary>
+    /// Snapshots are complete as of this commit: every entity's newest snapshot at or before it is that entity's state after it,
+    /// except for changes this client could not apply (see <see cref="Config.UnknownChangeHandling"/>), which only <see cref="DataModel.RegenerateSnapshots"/> folds in.
+    /// A commit that arrives out of order rolls snapshots back to the newest checkpoint before it and replays from there.
+    /// Local bookkeeping, never synced.
+    /// </summary>
+    [JsonIgnore]
+    public bool IsSnapshotCheckpoint { get; internal set; }
 }