Key a trip weather day in UTC so devices in different timezones converge - #1337
Merged
Merged
Conversation
tripDayMillis built the key from a local DateTime(y, m, d), whose epoch value differs in every timezone. Two devices looking at the same trip day derived different keys, therefore different UUIDv5 row ids, and never converged: each stored and refetched its own copy. Divers cross timezones by definition, and one diver flying home is enough to trigger it. The key is now UTC midnight for the calendar day, taking the fields as given rather than converting: toUtc() would shift a late evening onto the next day, and the story hands over a date whose y/m/d is the day it means, matching the app's wall-clock-as-UTC convention for dive timestamps. Reading needed the inverse or the fix would have been half a fix. DateTime.fromMillisecondsSinceEpoch returns a LOCAL DateTime, so re-extracting y/m/d walked the day backwards on every negative offset. tripDayDate is that inverse, and the three read sites use it. The story view was computing the key inline instead of through the shared helper, so it would have looked up local midnight against UTC-keyed rows and silently rendered no badges at all. It goes through tripDayMillis now, which is what makes the lookup unable to drift from the write. Also updates the design doc, which still described the id as a v4 uuid and the date column as local midnight.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes trip-day weather row convergence across devices by making the “day key” timezone-independent: trip days are now keyed at UTC midnight for the calendar day, and the read path explicitly inverts that mapping so stored keys don’t drift when interpreted on devices in negative UTC offsets.
Changes:
- Update
tripDayMillisto use UTC midnight for day identity and addtripDayDatefor correct UTC interpretation on readback. - Switch repository and UI lookup paths to consistently key/interpret stored rows via the shared helpers (avoiding inline/local-midnight derivations).
- Update tests and design doc to reflect UTC-keyed behavior and ensure stability across times-of-day.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/features/trips/presentation/widgets/story/trip_story_view_test.dart | Uses tripDayMillis for fixture map keys to match production lookup. |
| test/features/trips/domain/entities/trip_day_weather_test.dart | Adds coverage for UTC day-keying and stable deterministic row IDs. |
| test/features/trips/data/repositories/trip_day_weather_repository_test.dart | Updates expectations to UTC-midnight storage and keying through tripDayMillis. |
| lib/features/trips/presentation/widgets/story/trip_story_view.dart | Uses shared helper for stored-weather lookup to prevent key drift. |
| lib/features/trips/domain/entities/trip_day_weather.dart | Implements UTC day keying and adds tripDayDate helper for correct readback. |
| lib/features/trips/data/repositories/trip_day_weather_repository.dart | Reads stored date values in UTC (via tripDayDate) before day-key derivation and normalization. |
| docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md | Updates design doc to describe deterministic UUIDv5 and UTC-midnight day identity. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
tripDayDate was documented as returning "a UTC DateTime at midnight". It returns the instant it is given, read in UTC, and normalizes nothing, so it is the inverse of tripDayMillis only for a value tripDayMillis produced. The repository deliberately calls it on raw stored dates that can carry a time component, so the claim was not merely loose. It now says what the function does and tells callers to run the result through tripDayMillis when they want the day rather than the instant. Three more comments still described the local-midnight scheme this branch replaced: tripDayWeatherRowId told callers the day key "must already be normalized to local midnight", which is the exact mistake the branch exists to fix. A caller who followed it would derive a per-timezone id and lose the convergence the deterministic id buys. TripDayWeatherRepository._dayKey was still headed "Local midnight for [date]" while delegating to a UTC helper. _rowsForDay justified filtering in Dart by claiming SQLite cannot derive local midnight without the zone and its DST history. That reason died with the UTC move: the day is now integer arithmetic on the stored millis. The filter stays in Dart because a trip holds a few dozen rows and one Dart function cannot drift from tripDayMillis, which is the honest reason. No behaviour change.
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the remaining review comments on #1319. Stacked on
worktree-trip-day-weather.The day key was device-local
tripDayMillisderived the day identity asDateTime(y, m, d).millisecondsSinceEpoch. That is a local DateTime, so its epoch value differs in every timezone. The calendar triple is device-independent (the story builds each day from the trip's stored start date and a day index), but the number derived from it was not.Consequences, since that number is the row identity:
tripDayWeatherRowId, so two devices derived different UUIDv5 primary keys for the same trip day and never convergeddatehalf of the unique(trip_id, date)index, so the two rows did not even collide; each device quietly stored and refetched its own copyDivers cross timezones by definition. A single diver flying home from a trip is enough to trigger this, without a second device involved.
The key is now UTC midnight for the calendar day. The fields are taken as given rather than converted:
toUtc()would shift a late evening onto the following day, and the story hands over a date whose y/m/d is the day it means. That also matches the app's existing wall-clock-as-UTC convention for dive timestamps.The read path needed the inverse
Storing in UTC is only half a fix.
DateTime.fromMillisecondsSinceEpochreturns a local DateTime, so re-extracting y/m/d from a stored UTC-midnight value read the calendar fields in the device's frame: on any negative UTC offset, UTC midnight is the previous evening locally, and the day walked backwards on every round trip.tripDayDateis the explicit inverse, and the three read sites in the repository use it.The view was computing the key itself
trip_story_view.dartbuilt the lookup key inline rather than through the shared helper. Left alone it would have looked up local midnight against UTC-keyed rows, found nothing, and silently rendered no weather badges at all. It goes throughtripDayMillisnow, so the lookup cannot drift from the write. The existing view test caught this, which is how I know it exercises the real lookup.Docs
The design doc still described
idas "uuid v4" anddateas local midnight. Both now describe what is implemented, with the reasons.The PR description on #1319 still says v168 while the ladder is at v171; that is a description edit on the parent PR, not a code change here.
Testing
tripDayMillisis the calendar day in UTC, not local midnight. This fails on any machine that is not UTC before the change (verified on UTC-4).toUtc(), which would move a late evening onto the next day.tripDayWeatherRowIdis stable for one calendar day regardless of the time of day handed in.One behaviour note: "same day" is now a UTC question, so a legacy row stored off-midnight can fall on a neighbouring UTC day and stop being reconciled as a stray. That only affects rows written before this branch derived ids, and the table is unreleased.
1,848 tests pass across trips, sync, and database.
flutter analyzeclean.