[claude] Resume snapshot replays from checkpoints

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>,
@@ -50,29 +51,13 @@
   },
   {
     $type: Commit,
-    Snapshots: [
-      {
-        $type: ObjectSnapshot,
-        Id: Guid_5,
-        TypeName: Word,
-        Entity: {
-          $type: Word,
-          Text: first,
-          Note: a word note,
-          Id: Guid_2
-        },
-        EntityId: Guid_2,
-        EntityIsDeleted: false,
-        CommitId: Guid_6,
-        IsRoot: false
-      }
-    ],
     Hash: Hash_2,
     ParentHash: Hash_1,
+    IsSnapshotCheckpoint: false,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
-        CommitId: Guid_6,
+        CommitId: Guid_5,
         EntityId: Guid_2,
         Change: {
           $type: SetWordNoteChange,
@@ -85,9 +70,9 @@
     CompareKey: {
       $type: ValueTuple<DateTimeOffset, long,
       Item1: DateTimeOffset_2,
-      Item3: Guid_6
+      Item3: Guid_5
     },
-    Id: Guid_6,
+    Id: Guid_5,
     HybridDateTime: {
       $type: HybridDateTime,
       DateTime: DateTimeOffset_2
@@ -102,10 +87,11 @@
     $type: Commit,
     Hash: Hash_3,
     ParentHash: Hash_2,
+    IsSnapshotCheckpoint: false,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
-        CommitId: Guid_7,
+        CommitId: Guid_6,
         EntityId: Guid_2,
         Change: {
           $type: SetWordTextChange,
@@ -118,9 +104,9 @@
     CompareKey: {
       $type: ValueTuple<DateTimeOffset, long,
       Item1: DateTimeOffset_3,
-      Item3: Guid_7
+      Item3: Guid_6
     },
-    Id: Guid_7,
+    Id: Guid_6,
     HybridDateTime: {
       $type: HybridDateTime,
       DateTime: DateTimeOffset_3
@@ -136,7 +122,7 @@
     Snapshots: [
       {
         $type: ObjectSnapshot,
-        Id: Guid_8,
+        Id: Guid_7,
         TypeName: Word,
         Entity: {
           $type: Word,
@@ -146,16 +132,17 @@
         },
         EntityId: Guid_2,
         EntityIsDeleted: false,
-        CommitId: Guid_9,
+        CommitId: Guid_8,
         IsRoot: false
       }
     ],
     Hash: Hash_4,
     ParentHash: Hash_3,
+    IsSnapshotCheckpoint: true,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
-        CommitId: Guid_9,
+        CommitId: Guid_8,
         EntityId: Guid_2,
         Change: {
           $type: SetWordTextChange,
@@ -168,9 +155,9 @@
     CompareKey: {
       $type: ValueTuple<DateTimeOffset, long,
       Item1: DateTimeOffset_4,
-      Item3: Guid_9
+      Item3: Guid_8
     },
-    Id: Guid_9,
+    Id: Guid_8,
     HybridDateTime: {
       $type: HybridDateTime,
       DateTime: DateTimeOffset_4

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

@@ -89,15 +89,15 @@ public async ValueTask<Commit> WriteChangeBefore(Commit before, IChange change,
         return await WriteChange(_localClientId, before.DateTime.AddHours(-1), change, add);
     }
 
-    protected async ValueTask<Commit> WriteChange(Guid clientId,
+    public async ValueTask<Commit> WriteChange(Guid clientId,
         DateTimeOffset dateTime,
         IChange change,
         bool add = true)
     {
         return await WriteChange(clientId, dateTime, [change], add);
     }
 
-    protected async ValueTask<Commit> WriteChange(Guid clientId,
+    public async ValueTask<Commit> WriteChange(Guid clientId,
         DateTimeOffset dateTime,
         IEnumerable<IChange> changes,
         bool add = true)
@@ -122,7 +122,7 @@ protected async ValueTask<Commit> WriteChange(Guid clientId,
         return await DataModel.AddChanges(clientId, changes);
     }
 
-    protected async Task AddCommitsViaSync(IEnumerable<Commit> commits)
+    public async Task AddCommitsViaSync(IEnumerable<Commit> commits)
     {
         await ((ISyncable)DataModel).AddRangeFromSync(commits);
     }

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
@@ -24,7 +25,7 @@
     Keys: 
       Id PK
     Annotations: 
-      CustomIndex:CompositeIndexes: [{"paths":["HybridDateTime.DateTime","HybridDateTime.Counter","Id"],"unique":false,"name":"IX_Commits_DateTime_Counter_Id"}]
+      CustomIndex:CompositeIndexes: [{"paths":["HybridDateTime.DateTime","HybridDateTime.Counter","Id"],"unique":false,"name":"IX_Commits_DateTime_Counter_Id"},{"paths":["IsSnapshotCheckpoint","HybridDateTime.DateTime","HybridDateTime.Counter","Id"],"unique":false,"name":"IX_Commits_IsSnapshotCheckpoint_DateTime_Counter_Id"}]
       Relational:FunctionName: 
       Relational:Schema: 
       Relational:SqlQuery: 

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,67 @@
+using Microsoft.EntityFrameworkCore;
+using SIL.Harmony.Sample.Changes;
+using SIL.Harmony.Sample.Models;
+
+namespace SIL.Harmony.Tests;
+
+/// <summary>
+/// The smallest shape of the late commit bug, on a single client.
+/// Adding several commits at once only keeps a snapshot for every other one, so an entity ends up
+/// with a commit that has no snapshot. A commit dated between that commit and the next one that
+/// does have a snapshot then makes the replay start after its own parent (the commit with no
+/// snapshot), while snapshots are only deleted from the late commit onwards. The entity resumes
+/// from the older snapshot it still has, and everything between that snapshot and the late
+/// commit's parent is applied by nobody.
+/// </summary>
+public class LateCommitTests : DataModelTestBase
+{
+    private async Task AssertSnapshotWasPruned(Commit commit, Guid entityId)
+    {
+        var snapshots = await DbContext.Snapshots.AsNoTracking()
+            .CountAsync(s => s.CommitId == commit.Id && s.EntityId == entityId);
+        snapshots.Should().Be(0, "otherwise there's no gap and the test proves nothing");
+    }
+
+    [Fact]
+    public async Task ALateCommitKeepsAnEditWhoseSnapshotWasPruned()
+    {
+        var wordId = Guid.NewGuid();
+        // add: false builds the commit without applying it, so all three land in one batch below
+        var create = await WriteNextChange(SetWord(wordId, "word"), add: false);
+        var setNote = await WriteNextChange(new SetWordNoteChange(wordId, "a note"), add: false);
+        var rename = await WriteNextChange(new SetWordTextChange(wordId, "renamed word"), add: false);
+        await AddCommitsViaSync([create, setNote, rename]);
+        // the batch keeps the word's snapshots at create and rename, but not the one in the middle
+        await AssertSnapshotWasPruned(setNote, wordId);
+
+        await WriteChangeAfter(setNote, SetWord(Guid.NewGuid(), "written late"));
+
+        // the rename snapshot is gone and the word resumed from create, so nothing re-applied the note
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word!.Text.Should().Be("renamed word");
+        word.Note.Should().Be("a note");
+    }
+
+    [Fact]
+    public async Task ALateCommitKeepsACascadeDeleteWhoseSnapshotWasPruned()
+    {
+        var wordId = Guid.NewGuid();
+        var definitionId = Guid.NewGuid();
+        var create = await WriteNextChange(SetWord(wordId, "word"), add: false);
+        // only here to shift which snapshots the batch keeps, so the definition loses the one below
+        var unrelated = await WriteNextChange(SetWord(Guid.NewGuid(), "another word"), add: false);
+        var newDefinition = await WriteNextChange(NewDefinition(wordId, "a definition", "noun", definitionId: definitionId), add: false);
+        // deleting the word deletes its definition too, but only as a snapshot: no commit records it
+        var delete = await WriteNextChange(DeleteWord(wordId), add: false);
+        var editDefinition = await WriteNextChange(new SetDefinitionPartOfSpeechChange(definitionId, "verb"), add: false);
+        await AddCommitsViaSync([create, unrelated, newDefinition, delete, editDefinition]);
+        await AssertSnapshotWasPruned(delete, definitionId);
+
+        // the definition resumes from its creation snapshot, and replaying the delete commit does not
+        // cascade again, so the edit lands on a live definition whose word is still deleted.
+        // Projecting that row back in breaks the foreign key to the word.
+        await WriteChangeAfter(delete, SetWord(Guid.NewGuid(), "written late"));
+
+        (await DataModel.GetLatest<Definition>(definitionId))!.DeletedAt.Should().NotBeNull();
+    }
+}

src/SIL.Harmony.Tests/ModelSnapshotTests.cs

@@ -108,9 +108,15 @@ public async Task CanGetWordForASpecificTime()
         thirdWord.Text.Should().Be("third");
     }
 
-    private Task ClearNonRootSnapshots()
+    /// <summary>
+    /// leaves each entity nothing but its root snapshot, so reading state at a commit has to replay history to get there
+    /// </summary>
+    private async Task ClearNonRootSnapshots()
     {
-        return DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(TestContext.Current.CancellationToken);
+        //the flags go first: a checkpoint claims that the snapshots at or before it hold the state a replay resumes from
+        await DbContext.Commits.ExecuteUpdateAsync(s => s.SetProperty(c => c.IsSnapshotCheckpoint, false), TestContext.Current.CancellationToken);
+        await DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(TestContext.Current.CancellationToken);
+        DbContext.ChangeTracker.Clear();
     }
 
     [Theory]
@@ -125,15 +131,16 @@ public async Task CanGetSnapshotFromEarlier(int changeCount)
         var addNew = new List<Commit>(changeCount);
         for (var i = 0; i < changeCount; i++)
         {
-            // todo: these commits all have an odd index, so no intermediate snapshots will be persisted i.e. the snapshot count checking is somewhat deceptive
             changes.Add(await WriteNextChange(SetWord(entityId, $"change {i}"), false).AsTask());
             addNew.Add(await WriteNextChange(SetWord(Guid.NewGuid(), $"add {i}"), false).AsTask());
         }
 
         //adding all via sync means there's sparse snapshots
         await AddCommitsViaSync(changes.Concat(addNew));
-        //there will only be a snapshot for every other commit, but there's change count * 2 commits, plus a first and last change
-        DbContext.Snapshots.Should().HaveCount(2 + changeCount);
+        var commitCount = changeCount * 2;
+        var checkpointCount = Enumerable.Range(1, commitCount).Count(i => SnapshotCheckpointPolicy.Default.IsCheckpoint(i, commitCount));
+        //the root from the first change, a root per newly added word, and one snapshot of the edited word per checkpoint
+        DbContext.Snapshots.Should().HaveCount(1 + changeCount + checkpointCount);
 
         for (int i = 0; i < changeCount; i++)
         {
@@ -156,7 +163,7 @@ await AddCommitsViaSync(Enumerable.Range(0, changeCount)
 
         var latestSnapshot = await DataModel.GetLatestSnapshotByObjectId(entityId);
         //delete snapshots so when we get at then we need to re-apply
-        await DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(TestContext.Current.CancellationToken);
+        await ClearNonRootSnapshots();
 
         var computedModelSnapshots = await DataModel.GetSnapshotsAtCommit(latestSnapshot.Commit);