From 3d6ea7f665d284225a7726b6608f3be145cdbc8c Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 18:21:15 -0400 Subject: [PATCH 1/2] docs(trips): state the day-key contract where callers read it Three doc comments still said the stored-weather map is keyed by date.millisecondsSinceEpoch, and the entity still called its date field local midnight. Both were true before the key moved to UTC and are now exactly the wrong thing to believe: local-midnight millis differ in every timezone, so a caller keying a lookup that way finds nothing and the day renders no badge. Nothing throws, which is what makes a stale comment here worth fixing rather than tolerating. TripDayWeatherTarget.date genuinely is local midnight and stays that way: localNoon reads its calendar fields to ask the archive for noon at the dive site. Its doc now says why that is unrelated to identity, since it sits one call away from the key derivation and is the easiest place to conflate the two. --- .../data/repositories/trip_day_weather_repository.dart | 7 ++++++- lib/features/trips/domain/entities/trip_day_weather.dart | 7 ++++++- .../trips/domain/services/trip_day_weather_backfill.dart | 8 +++++++- .../providers/trip_day_weather_providers.dart | 7 ++++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/features/trips/data/repositories/trip_day_weather_repository.dart b/lib/features/trips/data/repositories/trip_day_weather_repository.dart index 30f2bda321..6bad185fa0 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -37,7 +37,12 @@ class TripDayWeatherRepository { Stream watchWeatherChanges() => _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); - /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + /// Stored weather for a trip, keyed by `tripDayMillis(date)`: the calendar + /// day at UTC midnight, not `date.millisecondsSinceEpoch`. + /// + /// The distinction is the contract. Local-midnight millis differ in every + /// timezone, so a caller keying a lookup that way silently misses every + /// stored row rather than failing. /// /// One entry per calendar day. Where more than one row lands on the same /// day, [_preferred] picks which one shows, and explains how a second row diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index c8fb0108c1..13960456c6 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -76,7 +76,12 @@ class TripDayWeather extends Equatable { final String id; final String tripId; - /// Local midnight for the day this describes. + /// The calendar day this describes, as UTC midnight. + /// + /// UTC rather than local because the day is part of the row identity: a + /// local-midnight instant differs in every timezone, so two devices would + /// key the same trip day differently and never converge. See + /// [tripDayMillis]. final DateTime date; /// The coordinates the lookup used. diff --git a/lib/features/trips/domain/services/trip_day_weather_backfill.dart b/lib/features/trips/domain/services/trip_day_weather_backfill.dart index 5cd9a66bca..1b33b654b4 100644 --- a/lib/features/trips/domain/services/trip_day_weather_backfill.dart +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -7,7 +7,13 @@ import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; /// One trip day that needs a weather lookup, with the coordinates to look it /// up at. class TripDayWeatherTarget extends Equatable { - /// Local midnight for the day. + /// The day to look up, at local midnight. + /// + /// Local is right here and irrelevant to identity: [localNoon] reads the + /// calendar fields to ask the archive for that day's noon at the dive site, + /// and the storage key is derived separately by `tripDayMillis`, which + /// takes the same calendar fields and pins them to UTC. Nothing keys a + /// stored row off this instant. final DateTime date; final double latitude; final double longitude; diff --git a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart index 14bfaa6e11..0bd3a49dbd 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -9,7 +9,12 @@ final tripDayWeatherRepositoryProvider = Provider( (ref) => TripDayWeatherRepository(), ); -/// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. +/// Stored weather for a trip, keyed by `tripDayMillis(date)`: the calendar +/// day at UTC midnight, not `date.millisecondsSinceEpoch`. +/// +/// Stated precisely because getting it wrong is silent. A caller that keys a +/// lookup with local-midnight millis finds nothing on any device that is not +/// on UTC, and the day simply renders no badge. /// /// Subscribes to the table tick, so a row written by the backfill or arriving /// through sync re-renders the day headers without the widget knowing a fetch From f6f2636cd13b18bf7270bb77a268374e005d4889 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 21:29:51 -0400 Subject: [PATCH 2/2] docs(trips): describe the day-key derivation, not the instant The entity's date doc asserted the field is UTC midnight, which the type does not enforce and one in-tree writer does not honour: the backfill builds a row with the target's local DateTime(y, m, d), and only the repository hands back a normalized UTC instant. A comment stating an invariant the code does not hold is worse than none, because it is what a reader keys their next lookup off. It now states the contract that is actually enforced. Identity is tripDayMillis(date), which reads y/m/d and pins them to UTC midnight, and every path that stores, keys, or looks a row up derives it that way, so the differing isUtc flag is invisible to all of them. With one exception, now named: date is in props, and DateTime equality compares the epoch value and isUtc, so two entities for the same day compare unequal across provenance. That is the same silent shape as the key bug this branch fixed. A test asserting entity equality across a repository read and a freshly fetched row fails for a reason that looks nothing like a timezone problem, so the doc says to compare day keys. Comments only. flutter analyze clean, 634 trips tests pass. --- .../domain/entities/trip_day_weather.dart | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index 13960456c6..d15c45e18e 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -76,12 +76,25 @@ class TripDayWeather extends Equatable { final String id; final String tripId; - /// The calendar day this describes, as UTC midnight. + /// The calendar day this describes. /// - /// UTC rather than local because the day is part of the row identity: a - /// local-midnight instant differs in every timezone, so two devices would - /// key the same trip day differently and never converge. See + /// Read as calendar fields, not as an instant: identity is + /// `tripDayMillis(date)`, which takes y/m/d and pins them to UTC midnight. + /// UTC because the day is part of the row identity, and a local-midnight + /// instant has a different epoch value in every timezone, so two devices + /// would key the same trip day differently and never converge. See /// [tripDayMillis]. + /// + /// So `isUtc` is not enforced here, and varies by provenance: the + /// repository hands back a normalized UTC-midnight instant, while a row + /// built from a freshly fetched day carries the backfill target's local + /// `DateTime(y, m, d)`. Both name the same day and derive the same key, and + /// no path that stores, keys, or looks a row up reads anything else. + /// + /// Except `==`. DateTime compares its epoch value and `isUtc`, and this + /// field is in [props], so two entities for the same day compare unequal + /// across provenance. Compare day keys, or normalize both sides, rather + /// than the entities. final DateTime date; /// The coordinates the lookup used.