Store trip day weather instead of fetching it on every view

[ericgriffin]

Problem

A trip story day that logs no dives showed a weather badge only if the app fetched one from the network, and it fetched on every view. surfaceDayWeatherProvider cached in memory for the provider container's lifetime, but that cache died with the process, so every cold open of a trip re-hit the Open-Meteo archive for weather that cannot change, because it is historical.

Two gaps followed from the same design:

• Only surface days fetched at all. TripStoryDay.isSurface is false whenever a day has an itinerary row, so travel days, port days, and sea days showed no weather even though the trip records where the diver was.
• Nothing was durable. The result never reached a backup, another device, or an export.

What this does

Weather for a trip day is now fetched at most once, stored as synced trip data, and read from the database on every later view. Coverage widens from surface days to every trip day whose dives supply no weather of their own.

Storage

A new synced table trip_day_weather holds one row per (trip, date), with a unique index on that pair so the day is the identity: two devices that both fetch the same day converge on one row.

The row id is not a v4 uuid. It is a deterministic UUIDv5 over (tripId, day), following the same convention as importedDiveComputerId and qualityFindingId, and the day is keyed at UTC midnight so it does not move with the device's timezone. Both are load-bearing for convergence and are covered in the follow-up PRs below.

It gets its own table rather than columns on trips or trip_itinerary_days for two reasons:

1. Row-level conflict resolution. HLC conflicts resolve per row. Parking an automatic, derived write on a row the diver also edits by hand would let a weather write race a trip rename or an itinerary note edit and lose it. Issue Dive sites / Create a new dive site or edit an existing one – Location based on coordinates #1187 is the standing example of partial-entity writes wiping fields.
2. Surface days have no itinerary row. Materializing one to hold weather would stop the day being a surface day, since TripStoryDay.hasContent counts any itinerary row as content, and would surface a weather-only row in the itinerary tab under a forced dayType.

The full WeatherData payload is stored, not just the three fields the header renders. The API returns them in one response at no extra cost, dive rows already store exactly this set, and widening the table later is a six-place change on a collision-prone version ladder.

Fetch policy

A day is fetched only when all of these hold: no dive on that day supplies weather, the day is not in the future, no row is stored, and the story's map geometry yields a coordinate. Requests run sequentially so a two-week trip does not open with a burst of parallel calls, and rows land progressively.

A miss writes nothing and is retried on the next view. This mirrors the rule already stated on ReefDataCache and BathymetryCache, and it is what makes the design correct against the Open-Meteo archive's few-day publication lag: a day fetched too early simply has no row yet.

Notes for review

Two things worth a look:

hasRenderableWeather deliberately excludes Precipitation.none. WeatherMapper.mapPrecipitation returns non-null always, folding a missing reading into none, even though WeatherData.precipitation is declared nullable. Counting none as data meant a fetch that resolved nothing still stored a row; the header draws nothing for it (weatherIconFor gives none no glyph), so the day would have gone permanently blank with its retry suppressed forever. Pinned by trip_day_weather_test.dart.

The backfill provider is marked // no-tick:. It must not subscribe to the weather tick: it renders nothing (its value is void), and every row it writes would invalidate the pass that wrote them. Its rows reach the UI through tripDayWeatherProvider, which does subscribe.

Schema

v171. Renumbered from 168 as parallel branches landed their own rungs while this PR was open; the ladder is non-contiguous by design, so the audit asserts monotonic, unique, and scalar equals max, never contiguous. Derived by scanning open PR diffs for the scalar rather than grepping main, because two branches writing the same number auto-merge with no conflict marker. minimumCompatibleSchemaVersion stays at 160, since a new table is additive.

Sync

tripDayWeather is registered end to end: hlc target, serializer payload and switch arms, merge order, updatedAt flag, and the trips FK parent so a changeset never imports weather ahead of its trip. Deleting a trip takes its weather rows with it, logged for sync like every other child record.

The FK completeness guard needed the table too; without it, that guard would have silently skipped the new table and a peer's weather row for a locally deleted trip would dangle its FK and fail the whole sync at COMMIT.

Testing

Tests were written first throughout. New: 6 migration tests, 10 repository tests, 5 sync tests, 10 backfill rule tests, 9 entity tests, 5 provider tests, plus rewritten header and story-view tests. surface_day_weather_provider.dart and its test are deleted.

flutter analyze is clean. The full suite was run; the only failures were known cross-test flakes in unrelated areas (media_item_view_provenance_test, security_settings_page_test), each verified passing when run alone.

Not included

• Range batching. The archive endpoint accepts a date range, so days sharing a coordinate could collapse into one request. Every day is fetched exactly once ever, so this is an optimization rather than a fix.
• A manual refresh action. Nothing in the UI re-fetches a stored day.

[Copilot]

Pull request overview

This PR makes trip-story day weather durable by storing historical weather per (trip, date) in a new synced trip_day_weather table, and rendering badges from stored data instead of fetching from the network on each view. It also broadens coverage from surface days to any trip day whose dives do not provide renderable weather.

Changes:

• Add synced trip_day_weather storage (schema v168), repository, and sync registration for cross-device/backup durability.
• Add a backfill/provider flow that fetches missing historical weather sequentially and persists only renderable results.
• Update trip story UI and tests so the day header is render-only (no fetching), reading stored weather from the DB and removing surfaceDayWeatherProvider.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
lib/core/database/database.dart	Adds TripDayWeather Drift table, v168 migration ladder entry, and idempotent schema assertion + unique index.
lib/features/trips/domain/entities/trip_day_weather.dart	Introduces domain entity for stored trip-day weather with renderability logic and mapping to header view model.
lib/features/trips/data/repositories/trip_day_weather_repository.dart	Adds DB repository for per-trip weather reads, upserts, deletes, and sync pending/deletion logging.
lib/features/trips/domain/services/trip_day_weather_backfill.dart	Adds pure backfill target selection rules based on story + stored rows.
lib/features/trips/presentation/providers/trip_day_weather_providers.dart	Adds providers for reading stored rows and running backfill fetch loop.
lib/features/trips/presentation/widgets/story/trip_story_view.dart	Watches stored weather once per trip and triggers backfill; threads stored rows into day headers.
lib/features/trips/presentation/widgets/story/trip_story_day_header.dart	Removes fetch behavior; renders only the provided (dive-logged or stored) weather.
lib/features/trips/presentation/providers/surface_day_weather_provider.dart	Deleted (replaced by stored weather + backfill).
lib/features/trips/data/repositories/trip_repository.dart	Ensures trip deletion cascades to stored trip-day weather rows.
lib/core/data/repositories/sync_repository.dart	Registers tripDayWeather as an HLC target.
lib/core/services/sync/sync_service.dart	Adds tripDayWeather to merge order, updatedAt map, and FK parent completeness map.
lib/core/services/sync/sync_data_serializer.dart	Adds tripDayWeather to sync payload, export/import switch arms, and delta export.
test/core/database/migration_v168_trip_day_weather_test.dart	Adds migration + schema assertions for v168 and unique-per-(trip,date) behavior.
test/features/trips/data/repositories/trip_day_weather_repository_test.dart	Adds repository tests for round-trip persistence, scoping, and cascade delete.
test/features/trips/domain/entities/trip_day_weather_test.dart	Adds tests for hasRenderableWeather, toStoryWeather, and copyWith semantics.
test/features/trips/domain/services/trip_day_weather_backfill_test.dart	Adds tests for backfill targeting rules (skip conditions, normalization, ordering).
test/features/trips/presentation/providers/trip_day_weather_providers_test.dart	Adds provider tests for sequential fetch, skip rules, and “don’t store unrenderable” behavior.
test/features/trips/presentation/providers/surface_day_weather_provider_test.dart	Deleted (provider removed).
test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart	Updates widget tests to pass stored weather directly and assert precedence vs dive-logged weather.
test/features/trips/presentation/widgets/story/trip_story_view_test.dart	Updates story view tests to override trip-day weather providers and verify no render-time fetch.
test/core/services/sync/trip_day_weather_sync_test.dart	Adds sync serializer round-trip and delta-export tests for tripDayWeather.
test/core/services/sync/sync_parent_refs_completeness_test.dart	Adds parent-ref completeness coverage for trip_day_weather.
test/helpers/mock_providers.dart	Adds base overrides to keep trip-day weather inert by default in widget tests.
docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md	Design spec documenting the motivation, schema, sync, and fetch policy.
docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md	Implementation plan documenting step-by-step work and constraints.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

lib/features/trips/presentation/widgets/story/trip_story_view.dart:254

• This duplicates the "normalize day to local midnight millis" logic even though trip_day_weather.dart already provides tripDayMillis() for consistency across callers. Using the shared helper reduces the risk of key drift if the normalization rule ever changes and keeps the lookup aligned with the repository/storage key.

    // Stored rows are keyed on local midnight millis.
    final dayDate = DateTime(day.date.year, day.date.month, day.date.day);
    final stored = storedWeather[dayDate.millisecondsSinceEpoch];

test/features/trips/data/repositories/trip_day_weather_repository_test.dart:139

• This test uses a nested immediately-invoked async closure (return () async { ... }();), which is inconsistent with the surrounding tests and makes failures harder to read/diagnose in stack traces. The test body can be declared async directly.

      return () async {
        await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30)));

        final stored = await repository.getForTrip(testTripId);

[codecov]

Codecov Report

❌ Patch coverage is 86.55172% with 39 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...atures/trips/domain/entities/trip_day_weather.dart	66.07%	19 Missing ⚠️
lib/core/services/sync/sync_data_serializer.dart	76.92%	9 Missing ⚠️
...data/repositories/trip_day_weather_repository.dart	95.37%	5 Missing ⚠️
...ips/domain/services/trip_day_weather_backfill.dart	83.33%	3 Missing ⚠️
lib/core/database/database.dart	80.00%	2 Missing ⚠️
...entation/providers/trip_day_weather_providers.dart	97.36%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

📦 Build artifacts for this PR · commit fc2ede8

Platform	Download
Android (APK)	android-apk
macOS	macos-build
Windows	windows-build
Linux	linux-build

Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the submersion-macos.zip inside it to get a runnable submersion.app. The build is ad-hoc signed — right-click → Open on first launch.

_{Updated automatically on each push.}

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.

Suppressed comments (3)

lib/features/trips/domain/entities/trip_day_weather.dart:18

• tripDayMillis derives the day identity from local-midnight epoch millis. That makes the (trip, day) key (and thus the UUIDv5 row id and unique (trip_id, date) constraint) vary by device timezone, which breaks cross-device convergence and contradicts the PR description’s UTC-midnight requirement.

/// Local midnight for [date], as epoch milliseconds.
///
/// The day is the identity of a weather row, so this is what turns a
/// DateTime into one. Shared rather than reimplemented per caller: the row
/// id, the repository's storage key, and the map key reads come back under
/// all have to agree, and three copies of the same two lines would drift.
int tripDayMillis(DateTime date) =>
    DateTime(date.year, date.month, date.day).millisecondsSinceEpoch;

lib/features/trips/data/repositories/trip_day_weather_repository.dart:218

• Same issue as getForTrip: _rowsForDay converts r.date using the local timezone. That can make a UTC-midnight day fall on the previous/next local date and prevent same-day stray detection.

    return rows
        .where(
          (r) =>
              _dayKey(DateTime.fromMillisecondsSinceEpoch(r.date)) == dayMillis,
        )
        .toList();

lib/features/trips/data/repositories/trip_day_weather_repository.dart:260

• _mapRow normalizes the stored day using DateTime.fromMillisecondsSinceEpoch without isUtc. With a UTC-midnight day key, that can hand downstream logic a date that is off by one day in some timezones.

      date: DateTime.fromMillisecondsSinceEpoch(
        _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)),
      ),

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.

Suppressed comments (2)

lib/features/trips/domain/services/trip_day_weather_backfill.dart:63

• TripDayWeatherBackfill.targetsFor checks stored.containsKey(date.millisecondsSinceEpoch), but stored rows are keyed with tripDayMillis(...) (UTC-midnight day key) from TripDayWeatherRepository. In non-UTC timezones this will miss existing rows, causing unnecessary refetches and preventing the "already stored" skip rule from working.

      // Ask with the key the rows are actually stored under. getForTrip keys
      // by tripDayMillis, which is UTC midnight so two devices converge on one
      // row; a local DateTime's epoch agrees with that only at UTC+0. Building
      // the lookup from the local value instead made every stored day read as
      // missing on any other device and refetch on every view, which is the

lib/core/database/database.dart:157

• The trip_day_weather.date column is documented as "local midnight", but the implementation derives and stores a UTC-midnight day key (tripDayMillis) so the identity is timezone-stable across devices. This comment should be updated to avoid future code writing local-midnight millis into the same column.

  /// UTC midnight for the day, as epoch milliseconds (milliseconds being the
  /// convention TripItineraryDays.date is written with).
  ///

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

test/core/services/sync/trip_day_weather_sync_test.dart:16

• This test uses DateTime(2026, 3, 8).millisecondsSinceEpoch as the canonical trip-day key. That value depends on the machine timezone, but trip-day identity in production is UTC-midnight via tripDayMillis(...), so this test can fail (or validate the wrong behavior) when run under a non-UTC TZ.

This issue also appears in the following locations of the same file:

• line 37
• line 124

  final dayMillis = DateTime(2026, 3, 8).millisecondsSinceEpoch;
  final rowId = tripDayWeatherRowId(tripId: 'trip-1', dayMillis: dayMillis);

test/core/services/sync/trip_day_weather_sync_test.dart:42

• The inserted fixture row is keyed using DateTime(...).millisecondsSinceEpoch (local-midnight epoch). Since trip_day_weather.date is defined as a UTC-midnight day key (via tripDayMillis), the test should insert using the same dayMillis key; otherwise the row id and unique (trip_id, date) behavior being tested can vary by timezone.

            id: tripDayWeatherRowId(
              tripId: 'trip-1',
              dayMillis: DateTime(2026, 3, 8).millisecondsSinceEpoch,
            ),
            tripId: 'trip-1',
            date: DateTime(2026, 3, 8).millisecondsSinceEpoch,

test/core/services/sync/trip_day_weather_sync_test.dart:142

• This test re-derives the day key using DateTime(...).millisecondsSinceEpoch, which is timezone-dependent. For consistency with the production invariant (UTC-midnight keys), use tripDayMillis(...) for both the base day and the comparison day.

  test('the row id is derived from trip and day, not minted per device', () {
    final day = DateTime(2026, 3, 8).millisecondsSinceEpoch;

    expect(
      tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day),
      tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day),
    );
    expect(
      tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day),
      isNot(tripDayWeatherRowId(tripId: 'trip-2', dayMillis: day)),
    );
    expect(
      tripDayWeatherRowId(tripId: 'trip-1', dayMillis: day),
      isNot(
        tripDayWeatherRowId(
          tripId: 'trip-1',
          dayMillis: DateTime(2026, 3, 9).millisecondsSinceEpoch,
        ),
      ),

lib/features/trips/domain/entities/trip_day_weather.dart:80

• TripDayWeather.date is described as “Local midnight”, but the storage and lookup key for a trip day is explicitly UTC-midnight (tripDayMillis / tripDayDate), and repository reads normalize back to UTC. This docstring is likely to mislead future callers into thinking local-midnight semantics are required or preserved.

  /// Local midnight for the day this describes.
  final DateTime date;