From 467b4d0c8d2a3c4d43b500641d99c1d313a9fecc Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 30 Jun 2026 10:29:54 +0200 Subject: [PATCH 1/9] Add cross-ORM date parity test; run FwLite tests under non-UTC zone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Locks in that every persisted DateTimeOffset round-trips to the same UTC instant through both EF Core and linq2db (#2092). linq2db skips the converter for the Harmony commit timestamp so HistoryService normalizes it by hand; comment dates use the global EF converter, which linq2db also applies. These errors equal the local UTC offset, so they vanish on UTC — hence the FwLite test job now runs under Eastern time (non-UTC, DST, negative offset). Co-Authored-By: Claude Opus 4.8 --- .github/workflows/fw-lite.yaml | 7 ++ .../DateTimeOffsetOrmParityTests.cs | 119 ++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs diff --git a/.github/workflows/fw-lite.yaml b/.github/workflows/fw-lite.yaml index 1096b5d193..bd336a4bf6 100644 --- a/.github/workflows/fw-lite.yaml +++ b/.github/workflows/fw-lite.yaml @@ -66,6 +66,13 @@ jobs: - name: Check for stale generated TypeScript types run: task fw-lite:has-stale-generated-types -- --no-build + - name: Set a non-UTC timezone so timezone bugs are observable + # Hosted runners default to UTC, which hides date bugs: the error equals the local UTC + # offset, which is zero on UTC (see #2092). Eastern is non-UTC, observes DST, and has a + # negative offset, so date round-trips run the way real users actually hit them. + shell: pwsh + run: Set-TimeZone -Id "Eastern Standard Time" + - name: Dotnet test # Benchmarks run in a separate Release-mode job — see `benchmark` below. run: dotnet test FwLiteOnly.slnf --logger GitHubActions --no-build -p:BuildAndroid=false --filter "Category!=Benchmark" diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs new file mode 100644 index 0000000000..1eb6955617 --- /dev/null +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -0,0 +1,119 @@ +using LcmCrdt.Changes; +using LinqToDB; +using LinqToDB.Async; +using LinqToDB.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore; +using Xunit.Abstractions; + +namespace LcmCrdt.Tests; + +// Locks in correct timezone handling for every DateTimeOffset we persist: each must round-trip to +// the same UTC instant through BOTH EF Core and linq2db. +// +// - Plain DateTimeOffset properties (comment CreatedAt/UpdatedAt) go through the global EF +// converter in LcmCrdtDbContext, which linq2db.EntityFrameworkCore also applies. +// - The Harmony commit timestamp (HybridDateTime.DateTime) is an owned-complex-type member that +// linq2db maps without that conversion, so HistoryService normalizes it by hand (issue #2092). +// +// These bugs shift the instant by exactly the local UTC offset, so they are invisible on UTC. The +// FwLite CI job runs under a non-UTC, DST-observing zone (see fw-lite.yaml) so the checks actually +// bite; the commit-timestamp test also self-skips on a UTC dev machine instead of passing vacuously. +// Touches no process-global state, so it is safe to run in parallel. +public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable +{ + private MiniLcmApiFixture _fixture = null!; + private HistoryService History => _fixture.GetService(); + private DataModel DataModel => _fixture.DataModel; + private Guid ClientId => _fixture.GetService().ProjectData.ClientId; + + public async Task InitializeAsync() + { + _fixture = MiniLcmApiFixture.Create(); + _fixture.LogTo(output); + await _fixture.InitializeAsync(); + } + + public async Task DisposeAsync() => await _fixture.DisposeAsync(); + async ValueTask IAsyncDisposable.DisposeAsync() => await DisposeAsync(); + + [Fact] + public async Task CommentTimestamps_SameUtcInstant_ThroughEfCoreAndLinq2db() + { + var before = DateTimeOffset.UtcNow; + var thread = await _fixture.Api.CreateCommentThread( + new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid() }, + new UserComment { Id = Guid.NewGuid(), Text = "hi" }); + var after = DateTimeOffset.UtcNow; + var comment = (await _fixture.Api.GetUserComments(thread.Id).ToArrayAsync()).Single(); + + await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); + var efThread = await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), t => t.Id == thread.Id); + var efComment = await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), c => c.Id == comment.Id); + + var fields = new (string Name, DateTimeOffset Ef, DateTimeOffset Linq2db)[] + { + ("CommentThread.CreatedAt", efThread.CreatedAt, + await Linq2db(ctx.Set().Where(t => t.Id == thread.Id).Select(t => t.CreatedAt))), + ("CommentThread.UpdatedAt", efThread.UpdatedAt, + await Linq2db(ctx.Set().Where(t => t.Id == thread.Id).Select(t => t.UpdatedAt))), + ("UserComment.CreatedAt", efComment.CreatedAt, + await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.CreatedAt))), + ("UserComment.UpdatedAt", efComment.UpdatedAt, + await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.UpdatedAt))), + }; + + foreach (var (name, ef, l2db) in fields) + { + output.WriteLine($"{name}: EF {ef:o} | linq2db {l2db:o}"); + l2db.UtcDateTime.Should().Be(ef.UtcDateTime, $"{name}: linq2db and EF must report the same instant"); + ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, + $"{name}: must round-trip the creation instant"); + ef.Offset.Should().Be(TimeSpan.Zero, $"{name}: EF should normalize to UTC"); + l2db.Offset.Should().Be(TimeSpan.Zero, $"{name}: linq2db should normalize to UTC"); + } + } + + [Fact] + public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() + { + if (TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow) == TimeSpan.Zero) + { + // The commit timestamp's linq2db discrepancy equals the local offset, so a UTC machine + // can't observe it. Bail loudly rather than pass vacuously (this xUnit has no dynamic skip). + output.WriteLine("INCONCLUSIVE: machine is on UTC; the commit-timestamp linq2db path is unobservable here."); + return; + } + + var entryId = Guid.NewGuid(); + var commit = await DataModel.AddChange(ClientId, + new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); + var truthUtc = commit.HybridDateTime.DateTime.UtcDateTime; + + await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); + var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; + + var activities = await History.ProjectActivity(0, 1000).ToArrayAsync(); + var projectActivityTimestamp = activities.First(a => a.CommitId == commit.Id).Timestamp; + + var history = await History.GetHistory(entryId).ToArrayAsync(); + var historyTimestamp = history.First(h => h.CommitId == commit.Id).Timestamp; + + foreach (var (name, value) in new[] + { + ("EF Core", efTimestamp), + ("ProjectActivity (linq2db)", projectActivityTimestamp), + ("GetHistory (linq2db)", historyTimestamp), + }) + { + output.WriteLine($"{name}: {value:o}"); + value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); + value.Offset.Should().Be(TimeSpan.Zero, $"{name} should carry a UTC offset, not the ambient zone"); + } + } + + private static Task Linq2db(IQueryable query) => + query.ToLinqToDB().FirstAsyncLinqToDB(); +} From 7c97a72d864a2dc291502eb30a112c2191898fab Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 30 Jun 2026 15:48:51 +0200 Subject: [PATCH 2/9] Reuse Harmony's linq2db schema so NormalizeTimestamp is unneeded lexbox's second UseLinqToDB created a fresh MappingSchema that shadowed Harmony's, dropping Harmony's Commit.HybridDateTime.DateTime UTC converter. linq2db then read commit timestamps in local time, so HistoryService normalized them by hand (#2092). Extend Harmony's existing schema instead of replacing it: the converter now applies, the redundant Commit column re-declaration is gone, and HistoryService.NormalizeTimestamp is deleted. The parity test now also covers UnreadComment.MarkedUnreadAt and fails loudly instead of a silent no-op when run on UTC. Co-Authored-By: Claude Opus 4.8 --- .../DateTimeOffsetOrmParityTests.cs | 113 ++++++++++-------- backend/FwLite/LcmCrdt/HistoryService.cs | 11 +- backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 14 ++- 3 files changed, 73 insertions(+), 65 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index 1eb6955617..af99ff866e 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -1,4 +1,5 @@ using LcmCrdt.Changes; +using LcmCrdt.Data; using LinqToDB; using LinqToDB.Async; using LinqToDB.EntityFrameworkCore; @@ -7,17 +8,21 @@ namespace LcmCrdt.Tests; -// Locks in correct timezone handling for every DateTimeOffset we persist: each must round-trip to -// the same UTC instant through BOTH EF Core and linq2db. -// -// - Plain DateTimeOffset properties (comment CreatedAt/UpdatedAt) go through the global EF +// Locks in correct timezone handling for every DateTimeOffset column we persist: each must +// round-trip to the same UTC instant through BOTH EF Core and linq2db. They all rely on a +// ValueConverter that stores UTC and reads back at offset zero: +// - the Harmony commit timestamp (HybridDateTime.DateTime) via Harmony's linq2db mapping, which +// LcmCrdtKernel must *extend* rather than replace (issue #2092); +// - the plain columns (comment CreatedAt/UpdatedAt, UnreadComment.MarkedUnreadAt) via the global // converter in LcmCrdtDbContext, which linq2db.EntityFrameworkCore also applies. -// - The Harmony commit timestamp (HybridDateTime.DateTime) is an owned-complex-type member that -// linq2db maps without that conversion, so HistoryService normalizes it by hand (issue #2092). // -// These bugs shift the instant by exactly the local UTC offset, so they are invisible on UTC. The -// FwLite CI job runs under a non-UTC, DST-observing zone (see fw-lite.yaml) so the checks actually -// bite; the commit-timestamp test also self-skips on a UTC dev machine instead of passing vacuously. +// A broken converter shifts the instant by exactly the local UTC offset, so it is invisible on UTC. +// The FwLite CI job runs under a non-UTC, DST-observing zone (see fw-lite.yaml); RequireNonUtc() +// fails loudly if that regresses rather than letting these tests pass vacuously. +// +// DeletedAt is deliberately absent: projected tables drop deleted rows, so it is always null in a +// queryable column; its historical values live only in snapshot JSON, exercised by the +// serialization regression tests, not via ORM column materialization. // Touches no process-global state, so it is safe to run in parallel. public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable { @@ -37,20 +42,59 @@ public async Task InitializeAsync() async ValueTask IAsyncDisposable.DisposeAsync() => await DisposeAsync(); [Fact] - public async Task CommentTimestamps_SameUtcInstant_ThroughEfCoreAndLinq2db() + public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() { + RequireNonUtc(); + + var entryId = Guid.NewGuid(); + var commit = await DataModel.AddChange(ClientId, + new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); + var truthUtc = commit.HybridDateTime.DateTime.UtcDateTime; + + await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); + var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; + + var activities = await History.ProjectActivity(0, 1000).ToArrayAsync(); + var projectActivityTimestamp = activities.First(a => a.CommitId == commit.Id).Timestamp; + + var history = await History.GetHistory(entryId).ToArrayAsync(); + var historyTimestamp = history.First(h => h.CommitId == commit.Id).Timestamp; + + foreach (var (name, value) in new[] + { + ("EF Core", efTimestamp), + ("ProjectActivity (linq2db)", projectActivityTimestamp), + ("GetHistory (linq2db)", historyTimestamp), + }) + { + output.WriteLine($"{name}: {value:o}"); + value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); + value.Offset.Should().Be(TimeSpan.Zero, $"{name} should carry a UTC offset, not the ambient zone"); + } + } + + [Fact] + public async Task PlainColumnTimestamps_SameUtcInstant_ThroughEfCoreAndLinq2db() + { + RequireNonUtc(); + var before = DateTimeOffset.UtcNow; var thread = await _fixture.Api.CreateCommentThread( new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid() }, new UserComment { Id = Guid.NewGuid(), Text = "hi" }); - var after = DateTimeOffset.UtcNow; var comment = (await _fixture.Api.GetUserComments(thread.Id).ToArrayAsync()).Single(); + await _fixture.GetService() + .MarkCommentsUnread([(comment.Id, thread.Id)]); + var after = DateTimeOffset.UtcNow; await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); var efThread = await EntityFrameworkQueryableExtensions.SingleAsync( ctx.Set().AsNoTracking(), t => t.Id == thread.Id); var efComment = await EntityFrameworkQueryableExtensions.SingleAsync( ctx.Set().AsNoTracking(), c => c.Id == comment.Id); + var efUnread = await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), u => u.CommentId == comment.Id); var fields = new (string Name, DateTimeOffset Ef, DateTimeOffset Linq2db)[] { @@ -62,6 +106,8 @@ await Linq2db(ctx.Set().Where(t => t.Id == thread.Id).Select(t => await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.CreatedAt))), ("UserComment.UpdatedAt", efComment.UpdatedAt, await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.UpdatedAt))), + ("UnreadComment.MarkedUnreadAt", efUnread.MarkedUnreadAt, + await Linq2db(ctx.Set().Where(u => u.CommentId == comment.Id).Select(u => u.MarkedUnreadAt))), }; foreach (var (name, ef, l2db) in fields) @@ -69,50 +115,17 @@ await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => output.WriteLine($"{name}: EF {ef:o} | linq2db {l2db:o}"); l2db.UtcDateTime.Should().Be(ef.UtcDateTime, $"{name}: linq2db and EF must report the same instant"); ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, - $"{name}: must round-trip the creation instant"); + $"{name}: must round-trip the instant it was written"); ef.Offset.Should().Be(TimeSpan.Zero, $"{name}: EF should normalize to UTC"); l2db.Offset.Should().Be(TimeSpan.Zero, $"{name}: linq2db should normalize to UTC"); } } - [Fact] - public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() - { - if (TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow) == TimeSpan.Zero) - { - // The commit timestamp's linq2db discrepancy equals the local offset, so a UTC machine - // can't observe it. Bail loudly rather than pass vacuously (this xUnit has no dynamic skip). - output.WriteLine("INCONCLUSIVE: machine is on UTC; the commit-timestamp linq2db path is unobservable here."); - return; - } - - var entryId = Guid.NewGuid(); - var commit = await DataModel.AddChange(ClientId, - new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); - var truthUtc = commit.HybridDateTime.DateTime.UtcDateTime; - - await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); - var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( - ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; - - var activities = await History.ProjectActivity(0, 1000).ToArrayAsync(); - var projectActivityTimestamp = activities.First(a => a.CommitId == commit.Id).Timestamp; - - var history = await History.GetHistory(entryId).ToArrayAsync(); - var historyTimestamp = history.First(h => h.CommitId == commit.Id).Timestamp; - - foreach (var (name, value) in new[] - { - ("EF Core", efTimestamp), - ("ProjectActivity (linq2db)", projectActivityTimestamp), - ("GetHistory (linq2db)", historyTimestamp), - }) - { - output.WriteLine($"{name}: {value:o}"); - value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); - value.Offset.Should().Be(TimeSpan.Zero, $"{name} should carry a UTC offset, not the ambient zone"); - } - } + // These bugs are unobservable at offset zero, so a UTC run proves nothing. CI sets a non-UTC + // zone (fw-lite.yaml); fail loudly here rather than pass vacuously if the run is on UTC. + private static void RequireNonUtc() => + TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow).Should().NotBe(TimeSpan.Zero, + "this test only observes timezone bugs under a non-UTC zone — CI sets one in fw-lite.yaml; set your local timezone to run it"); private static Task Linq2db(IQueryable query) => query.ToLinqToDB().FirstAsyncLinqToDB(); diff --git a/backend/FwLite/LcmCrdt/HistoryService.cs b/backend/FwLite/LcmCrdt/HistoryService.cs index 2c6407a7c9..3c002a4c18 100644 --- a/backend/FwLite/LcmCrdt/HistoryService.cs +++ b/backend/FwLite/LcmCrdt/HistoryService.cs @@ -140,7 +140,7 @@ public async IAsyncEnumerable ProjectActivity(int skip = 0, int var queryable = from commit in commits.Skip(skip).Take(take) select new ProjectActivity(commit.Id, - NormalizeTimestamp(commit.HybridDateTime.DateTime), + commit.HybridDateTime.DateTime, commit.ChangeEntities.ToList(), commit.Metadata); await foreach (var projectActivity in queryable.ToLinqToDB().AsAsyncEnumerable()) @@ -277,7 +277,7 @@ from change in changeEntities.LeftJoin(c => #pragma warning restore CS8073 // The result of the expression is always the same since a value of this type is never equal to 'null' select new HistoryLineItem(commit, entityId, - NormalizeTimestamp(commit.HybridDateTime.DateTime), + commit.HybridDateTime.DateTime, snapshot.Id, change.Index, change, @@ -361,13 +361,6 @@ private async IAsyncEnumerable GetAffectedEntryIds(ChangeEntity c } } - internal static DateTimeOffset NormalizeTimestamp(DateTimeOffset timestamp) - { - // Linq2DB materializes datetime columns as local time; reinterpret the captured ticks as UTC to avoid DST offsets. - // see: https://github.com/sillsdev/languageforge-lexbox/issues/2092 - return new DateTimeOffset(timestamp.Ticks, TimeSpan.Zero); - } - public static string ChangesNameHelper(List> changeEntities) { return changeEntities switch diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 970116a127..882a51a1e1 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -130,11 +130,13 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .UseLinqToDbCrdt(provider) .UseLinqToDB(optionsBuilder => { - var mappingSchema = new MappingSchema(); - new FluentMappingBuilder(mappingSchema).HasAttribute(new ColumnAttribute("DateTime", - nameof(Commit.HybridDateTime) + "." + nameof(HybridDateTime.DateTime))) - .HasAttribute(new ColumnAttribute(nameof(HybridDateTime.Counter), - nameof(Commit.HybridDateTime) + "." + nameof(HybridDateTime.Counter))) + // Reuse Harmony's existing mapping schema; a fresh one shadows it and drops Harmony's + // Commit.HybridDateTime.DateTime UTC conversion, making linq2db read commit timestamps + // in local time (issue #2092). Extending it keeps the conversion — no manual fixup needed. + var mappingSchema = optionsBuilder.DbContextOptions.GetLinqToDBOptions()?.ConnectionOptions.MappingSchema; + var isNewSchema = mappingSchema is null; + mappingSchema ??= new MappingSchema(); + new FluentMappingBuilder(mappingSchema) //tells linq2db to rewrite Sense.SemanticDomainRows / Entry.PublishInRows into //Json.Query(). The rewrite lives on the *Rows shadow accessors //rather than the real IList columns; see Entry.PublishInRows for why. @@ -151,7 +153,7 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .Build(); mappingSchema.SetConvertExpression((WritingSystemId id) => new DataParameter { Value = id.Code, DataType = DataType.Text }); - optionsBuilder.AddMappingSchema(mappingSchema); + if (isNewSchema) optionsBuilder.AddMappingSchema(mappingSchema); optionsBuilder.AddCustomOptions(options => options.UseSQLite()); // Register read-relevant interceptors for LinqToDB From ecd7af4e18a4bc4da8f0a8e2f5a6a9466ca3d143 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 30 Jun 2026 15:59:40 +0200 Subject: [PATCH 3/9] Cover DeletedAt and de-duplicate the timestamp parity test Collapse the repetitive per-column tuples into a generic AssertColumnsAreUtc helper, and add a DeletedAt test that reads it from the deleted snapshot (EntityIsDeleted column) since it is never non-null in a projected column. Tightened comments. Co-Authored-By: Claude Opus 4.8 --- .../DateTimeOffsetOrmParityTests.cs | 117 ++++++++---------- 1 file changed, 54 insertions(+), 63 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index af99ff866e..a019a535d2 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -1,35 +1,28 @@ +using System.Linq.Expressions; using LcmCrdt.Changes; using LcmCrdt.Data; using LinqToDB; using LinqToDB.Async; using LinqToDB.EntityFrameworkCore; using Microsoft.EntityFrameworkCore; +using SIL.Harmony.Changes; +using SIL.Harmony.Db; using Xunit.Abstractions; namespace LcmCrdt.Tests; -// Locks in correct timezone handling for every DateTimeOffset column we persist: each must -// round-trip to the same UTC instant through BOTH EF Core and linq2db. They all rely on a -// ValueConverter that stores UTC and reads back at offset zero: -// - the Harmony commit timestamp (HybridDateTime.DateTime) via Harmony's linq2db mapping, which -// LcmCrdtKernel must *extend* rather than replace (issue #2092); -// - the plain columns (comment CreatedAt/UpdatedAt, UnreadComment.MarkedUnreadAt) via the global -// converter in LcmCrdtDbContext, which linq2db.EntityFrameworkCore also applies. -// -// A broken converter shifts the instant by exactly the local UTC offset, so it is invisible on UTC. -// The FwLite CI job runs under a non-UTC, DST-observing zone (see fw-lite.yaml); RequireNonUtc() -// fails loudly if that regresses rather than letting these tests pass vacuously. -// -// DeletedAt is deliberately absent: projected tables drop deleted rows, so it is always null in a -// queryable column; its historical values live only in snapshot JSON, exercised by the -// serialization regression tests, not via ORM column materialization. -// Touches no process-global state, so it is safe to run in parallel. +// Every persisted DateTimeOffset must round-trip to the same UTC instant through BOTH EF Core and +// linq2db. A broken converter shifts the instant by exactly the local UTC offset, so it is invisible +// on UTC (why #2092 shipped); CI runs a non-UTC zone (fw-lite.yaml) and RequireNonUtc() fails loudly +// if that regresses. DeletedAt is null in projected columns, so it's read from the snapshot entity JSON. public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable { private MiniLcmApiFixture _fixture = null!; private HistoryService History => _fixture.GetService(); private DataModel DataModel => _fixture.DataModel; private Guid ClientId => _fixture.GetService().ProjectData.ClientId; + private Task NewContext() => + _fixture.GetService>().CreateDbContextAsync(); public async Task InitializeAsync() { @@ -51,26 +44,18 @@ public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); var truthUtc = commit.HybridDateTime.DateTime.UtcDateTime; - await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); + await using var ctx = await NewContext(); var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; - - var activities = await History.ProjectActivity(0, 1000).ToArrayAsync(); - var projectActivityTimestamp = activities.First(a => a.CommitId == commit.Id).Timestamp; - - var history = await History.GetHistory(entryId).ToArrayAsync(); - var historyTimestamp = history.First(h => h.CommitId == commit.Id).Timestamp; + var activityTimestamp = (await History.ProjectActivity(0, 1000).ToArrayAsync()).First(a => a.CommitId == commit.Id).Timestamp; + var historyTimestamp = (await History.GetHistory(entryId).ToArrayAsync()).First(h => h.CommitId == commit.Id).Timestamp; foreach (var (name, value) in new[] - { - ("EF Core", efTimestamp), - ("ProjectActivity (linq2db)", projectActivityTimestamp), - ("GetHistory (linq2db)", historyTimestamp), - }) + { ("EF Core", efTimestamp), ("ProjectActivity", activityTimestamp), ("GetHistory", historyTimestamp) }) { output.WriteLine($"{name}: {value:o}"); value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); - value.Offset.Should().Be(TimeSpan.Zero, $"{name} should carry a UTC offset, not the ambient zone"); + value.Offset.Should().Be(TimeSpan.Zero, $"{name} should be UTC, not the ambient zone"); } } @@ -84,49 +69,55 @@ public async Task PlainColumnTimestamps_SameUtcInstant_ThroughEfCoreAndLinq2db() new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid() }, new UserComment { Id = Guid.NewGuid(), Text = "hi" }); var comment = (await _fixture.Api.GetUserComments(thread.Id).ToArrayAsync()).Single(); - await _fixture.GetService() - .MarkCommentsUnread([(comment.Id, thread.Id)]); + await _fixture.GetService().MarkCommentsUnread([(comment.Id, thread.Id)]); var after = DateTimeOffset.UtcNow; - await using var ctx = await _fixture.GetService>().CreateDbContextAsync(); - var efThread = await EntityFrameworkQueryableExtensions.SingleAsync( - ctx.Set().AsNoTracking(), t => t.Id == thread.Id); - var efComment = await EntityFrameworkQueryableExtensions.SingleAsync( - ctx.Set().AsNoTracking(), c => c.Id == comment.Id); - var efUnread = await EntityFrameworkQueryableExtensions.SingleAsync( - ctx.Set().AsNoTracking(), u => u.CommentId == comment.Id); + await using var ctx = await NewContext(); + await AssertColumnsAreUtc(ctx, t => t.Id == thread.Id, before, after, t => t.CreatedAt, t => t.UpdatedAt); + await AssertColumnsAreUtc(ctx, c => c.Id == comment.Id, before, after, c => c.CreatedAt, c => c.UpdatedAt); + await AssertColumnsAreUtc(ctx, u => u.CommentId == comment.Id, before, after, u => u.MarkedUnreadAt); + } - var fields = new (string Name, DateTimeOffset Ef, DateTimeOffset Linq2db)[] - { - ("CommentThread.CreatedAt", efThread.CreatedAt, - await Linq2db(ctx.Set().Where(t => t.Id == thread.Id).Select(t => t.CreatedAt))), - ("CommentThread.UpdatedAt", efThread.UpdatedAt, - await Linq2db(ctx.Set().Where(t => t.Id == thread.Id).Select(t => t.UpdatedAt))), - ("UserComment.CreatedAt", efComment.CreatedAt, - await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.CreatedAt))), - ("UserComment.UpdatedAt", efComment.UpdatedAt, - await Linq2db(ctx.Set().Where(c => c.Id == comment.Id).Select(c => c.UpdatedAt))), - ("UnreadComment.MarkedUnreadAt", efUnread.MarkedUnreadAt, - await Linq2db(ctx.Set().Where(u => u.CommentId == comment.Id).Select(u => u.MarkedUnreadAt))), - }; - - foreach (var (name, ef, l2db) in fields) + [Fact] + public async Task DeletedAt_RoundTripsInUtc_ThroughEfCoreAndLinq2db() + { + var entryId = Guid.NewGuid(); + await DataModel.AddChange(ClientId, new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "bye" } })); + var deletedAtUtc = (await DataModel.AddChange(ClientId, new DeleteChange(entryId))).HybridDateTime.DateTime.UtcDateTime; + + await using var ctx = await NewContext(); + // Find the deleted snapshot via the EntityIsDeleted column; DeletedAt itself is in its entity JSON. + var deletedSnapshot = ctx.Set().AsNoTracking().Where(s => s.EntityId == entryId && s.EntityIsDeleted); + var ef = (await EntityFrameworkQueryableExtensions.SingleAsync(deletedSnapshot)).Entity.DeletedAt!.Value; + var l2db = (await deletedSnapshot.ToLinqToDB().FirstAsyncLinqToDB()).Entity.DeletedAt!.Value; + + output.WriteLine($"DeletedAt: EF {ef:o} | linq2db {l2db:o}"); + ef.UtcDateTime.Should().Be(deletedAtUtc); + l2db.UtcDateTime.Should().Be(deletedAtUtc); + ef.Offset.Should().Be(TimeSpan.Zero); + l2db.Offset.Should().Be(TimeSpan.Zero); + } + + // Asserts a column reads as the same UTC instant (offset zero) via EF and linq2db, within the write window. + private async Task AssertColumnsAreUtc(LcmCrdtDbContext ctx, Expression> row, + DateTimeOffset before, DateTimeOffset after, params Expression>[] columns) where T : class + { + foreach (var column in columns) { + var name = $"{typeof(T).Name}.{((MemberExpression)column.Body).Member.Name}"; + var rows = ctx.Set().AsNoTracking().Where(row); + var ef = await EntityFrameworkQueryableExtensions.SingleAsync(rows.Select(column)); + var l2db = await rows.ToLinqToDB().Select(column).FirstAsyncLinqToDB(); + output.WriteLine($"{name}: EF {ef:o} | linq2db {l2db:o}"); - l2db.UtcDateTime.Should().Be(ef.UtcDateTime, $"{name}: linq2db and EF must report the same instant"); - ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, - $"{name}: must round-trip the instant it was written"); - ef.Offset.Should().Be(TimeSpan.Zero, $"{name}: EF should normalize to UTC"); - l2db.Offset.Should().Be(TimeSpan.Zero, $"{name}: linq2db should normalize to UTC"); + l2db.UtcDateTime.Should().Be(ef.UtcDateTime, $"{name}: EF and linq2db must agree"); + ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, $"{name}: round-trips the written instant"); + ef.Offset.Should().Be(TimeSpan.Zero, $"{name}: EF should be UTC"); + l2db.Offset.Should().Be(TimeSpan.Zero, $"{name}: linq2db should be UTC"); } } - // These bugs are unobservable at offset zero, so a UTC run proves nothing. CI sets a non-UTC - // zone (fw-lite.yaml); fail loudly here rather than pass vacuously if the run is on UTC. private static void RequireNonUtc() => TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow).Should().NotBe(TimeSpan.Zero, "this test only observes timezone bugs under a non-UTC zone — CI sets one in fw-lite.yaml; set your local timezone to run it"); - - private static Task Linq2db(IQueryable query) => - query.ToLinqToDB().FirstAsyncLinqToDB(); } From ad982a5c7aa983551e9fe425a9a22eb2e3bc3b1f Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 30 Jun 2026 16:10:16 +0200 Subject: [PATCH 4/9] Trim parity test to one representative plain column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All plain DateTimeOffset columns share one global converter, so testing many proves nothing extra. Drop the generic helper and MarkedUnreadAt; keep one comment timestamp, the commit timestamp, and DeletedAt — the three distinct mechanisms. Co-Authored-By: Claude Opus 4.8 --- .../DateTimeOffsetOrmParityTests.cs | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index a019a535d2..c26996c6a0 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -1,6 +1,4 @@ -using System.Linq.Expressions; using LcmCrdt.Changes; -using LcmCrdt.Data; using LinqToDB; using LinqToDB.Async; using LinqToDB.EntityFrameworkCore; @@ -60,22 +58,28 @@ public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() } [Fact] - public async Task PlainColumnTimestamps_SameUtcInstant_ThroughEfCoreAndLinq2db() + public async Task CommentTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() { RequireNonUtc(); + // One plain column is representative: every DateTimeOffset column shares the same global converter. var before = DateTimeOffset.UtcNow; var thread = await _fixture.Api.CreateCommentThread( new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid() }, new UserComment { Id = Guid.NewGuid(), Text = "hi" }); - var comment = (await _fixture.Api.GetUserComments(thread.Id).ToArrayAsync()).Single(); - await _fixture.GetService().MarkCommentsUnread([(comment.Id, thread.Id)]); var after = DateTimeOffset.UtcNow; await using var ctx = await NewContext(); - await AssertColumnsAreUtc(ctx, t => t.Id == thread.Id, before, after, t => t.CreatedAt, t => t.UpdatedAt); - await AssertColumnsAreUtc(ctx, c => c.Id == comment.Id, before, after, c => c.CreatedAt, c => c.UpdatedAt); - await AssertColumnsAreUtc(ctx, u => u.CommentId == comment.Id, before, after, u => u.MarkedUnreadAt); + var ef = (await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), t => t.Id == thread.Id)).CreatedAt; + var l2db = await ctx.Set().Where(t => t.Id == thread.Id) + .ToLinqToDB().Select(t => t.CreatedAt).FirstAsyncLinqToDB(); + + output.WriteLine($"CommentThread.CreatedAt: EF {ef:o} | linq2db {l2db:o}"); + l2db.UtcDateTime.Should().Be(ef.UtcDateTime, "EF and linq2db must agree"); + ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, "round-trips the written instant"); + ef.Offset.Should().Be(TimeSpan.Zero, "EF should be UTC"); + l2db.Offset.Should().Be(TimeSpan.Zero, "linq2db should be UTC"); } [Fact] @@ -98,25 +102,6 @@ public async Task DeletedAt_RoundTripsInUtc_ThroughEfCoreAndLinq2db() l2db.Offset.Should().Be(TimeSpan.Zero); } - // Asserts a column reads as the same UTC instant (offset zero) via EF and linq2db, within the write window. - private async Task AssertColumnsAreUtc(LcmCrdtDbContext ctx, Expression> row, - DateTimeOffset before, DateTimeOffset after, params Expression>[] columns) where T : class - { - foreach (var column in columns) - { - var name = $"{typeof(T).Name}.{((MemberExpression)column.Body).Member.Name}"; - var rows = ctx.Set().AsNoTracking().Where(row); - var ef = await EntityFrameworkQueryableExtensions.SingleAsync(rows.Select(column)); - var l2db = await rows.ToLinqToDB().Select(column).FirstAsyncLinqToDB(); - - output.WriteLine($"{name}: EF {ef:o} | linq2db {l2db:o}"); - l2db.UtcDateTime.Should().Be(ef.UtcDateTime, $"{name}: EF and linq2db must agree"); - ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, $"{name}: round-trips the written instant"); - ef.Offset.Should().Be(TimeSpan.Zero, $"{name}: EF should be UTC"); - l2db.Offset.Should().Be(TimeSpan.Zero, $"{name}: linq2db should be UTC"); - } - } - private static void RequireNonUtc() => TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow).Should().NotBe(TimeSpan.Zero, "this test only observes timezone bugs under a non-UTC zone — CI sets one in fw-lite.yaml; set your local timezone to run it"); From 64e8450e469b5a9c9735c5ce3aede00124e7d839 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 30 Jun 2026 20:03:52 +0200 Subject: [PATCH 5/9] Tighten comments per review Explain the conditional AddMappingSchema (UseLinqToDbCrdt already registered the schema); drop the duplicated DeletedAt note from the test header and state it once at the call site, including why that test isn't gated on a non-UTC zone. Co-Authored-By: Claude Opus 4.8 --- backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs | 5 +++-- backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index c26996c6a0..17d77a44aa 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -12,7 +12,7 @@ namespace LcmCrdt.Tests; // Every persisted DateTimeOffset must round-trip to the same UTC instant through BOTH EF Core and // linq2db. A broken converter shifts the instant by exactly the local UTC offset, so it is invisible // on UTC (why #2092 shipped); CI runs a non-UTC zone (fw-lite.yaml) and RequireNonUtc() fails loudly -// if that regresses. DeletedAt is null in projected columns, so it's read from the snapshot entity JSON. +// if that regresses. public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable { private MiniLcmApiFixture _fixture = null!; @@ -90,7 +90,8 @@ public async Task DeletedAt_RoundTripsInUtc_ThroughEfCoreAndLinq2db() var deletedAtUtc = (await DataModel.AddChange(ClientId, new DeleteChange(entryId))).HybridDateTime.DateTime.UtcDateTime; await using var ctx = await NewContext(); - // Find the deleted snapshot via the EntityIsDeleted column; DeletedAt itself is in its entity JSON. + // No RequireNonUtc: DeletedAt lives in the snapshot's entity JSON (offset-preserving, so correct on + // any zone). Find the deleted snapshot via the EntityIsDeleted column. var deletedSnapshot = ctx.Set().AsNoTracking().Where(s => s.EntityId == entryId && s.EntityIsDeleted); var ef = (await EntityFrameworkQueryableExtensions.SingleAsync(deletedSnapshot)).Entity.DeletedAt!.Value; var l2db = (await deletedSnapshot.ToLinqToDB().FirstAsyncLinqToDB()).Entity.DeletedAt!.Value; diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 882a51a1e1..23cc0d0e91 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -131,8 +131,9 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .UseLinqToDB(optionsBuilder => { // Reuse Harmony's existing mapping schema; a fresh one shadows it and drops Harmony's - // Commit.HybridDateTime.DateTime UTC conversion, making linq2db read commit timestamps - // in local time (issue #2092). Extending it keeps the conversion — no manual fixup needed. + // Commit.HybridDateTime.DateTime UTC conversion, making linq2db read commit timestamps in + // local time (issue #2092). UseLinqToDbCrdt already registered this schema, so only a + // freshly-created fallback needs adding. var mappingSchema = optionsBuilder.DbContextOptions.GetLinqToDBOptions()?.ConnectionOptions.MappingSchema; var isNewSchema = mappingSchema is null; mappingSchema ??= new MappingSchema(); From a7aba069f57c94c697ca2e700bead08242f1657d Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Wed, 1 Jul 2026 10:02:44 +0200 Subject: [PATCH 6/9] Address review feedback on date-parity test - Move RequireNonUtc() into InitializeAsync so it guards every test in the class (per hahn-kev); DeletedAt is now gated too, fine since it still round-trips correctly under a non-UTC zone. - Add an explicit linq2db read of the commit timestamp so the linq2db path stays covered even if HistoryService ever moves to EF. - Set the comment thread CreatedAt to a known non-UTC-offset instant and assert it exactly, testing that the write is recorded (not just the round-tripped return). Co-Authored-By: Claude Opus 4.8 --- .../DateTimeOffsetOrmParityTests.cs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index 17d77a44aa..4d224eaef9 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -11,8 +11,8 @@ namespace LcmCrdt.Tests; // Every persisted DateTimeOffset must round-trip to the same UTC instant through BOTH EF Core and // linq2db. A broken converter shifts the instant by exactly the local UTC offset, so it is invisible -// on UTC (why #2092 shipped); CI runs a non-UTC zone (fw-lite.yaml) and RequireNonUtc() fails loudly -// if that regresses. +// on UTC (why #2092 shipped); CI runs a non-UTC zone (fw-lite.yaml) and RequireNonUtc() (in setup) +// fails loudly if that regresses. public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable { private MiniLcmApiFixture _fixture = null!; @@ -24,6 +24,7 @@ private Task NewContext() => public async Task InitializeAsync() { + RequireNonUtc(); _fixture = MiniLcmApiFixture.Create(); _fixture.LogTo(output); await _fixture.InitializeAsync(); @@ -35,8 +36,6 @@ public async Task InitializeAsync() [Fact] public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() { - RequireNonUtc(); - var entryId = Guid.NewGuid(); var commit = await DataModel.AddChange(ClientId, new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); @@ -45,11 +44,15 @@ public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() await using var ctx = await NewContext(); var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; + // Read the column directly through linq2db too: HistoryService uses linq2db today, but if it ever + // switches to EF this keeps the linq2db path (the one that needed the fix) under test. + var l2dbTimestamp = await ctx.Set().Where(c => c.Id == commit.Id) + .ToLinqToDB().Select(c => c.HybridDateTime.DateTime).FirstAsyncLinqToDB(); var activityTimestamp = (await History.ProjectActivity(0, 1000).ToArrayAsync()).First(a => a.CommitId == commit.Id).Timestamp; var historyTimestamp = (await History.GetHistory(entryId).ToArrayAsync()).First(h => h.CommitId == commit.Id).Timestamp; foreach (var (name, value) in new[] - { ("EF Core", efTimestamp), ("ProjectActivity", activityTimestamp), ("GetHistory", historyTimestamp) }) + { ("EF Core", efTimestamp), ("linq2db", l2dbTimestamp), ("ProjectActivity", activityTimestamp), ("GetHistory", historyTimestamp) }) { output.WriteLine($"{name}: {value:o}"); value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); @@ -60,14 +63,13 @@ public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() [Fact] public async Task CommentTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() { - RequireNonUtc(); - // One plain column is representative: every DateTimeOffset column shares the same global converter. - var before = DateTimeOffset.UtcNow; + // Set an explicit non-UTC-offset instant so we also verify the write records the value (the API + // return is already round-tripped through the DB) and that it's normalized to UTC. + var created = new DateTimeOffset(2024, 3, 15, 10, 30, 0, TimeSpan.FromHours(5)); var thread = await _fixture.Api.CreateCommentThread( - new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid() }, + new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid(), CreatedAt = created }, new UserComment { Id = Guid.NewGuid(), Text = "hi" }); - var after = DateTimeOffset.UtcNow; await using var ctx = await NewContext(); var ef = (await EntityFrameworkQueryableExtensions.SingleAsync( @@ -77,7 +79,7 @@ public async Task CommentTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() output.WriteLine($"CommentThread.CreatedAt: EF {ef:o} | linq2db {l2db:o}"); l2db.UtcDateTime.Should().Be(ef.UtcDateTime, "EF and linq2db must agree"); - ef.UtcDateTime.Should().BeOnOrAfter(before.UtcDateTime).And.BeOnOrBefore(after.UtcDateTime, "round-trips the written instant"); + ef.UtcDateTime.Should().Be(created.UtcDateTime, "the written instant must be recorded exactly"); ef.Offset.Should().Be(TimeSpan.Zero, "EF should be UTC"); l2db.Offset.Should().Be(TimeSpan.Zero, "linq2db should be UTC"); } @@ -90,8 +92,7 @@ public async Task DeletedAt_RoundTripsInUtc_ThroughEfCoreAndLinq2db() var deletedAtUtc = (await DataModel.AddChange(ClientId, new DeleteChange(entryId))).HybridDateTime.DateTime.UtcDateTime; await using var ctx = await NewContext(); - // No RequireNonUtc: DeletedAt lives in the snapshot's entity JSON (offset-preserving, so correct on - // any zone). Find the deleted snapshot via the EntityIsDeleted column. + // DeletedAt lives in the snapshot's entity JSON; find the deleted snapshot via the EntityIsDeleted column. var deletedSnapshot = ctx.Set().AsNoTracking().Where(s => s.EntityId == entryId && s.EntityIsDeleted); var ef = (await EntityFrameworkQueryableExtensions.SingleAsync(deletedSnapshot)).Entity.DeletedAt!.Value; var l2db = (await deletedSnapshot.ToLinqToDB().FirstAsyncLinqToDB()).Entity.DeletedAt!.Value; From 1d8ffe901c56b21fdb6033fd1f370f1be920211f Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Thu, 2 Jul 2026 10:04:38 +0200 Subject: [PATCH 7/9] Throw when Harmony's linq2db mapping schema is missing Falling back to a fresh MappingSchema silently dropped Harmony's Commit UTC conversion, reintroducing local-time timestamps (#2092). Fail loudly. Co-Authored-By: Claude Opus 4.8 --- backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 23cc0d0e91..fafb129290 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -130,13 +130,13 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .UseLinqToDbCrdt(provider) .UseLinqToDB(optionsBuilder => { - // Reuse Harmony's existing mapping schema; a fresh one shadows it and drops Harmony's - // Commit.HybridDateTime.DateTime UTC conversion, making linq2db read commit timestamps in - // local time (issue #2092). UseLinqToDbCrdt already registered this schema, so only a - // freshly-created fallback needs adding. - var mappingSchema = optionsBuilder.DbContextOptions.GetLinqToDBOptions()?.ConnectionOptions.MappingSchema; - var isNewSchema = mappingSchema is null; - mappingSchema ??= new MappingSchema(); + // Extend the mapping schema UseLinqToDbCrdt (above) registered: it configures Harmony's + // Commit.HybridDateTime.DateTime UTC conversion there, and a fresh schema would shadow it, + // making linq2db read commit timestamps in local time (issue #2092). A null schema means + // that invariant broke, so fail loudly rather than silently regressing. + var mappingSchema = optionsBuilder.DbContextOptions.GetLinqToDBOptions()?.ConnectionOptions.MappingSchema + ?? throw new InvalidOperationException( + "linq2db mapping schema was not registered by UseLinqToDbCrdt; Harmony's Commit UTC conversion would be missing (issue #2092)."); new FluentMappingBuilder(mappingSchema) //tells linq2db to rewrite Sense.SemanticDomainRows / Entry.PublishInRows into //Json.Query(). The rewrite lives on the *Rows shadow accessors @@ -154,7 +154,6 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .Build(); mappingSchema.SetConvertExpression((WritingSystemId id) => new DataParameter { Value = id.Code, DataType = DataType.Text }); - if (isNewSchema) optionsBuilder.AddMappingSchema(mappingSchema); optionsBuilder.AddCustomOptions(options => options.UseSQLite()); // Register read-relevant interceptors for LinqToDB From 37a7d3de4ad518054954bf9bbcbff88636f39daa Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 7 Jul 2026 10:08:28 +0200 Subject: [PATCH 8/9] Delete commits after a snapshot target via EF, not linq2db MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit linq2db renders timestamp parameters truncated to milliseconds while SQLite's strftime (which linq2db wraps around every timestamp comparison) rounds to the nearest millisecond, so WhereAfter via ToLinqToDB() classified targetCommit as after itself and deleted it — the PreserveAllFieldWorksCommits CI failure (~25% of runs, timezone-independent; reproducible on this branch, not on develop, because reusing Harmony's mapping schema changed how the parameter renders). sillsdev/harmony#78 fixes the parameter rendering, but linq2db comparisons stay millisecond-grained, so exact commit ordering belongs in EF regardless. Co-Authored-By: Claude Fable 5 --- backend/FwLite/AGENTS.md | 4 ++++ .../FwLite/LcmCrdt/SnapshotAtCommitService.cs | 21 ++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/backend/FwLite/AGENTS.md b/backend/FwLite/AGENTS.md index 4b97232e8d..b935a7c511 100644 --- a/backend/FwLite/AGENTS.md +++ b/backend/FwLite/AGENTS.md @@ -247,6 +247,10 @@ if (entity?.DeletedAt is not null) return; --- +## 🚨 linq2db and timestamps + +linq2db wraps every SQLite timestamp comparison in `strftime('...%f', ...)` — millisecond precision, not configurable. Never filter or compare commit timestamps through `ToLinqToDB()`; use EF for those predicates (full-precision TEXT comparison, like Harmony's own `CrdtRepository`). `OrderBy` on a timestamp column is safe. See `SnapshotAtCommitService.DeleteCommitsAfter`. + ## 🚨 Harmony Projected Tables (`LcmCrdtDbContext`) `LcmCrdtDbContext` DbSets (`Entries`, `Senses`, etc.) are Harmony's **projected snapshot tables**. They contain only the latest, **un-deleted** state. diff --git a/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs b/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs index 6130acfa79..ddf2ec2f47 100644 --- a/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs +++ b/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs @@ -91,16 +91,23 @@ private async Task ForkDatabase(string sourcePath, string destPath) private async Task DeleteCommitsAfter(ICrdtDbContext context, Commit targetCommit, bool preserveAllFieldWorksCommits) { - var commitsToDelete = context.Commits.WhereAfter(targetCommit); + // WhereAfter must run through EF (like Harmony's own CrdtRepository): linq2db wraps SQLite timestamp + // comparisons in strftime, which is millisecond-grained, so it cannot order commits exactly. + // Only the AuthorName lookup needs linq2db (Json.Value has no EF translation), and it filters on + // commit ids, not timestamps. + var idsAfter = await context.Commits.WhereAfter(targetCommit).Select(c => c.Id).ToArrayAsync(); if (preserveAllFieldWorksCommits) { - commitsToDelete = commitsToDelete.ToLinqToDB().Where(c => - // JSON Sqlite gotcha: null != "FieldWorks" == false (apparently) - (Json.Value(c.Metadata, m => m.AuthorName) ?? "") != "FieldWorks"); + var fieldWorksIds = await context.Commits.ToLinqToDB() + .Where(c => idsAfter.Contains(c.Id) && + // JSON Sqlite gotcha: null != "FieldWorks" == false (apparently) + (Json.Value(c.Metadata, m => m.AuthorName) ?? "") == "FieldWorks") + .Select(c => c.Id) + .ToArrayAsyncLinqToDB(); + idsAfter = idsAfter.Except(fieldWorksIds).ToArray(); } - var count = await commitsToDelete.CountAsyncLinqToDB(); - context.Set().RemoveRange(commitsToDelete); + context.Set().RemoveRange(context.Set().Where(c => idsAfter.Contains(c.Id))); await context.SaveChangesAsync(); - return count; + return idsAfter.Length; } } From d82cc33bc0cd103bf441aaa6a90ffc4735352b59 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 7 Jul 2026 10:08:32 +0200 Subject: [PATCH 9/9] Set a last user in the comment timestamp parity test CreateCommentThread requires a known user identity since Comments (#2382). Co-Authored-By: Claude Fable 5 --- backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs index 4d224eaef9..5b35773e58 100644 --- a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -66,6 +66,8 @@ public async Task CommentTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() // One plain column is representative: every DateTimeOffset column shares the same global converter. // Set an explicit non-UTC-offset instant so we also verify the write records the value (the API // return is already round-tripped through the DB) and that it's normalized to UTC. + await _fixture.GetService().UpdateLastUser("tester", Guid.NewGuid().ToString()); + var created = new DateTimeOffset(2024, 3, 15, 10, 30, 0, TimeSpan.FromHours(5)); var thread = await _fixture.Api.CreateCommentThread( new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid(), CreatedAt = created },