Add DataModel.GetBeforeCommit to get entity state before a commit

[hahn-kev-bot]

Adds GetBeforeCommit<T>(Guid commitId, Guid entityId) and GetBeforeCommit<T>(Commit commit, Guid entityId), mirroring GetAtCommit but returning the entity state as it was immediately before the given commit was applied.

The methods return default when there is no state before the commit (i.e. the commit is the first commit, or it is the commit that created the entity). The snapshot-resolution logic shared with GetAtCommit is extracted into a private GetSnapshotAtCommit helper.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved retrieval of records at and immediately before a specific commit.
  • Correctly handles cases where no snapshot exists before the requested commit.

• Tests

  • Added coverage verifying historical record values across successive commits.

[coderabbitai]

📝 Walkthrough

Walkthrough

DataModel commit-scoped reads now centralize snapshot retrieval through repository-scoped helpers. A new test verifies that GetBeforeCommit<Word> returns no state for the first commit and the immediately preceding state for subsequent commits.

Changes

Commit Snapshot Reads

Layer / File(s)	Summary
Commit-scoped read flow src/SIL.Harmony/DataModel.cs	GetAtCommit and GetBeforeCommit delegate through commit-scoped repository helpers, while snapshot lookup returns ObjectSnapshot? before the final database-object cast.
Preceding-state regression test src/SIL.Harmony.Tests/ModelSnapshotTests.cs	Adds coverage for three successive commits, validating null before the first commit and prior text values for later commits.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: hahn-kev

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: adding DataModel.GetBeforeCommit to retrieve state before a commit.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch add-getbeforecommit

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/SIL.Harmony/DataModel.cs (1)

328-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding where T : class generic constraint to GetAtCommit and GetBeforeCommit.

In C# 9+, an unconstrained T? means T if T is a value type, rather than Nullable<T>. This can lead to unexpected behavior where returning default yields an uninitialized struct rather than null. Adding the where T : class constraint aligns these methods with GetLatest<T> and ensures that T? strictly represents a nullable reference type.

♻️ Proposed refactor

-    public async Task<T> GetAtCommit<T>(Commit commit, Guid entityId)
+    public async Task<T> GetAtCommit<T>(Commit commit, Guid entityId) where T : class
     {
         await using var repo = await _crdtRepositoryFactory.CreateRepository();
         return await GetAtCommit<T>(commit, entityId, repo);
     }
 
-    private async Task<T> GetAtCommit<T>(Commit commit, Guid entityId, CrdtRepository repo)
+    private async Task<T> GetAtCommit<T>(Commit commit, Guid entityId, CrdtRepository repo) where T : class
     {
         var snapshot = await GetSnapshotAtCommit(commit, entityId, repo);
         ArgumentNullException.ThrowIfNull(snapshot);
         return (T)snapshot.Entity.DbObject;
     }
 
-    public async Task<T?> GetBeforeCommit<T>(Guid commitId, Guid entityId)
+    public async Task<T?> GetBeforeCommit<T>(Guid commitId, Guid entityId) where T : class
     {
         await using var repo = await _crdtRepositoryFactory.CreateRepository();
         var commit = await repo.CurrentCommits().SingleAsync(c => c.Id == commitId);
         return await GetBeforeCommit<T>(commit, entityId, repo);
     }
 
-    public async Task<T?> GetBeforeCommit<T>(Commit commit, Guid entityId)
+    public async Task<T?> GetBeforeCommit<T>(Commit commit, Guid entityId) where T : class
     {
         await using var repo = await _crdtRepositoryFactory.CreateRepository();
         return await GetBeforeCommit<T>(commit, entityId, repo);
     }
 
-    private async Task<T?> GetBeforeCommit<T>(Commit commit, Guid entityId, CrdtRepository repo)
+    private async Task<T?> GetBeforeCommit<T>(Commit commit, Guid entityId, CrdtRepository repo) where T : class

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/SIL.Harmony/DataModel.cs` around lines 328 - 355, Add a where T : class
constraint to every public and private GetAtCommit and GetBeforeCommit overload,
matching the existing GetLatest<T> contract. Ensure nullable T? in
GetBeforeCommit represents only nullable reference types and keep the current
method behavior otherwise unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/SIL.Harmony/DataModel.cs`:
- Around line 328-355: Add a where T : class constraint to every public and
private GetAtCommit and GetBeforeCommit overload, matching the existing
GetLatest<T> contract. Ensure nullable T? in GetBeforeCommit represents only
nullable reference types and keep the current method behavior otherwise
unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6da43833-74db-416d-987a-039652b51625

📥 Commits

Reviewing files that changed from the base of the PR and between 01efe25 and a31bc8c.

📒 Files selected for processing (2)

• src/SIL.Harmony.Tests/ModelSnapshotTests.cs
• src/SIL.Harmony/DataModel.cs