From 382fca31ddbb20d59bef056bb64d705813c0f3c4 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 15:34:08 -0400 Subject: [PATCH 01/18] docs: design for stored trip day weather Surface-day weather is fetched from Open-Meteo on every trip view and never persisted. Store it as trip data in a new synced table instead, covering every trip day whose dives supply no weather. --- ...6-08-26-trip-day-weather-storage-design.md | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md diff --git a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md new file mode 100644 index 0000000000..6991c63a2b --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md @@ -0,0 +1,221 @@ +# Stored trip day weather + +## Problem + +A trip story day that logs no dives shows a weather badge only if the app +fetches one from the network. `surfaceDayWeatherProvider` does that fetch on +every view: it is a `FutureProvider.family` that is deliberately not +auto-disposed, so it caches for the lifetime of the provider container, but +that cache dies with the process. Every cold open of a trip re-hits the +Open-Meteo archive for weather that cannot change, because it is historical. + +Two further gaps follow from the same design: + +- Only surface days fetch at all. `TripStoryDay.isSurface` is false whenever + the day has an itinerary row, so travel days, port days, and sea days show no + weather even though the trip records where the diver was. +- Nothing about the result is durable. It never reaches a backup, another + device, or an export. + +## Goal + +Weather for a trip day is fetched at most once, stored as trip data, and read +from the database on every later view. + +## Scope + +In scope: every trip day whose dives supply no weather, which includes surface +days and dive-free itinerary days. Days in the future are excluded, because a +historical archive has nothing for them. + +Out of scope: dive days. Dives already persist their own weather +(`cloudCover`, `precipitation`, `weatherCode`, `weatherSource`, +`weatherFetchedAt` on the dive row), and `TripStoryDay.weather` already +composes a day summary from them. That remains the higher-precedence source. + +## Storage decision + +The weather lives in a new synced table in the main database rather than in +`local_cache_database.dart` (where `BathymetryCache`, `ReefDataCache`, and +`NoaaTideStations` keep comparable third-party lookups), because it is trip +data: it belongs in backups, in sync, and in exports. + +It gets its own table rather than columns on `trips` or on +`trip_itinerary_days`. Two reasons: + +1. **Row-level conflict resolution.** HLC conflicts are resolved per row. A + background weather write onto the `trips` row or an itinerary row would put + a derived, automatic write in conflict with hand-entered data on the same + row; a weather write racing a trip rename or an itinerary note edit from + another device can lose that edit. Issue #1187 is the standing example of + partial-entity writes wiping fields. +2. **Surface days have no itinerary row.** Materializing one to hold weather + would make the day stop being a surface day, since + `TripStoryDay.hasContent` counts any itinerary row as content, and would + surface a weather-only row in the itinerary tab with a forced `dayType`. + +`LiveaboardDetailRecords` is the existing template for a synced child record +of a trip and is the shape to copy. + +## Data model + +New table `TripDayWeather` in `lib/core/database/database.dart`: + +| Column | Type | Notes | +| --- | --- | --- | +| `id` | text, pk | uuid v4 | +| `tripId` | text | references `Trips(#id)` | +| `date` | int | unix seconds, local midnight, same convention as `trip_itinerary_days.date` | +| `latitude` | real | the coordinate the lookup used | +| `longitude` | real | the coordinate the lookup used | +| `airTemp` | real, nullable | celsius | +| `cloudCover` | text, nullable | `CloudCover.name` | +| `precipitation` | text, nullable | `Precipitation.name` | +| `windSpeed` | real, nullable | m/s | +| `windDirection` | text, nullable | `CurrentDirection.name` | +| `humidity` | real, nullable | 0-100 | +| `surfacePressure` | real, nullable | bar | +| `weatherCode` | int, nullable | raw WMO code, so prose renders in the diver's locale at display time | +| `weatherSource` | text | defaults to `openMeteo` | +| `fetchedAt` | int | unix seconds | +| `createdAt` | int | unix seconds | +| `updatedAt` | int | unix seconds | +| `hlc` | text, nullable | matches every other synced table | + +Unique index on (`tripId`, `date`). + +The stored field set is the full `WeatherData` payload, not just the three +fields the day header renders. The API returns them in one response at no +extra cost, dive rows already store exactly this set, and a later migration to +widen the table is a six-place change on a collision-prone version ladder. +Storing them now costs nothing and renders nothing new. + +Domain entity `TripDayWeather` in +`lib/features/trips/domain/entities/trip_day_weather.dart`, with `copyWith` +per the project convention, plus a mapping to the existing +`TripStoryDayWeather` view model that the day header already consumes. + +## Schema version + +**v168.** Derived by scanning open PR diffs for the scalar, not by grepping +main: main is at v164, and v165, v166, v167 are claimed by PRs #1300, #1290, +and #1276 respectively. Re-verify at implementation time, and re-grep the +scalar after any merge from main, because two branches writing the same number +auto-merge with no conflict marker. + +The claim touches the six places the ladder requires: the +`currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the +`_assertTripDayWeatherTable()` helper docstring, the `if (from < 168)` +onUpgrade guard and its `reportProgress()` twin, the `beforeOpen` backstop +comment, and the `migration_v168_trip_day_weather_test.dart` filename with its +version assertions. The ladder is non-contiguous by design (v162 is +permanently skipped, and reserved rungs may be missing), so the migration +audit asserts monotonic, unique, and scalar equals max, never contiguous. + +`minimumCompatibleSchemaVersion` does not move: a new table is additive, and +an older build simply ignores it. + +## Fetch and write + +A new `TripDayWeatherRepository` in +`lib/features/trips/data/repositories/trip_day_weather_repository.dart`: + +- `watchForTrip(String tripId)` streams the trip's rows keyed by date. +- `getForTrip(String tripId)` reads them once. +- `upsert(TripDayWeather)` writes one row, stamping `updatedAt` and `hlc` the + way the sibling trip repositories do. +- `deleteForTrip(String tripId)` removes them, called from the trip-delete + path alongside itinerary days. + +A backfill service in +`lib/features/trips/domain/services/trip_day_weather_backfill.dart` decides +what to fetch. Given a built `TripStory` and the rows already stored, a day is +fetched only when all of these hold: + +1. `day.weather == null`, so no dive on that day supplies weather. +2. `day.kind != TripStoryDayKind.future`. +3. No stored row exists for (`tripId`, `date`). +4. `story.mapGeometry.nearestPointForDay(index)` yields a coordinate. + +Each qualifying day is fetched through the existing +`WeatherService.fetchWeather` at local noon with `useLocationTimezone: true`, +matching what `surfaceDayWeatherProvider` does today. Requests run with a +small concurrency cap rather than all at once, since widening from surface +days to all dive-free days raises the first-view request count. + +**A fetch that returns null, or returns a `WeatherData` with no usable field, +writes no row and is retried on the next view.** This mirrors the rule already +stated on `ReefDataCache` and `BathymetryCache`: transient failures write no +row. It is also what makes the design correct against the Open-Meteo archive's +few-day lag, since a day fetched too early simply has no row yet and is +retried later. The cost of that policy is that a location with genuinely no +archive data is re-requested once per trip view; that is the same behavior as +today and is bounded by how often a trip is opened. + +Range batching is deliberately not implemented. The archive endpoint accepts a +`start_date`/`end_date` range, so days sharing a coordinate could collapse +into one request (a fixed-base resort trip would go from seven requests to +one), but every day is fetched exactly once ever and then never again. Noted +as a possible follow-up, not built here. + +## Read path + +`trip_story_view.dart` watches `tripDayWeatherProvider(tripId)` once for the +whole trip and passes each day's stored weather into `TripStoryDayHeader`, +replacing the per-day `SurfaceDayWeatherRequest` it builds today. Precedence +in the header is unchanged in spirit: `day.weather ?? storedWeather`, so +dive-logged weather always wins over a fetched summary. + +The view also watches a backfill provider whose job is the side effect of +filling gaps. Because the display provider subscribes to the table's change +tick, a row written by the backfill re-renders the header without the widget +knowing a fetch occurred. + +`lib/features/trips/presentation/providers/surface_day_weather_provider.dart` +and its test are deleted. This design replaces them. + +## Sync + +The serializer works on whole rows (`row.toJson()` on export, +`Entity.fromJson(data).toCompanion(false)` on import), so registering a new +table is mechanical but touches several sites: + +- `sync_repository.dart`'s table map gains + `'tripDayWeather': (table: 'trip_day_weather', pk: 'id')`. +- `sync_data_serializer.dart` gains the payload field, its `toJson`/`fromJson` + entries, its export entry, and the roughly ten `switch` arms that every + synced collection has. +- Tombstones and the trip-deletion FK path follow whatever `itineraryDays` + does, so deleting a trip takes its weather rows with it on every device. + +## Testing + +Tests come first, per the project's TDD rule. + +- **Repository:** upsert, read back, per-trip scoping, and that deleting a + trip removes its weather rows. +- **Backfill:** one test per skip condition (dive-sourced weather present, + future day, row already stored, no coordinate available), plus the two + negative-result cases that must write no row (service returns null; service + returns an all-null `WeatherData`). +- **Migration:** `migration_v168_trip_day_weather_test.dart`, including a + stranded-database fixture at the previous `PRAGMA user_version`, plus the + existing ladder audit in `test/core/database/`. +- **Sync:** a round trip proving a row exports and re-imports intact. +- **Header widget:** a stored row renders the badge; a day whose dives carry + weather still prefers the dive-sourced summary. +- The deleted `surface_day_weather_provider_test.dart` is replaced by the + backfill and header tests above. + +## Risks + +- **Version collision.** The ladder scalar auto-merges with no conflict marker + when two branches write the same number. Re-grep after every merge from + main and re-run `test/core/database/`. +- **Request volume on first view.** Widening from surface days to all + dive-free days increases first-view requests for a trip. Bounded by the + concurrency cap, and one-time per day per trip. +- **Coordinate drift.** Stored rows record the coordinate used. If a site + later moves, the stored weather is not re-fetched. Accepted: the row records + what it was fetched for, and a day's weather is not materially sensitive to + a site correction of a few kilometers. From 2fb05e5ba4bf2b7165295923dbf4a3a103ec9abe Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 15:56:40 -0400 Subject: [PATCH 02/18] docs: implementation plan for stored trip day weather Seven tasks: schema v168 and the entity, repository plus trip-delete cascade, sync registration, pure backfill rules, providers and the sequential fetch loop, the read path with the per-view fetch deleted, and whole-project verification. Also corrects the spec: these tables store epoch milliseconds, not seconds, and the repository method names now follow ItineraryDayRepository. --- .../2026-08-26-trip-day-weather-storage.md | 2031 +++++++++++++++++ ...6-08-26-trip-day-weather-storage-design.md | 36 +- 2 files changed, 2052 insertions(+), 15 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md diff --git a/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md new file mode 100644 index 0000000000..018f4cfe69 --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md @@ -0,0 +1,2031 @@ +# Stored Trip Day Weather Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fetch weather for a trip day at most once, store it as synced trip data, and read it from the database on every later view instead of re-hitting Open-Meteo. + +**Architecture:** A new synced table `trip_day_weather` holds one row per (trip, date), written by a backfill pass that runs when the trip story is viewed and reads by a Riverpod provider subscribed to the table's change tick. The day header stops fetching entirely and becomes a pure widget that renders whatever weather it is handed, with dive-logged weather still taking precedence over a fetched summary. + +**Tech Stack:** Flutter, Drift (SQLite), Riverpod 3, Equatable, Open-Meteo archive API via the existing `WeatherService`. + +**Spec:** `docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md` + +## Global Constraints + +- **Worktree:** all work happens in `/Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather` on branch `worktree-trip-day-weather`. The Bash working directory does NOT reliably persist across turns: put `cd &&` in the same compound command as the work and echo `pwd`, or use absolute paths. A relative-path write from the wrong cwd silently edits the main checkout. +- **Schema version is 168.** Verified by scanning open PR diffs: main is at 164, and 165/166/167 are claimed by PRs #1300, #1290, #1276. Re-verify with the scan in Task 1 before writing the number. Do NOT raise `minimumCompatibleSchemaVersion` (stays 160): a new table is additive. +- **No em-dashes** (`—`, U+2014) in any output: code, comments, docs, commit messages. En-dashes and " - " as prose punctuation are equally forbidden. A hyphen inside a compound word or CLI flag is fine. +- **No emojis** in code, comments, or documentation. +- **Timestamps are epoch MILLISECONDS** in these tables (`DateTime.millisecondsSinceEpoch`), matching `ItineraryDayRepository`. The `// Unix timestamp` comment on `trip_itinerary_days.date` is misleading; the repository writes milliseconds. +- **Immutability:** never mutate objects or lists. All domain entities get `copyWith`. +- **TDD:** write the failing test first, watch it fail, then implement. +- **Formatting:** run `dart format .` before every commit. +- **Commit messages:** no `Co-Authored-By` trailer, no Claude Code attribution line, no session URL. +- **Units:** anything displaying units must respect the active diver's unit settings. Weather is stored in metric (celsius, m/s, bar) and converted at display time by `UnitFormatter`. + +--- + +### Task 1: Schema, entity, and the v168 migration + +**Files:** +- Create: `lib/features/trips/domain/entities/trip_day_weather.dart` +- Modify: `lib/core/database/database.dart` (table class near `TripItineraryDays` at line 117; `@DriftDatabase(tables: [...])` list; `currentSchemaVersion` at line 3183; `migrationVersions` list; a new `_assertTripDayWeatherSchema()` helper next to `_assertQualityFindingsSchema()` at line 3932; the `onUpgrade` ladder tail at line 8614; the `beforeOpen` backstop block around line 8823) +- Test: `test/core/database/migration_v168_trip_day_weather_test.dart` + +**Interfaces:** +- Consumes: nothing (first task). +- Produces: + - Drift table `TripDayWeather` -> generated row class `TripDayWeatherData`, accessor `_db.tripDayWeather`, companion `TripDayWeatherCompanion`. + - Domain entity `TripDayWeather` (in the `features/trips` namespace; import it `as domain` wherever the Drift row class is also in scope, per the project's import-alias convention) with fields `id`, `tripId`, `date`, `latitude`, `longitude`, `airTemp`, `cloudCover`, `precipitation`, `windSpeed`, `windDirection`, `humidity`, `surfacePressure`, `weatherCode`, `weatherSource`, `fetchedAt`, `createdAt`, `updatedAt`; `copyWith`; and `TripStoryDayWeather toStoryWeather()`. + - `AppDatabase.currentSchemaVersion == 168`. + +- [ ] **Step 1: Re-verify the schema version claim** + +Run from the worktree: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion && \ + for n in $(gh pr list --state open --json number --jq '.[].number'); do \ + v=$(gh pr diff $n 2>/dev/null | grep -E '^\+\s*static const int currentSchemaVersion' | head -1); \ + [ -n "$v" ] && echo "PR $n: $v"; \ + done +``` + +Expected: claims at 165 (#1290), 166 (#1300), 167 (#1276), plus stale claims from #1237 and #603 that are far below main and do not count. Combined with `currentSchemaVersion = 164` on this branch, the next free rung is **168**. If the scan shows a claim at 168, use the next free number above every claim and substitute it everywhere `168` appears in this plan, including the test filename. + +- [ ] **Step 2: Write the failing migration test** + +Create `test/core/database/migration_v168_trip_day_weather_test.dart`. Model it on the other `migration_v*_test.dart` files in that directory: read one first to copy the exact in-memory database setup helper they use. + +```dart +import 'package:drift/drift.dart'; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/database/database.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('v168 trip_day_weather', () { + test('the ladder claims 168', () { + expect( + AppDatabase.currentSchemaVersion, + greaterThanOrEqualTo(168), + ); + expect(AppDatabase.migrationVersions, contains(168)); + }); + + test('a new database has the trip_day_weather table with every column', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final rows = await db + .customSelect('PRAGMA table_info(trip_day_weather)') + .get(); + final names = rows.map((r) => r.data['name'] as String).toSet(); + + expect(names, containsAll({ + 'id', + 'trip_id', + 'date', + 'latitude', + 'longitude', + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + 'weather_source', + 'fetched_at', + 'created_at', + 'updated_at', + 'hlc', + })); + }); + + test('one row per trip and date', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + await db.customStatement('PRAGMA foreign_keys = OFF'); + + Future insert(String id) => db.customStatement( + 'INSERT INTO trip_day_weather ' + '(id, trip_id, date, latitude, longitude, weather_source, ' + 'fetched_at, created_at, updated_at) ' + "VALUES ('$id', 'trip-1', 1000, 1.0, 2.0, 'openMeteo', 1, 1, 1)", + ); + + await insert('a'); + await expectLater(insert('b'), throwsA(isA())); + }); + + test('the migration adds the table to a stranded v164 database', () async { + // A database stamped at the previous rung must gain the table on open. + final executor = NativeDatabase.memory(); + final raw = AppDatabase(executor); + await raw.customStatement('PRAGMA user_version = 164'); + await raw.customStatement('DROP TABLE IF EXISTS trip_day_weather'); + await raw.close(); + + final db = AppDatabase(executor); + addTearDown(db.close); + final rows = await db + .customSelect('PRAGMA table_info(trip_day_weather)') + .get(); + + expect(rows, isNotEmpty); + }); + }); +} +``` + +Note on the last test: check how the sibling `migration_v*_test.dart` files build a stranded database. If they use a shared helper (a temp file database re-opened at a pinned `user_version`), use that helper verbatim instead of the sketch above, because an in-memory executor cannot always be reopened. + +- [ ] **Step 3: Run the test and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/database/migration_v168_trip_day_weather_test.dart +``` + +Expected: FAIL. `migrationVersions` does not contain 168, and `PRAGMA table_info(trip_day_weather)` returns no rows. + +- [ ] **Step 4: Add the Drift table** + +In `lib/core/database/database.dart`, directly after the `TripItineraryDays` class (which ends at line 136), add: + +```dart +/// Fetched historical weather for one trip day, for days whose dives supply +/// no weather of their own (surface days and dive-free itinerary days). +/// +/// A separate table rather than columns on `trips` or `trip_itinerary_days` +/// on purpose: HLC conflicts resolve per row, so parking an automatic, +/// derived write on a row the diver also edits by hand lets a weather write +/// race a rename or a note edit and lose it. Weather owns its own row and its +/// own clock. +/// +/// Metric storage throughout (celsius, m/s, bar); conversion to the diver's +/// units happens at display time. +class TripDayWeather extends Table { + TextColumn get id => text()(); + TextColumn get tripId => text().references(Trips, #id)(); + + /// Local midnight for the day, as epoch milliseconds (the same convention + /// TripItineraryDays.date is written with). + IntColumn get date => integer()(); + + /// The coordinates the lookup actually used, so a row records what it was + /// fetched for even if the trip's sites later move. + RealColumn get latitude => real()(); + RealColumn get longitude => real()(); + + RealColumn get airTemp => real().nullable()(); // celsius + TextColumn get cloudCover => text().nullable()(); // enum: CloudCover.name + TextColumn get precipitation => + text().nullable()(); // enum: Precipitation.name + RealColumn get windSpeed => real().nullable()(); // m/s + TextColumn get windDirection => + text().nullable()(); // enum: CurrentDirection.name + RealColumn get humidity => real().nullable()(); // 0-100 + RealColumn get surfacePressure => real().nullable()(); // bar + + /// Raw WMO weather code, kept so prose renders in the diver's locale at + /// display time rather than being frozen as English at fetch time. + IntColumn get weatherCode => integer().nullable()(); + + TextColumn get weatherSource => + text().withDefault(const Constant('openMeteo'))(); + IntColumn get fetchedAt => integer()(); + IntColumn get createdAt => integer()(); + IntColumn get updatedAt => integer()(); + + /// Hybrid Logical Clock for cross-device conflict resolution + /// (nullable: rows written before HLC rollout fall back to updatedAt). + TextColumn get hlc => text().nullable()(); + + @override + Set get primaryKey => {id}; +} +``` + +Then add `TripDayWeather,` to the `tables: [...]` list in the `@DriftDatabase` annotation, next to `TripItineraryDays`. + +- [ ] **Step 5: Add the schema-assert helper** + +Next to `_assertQualityFindingsSchema()` (line 3932), which is the pattern to copy, add: + +```dart + /// v168: fetched per-day trip weather. Idempotent, so it doubles as the + /// beforeOpen backstop for a database that took the rung before the unique + /// index existed. + Future _assertTripDayWeatherSchema() async { + await customStatement(''' + CREATE TABLE IF NOT EXISTS trip_day_weather ( + id TEXT NOT NULL PRIMARY KEY, + trip_id TEXT NOT NULL REFERENCES trips (id), + date INTEGER NOT NULL, + latitude REAL NOT NULL, + longitude REAL NOT NULL, + air_temp REAL, + cloud_cover TEXT, + precipitation TEXT, + wind_speed REAL, + wind_direction TEXT, + humidity REAL, + surface_pressure REAL, + weather_code INTEGER, + weather_source TEXT NOT NULL DEFAULT 'openMeteo', + fetched_at INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + hlc TEXT + ) + '''); + await customStatement( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_trip_day_weather_trip_date ' + 'ON trip_day_weather (trip_id, date)', + ); + } +``` + +- [ ] **Step 6: Claim the rung in all six places** + +1. `static const int currentSchemaVersion = 168;` (was 164, line 3183). +2. Append `168,` to the end of the `migrationVersions` list. The ladder is non-contiguous by design (162 is permanently skipped and reserved rungs may be missing); do not "fix" the gaps. +3. The helper docstring above already names v168. +4. In `onUpgrade`, after the `if (from < 164) await reportProgress();` pair at line 8617, add both halves: + +```dart + // v168: trip_day_weather, fetched per-day weather for trip days whose + // dives supply none. + if (from < 168) { + await _assertTripDayWeatherSchema(); + } + if (from < 168) await reportProgress(); +``` + +5. In `beforeOpen`, alongside the other backstops (around line 8823), add: + +```dart + // v168 backstop: re-assert the trip day weather table (the helper is + // CREATE TABLE IF NOT EXISTS, so it is safe on every open). + await _assertTripDayWeatherSchema(); +``` + +6. The test file created in Step 2 already carries the version in its filename and assertions. + +Leave `minimumCompatibleSchemaVersion` at 160. + +- [ ] **Step 7: Run codegen** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + dart run build_runner build --delete-conflicting-outputs +``` + +Expected: succeeds, and `lib/core/database/database.g.dart` now defines `TripDayWeatherData` and `$TripDayWeatherTable`. + +- [ ] **Step 8: Write the domain entity** + +Create `lib/features/trips/domain/entities/trip_day_weather.dart`: + +```dart +import 'package:equatable/equatable.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// Stored historical weather for one trip day. +/// +/// Written only for days whose dives supply no weather of their own; a day +/// with dive-logged weather always renders that instead. +class TripDayWeather extends Equatable { + final String id; + final String tripId; + + /// Local midnight for the day this describes. + final DateTime date; + + /// The coordinates the lookup used. + final double latitude; + final double longitude; + + final double? airTemp; // celsius + final CloudCover? cloudCover; + final Precipitation? precipitation; + final double? windSpeed; // m/s + final CurrentDirection? windDirection; + final double? humidity; // 0-100 + final double? surfacePressure; // bar + final int? weatherCode; + final WeatherSource weatherSource; + final DateTime fetchedAt; + final DateTime createdAt; + final DateTime updatedAt; + + const TripDayWeather({ + required this.id, + required this.tripId, + required this.date, + required this.latitude, + required this.longitude, + this.airTemp, + this.cloudCover, + this.precipitation, + this.windSpeed, + this.windDirection, + this.humidity, + this.surfacePressure, + this.weatherCode, + this.weatherSource = WeatherSource.openMeteo, + required this.fetchedAt, + required this.createdAt, + required this.updatedAt, + }); + + /// True when at least one field the day header can render is present. + /// A result with nothing renderable is not worth a row. + bool get hasRenderableWeather => + airTemp != null || cloudCover != null || precipitation != null; + + /// The compact view model the day header consumes. + TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); + + TripDayWeather copyWith({ + String? id, + String? tripId, + DateTime? date, + double? latitude, + double? longitude, + Object? airTemp = _undefined, + Object? cloudCover = _undefined, + Object? precipitation = _undefined, + Object? windSpeed = _undefined, + Object? windDirection = _undefined, + Object? humidity = _undefined, + Object? surfacePressure = _undefined, + Object? weatherCode = _undefined, + WeatherSource? weatherSource, + DateTime? fetchedAt, + DateTime? createdAt, + DateTime? updatedAt, + }) { + return TripDayWeather( + id: id ?? this.id, + tripId: tripId ?? this.tripId, + date: date ?? this.date, + latitude: latitude ?? this.latitude, + longitude: longitude ?? this.longitude, + airTemp: airTemp == _undefined ? this.airTemp : airTemp as double?, + cloudCover: cloudCover == _undefined + ? this.cloudCover + : cloudCover as CloudCover?, + precipitation: precipitation == _undefined + ? this.precipitation + : precipitation as Precipitation?, + windSpeed: windSpeed == _undefined + ? this.windSpeed + : windSpeed as double?, + windDirection: windDirection == _undefined + ? this.windDirection + : windDirection as CurrentDirection?, + humidity: humidity == _undefined ? this.humidity : humidity as double?, + surfacePressure: surfacePressure == _undefined + ? this.surfacePressure + : surfacePressure as double?, + weatherCode: weatherCode == _undefined + ? this.weatherCode + : weatherCode as int?, + weatherSource: weatherSource ?? this.weatherSource, + fetchedAt: fetchedAt ?? this.fetchedAt, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } + + @override + List get props => [ + id, + tripId, + date, + latitude, + longitude, + airTemp, + cloudCover, + precipitation, + windSpeed, + windDirection, + humidity, + surfacePressure, + weatherCode, + weatherSource, + fetchedAt, + createdAt, + updatedAt, + ]; +} + +// Sentinel value for distinguishing null from undefined in copyWith +const _undefined = Object(); +``` + +Before writing this, confirm `CurrentDirection` and `WeatherSource` exist in `lib/core/constants/enums.dart` (they do: `WeatherSource` is at line 567, and `windDirection` on the dives table is documented as `CurrentDirection.name` at line 733). + +- [ ] **Step 9: Run the migration test and the database suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/database/ +``` + +Expected: PASS, including the pre-existing ladder audit tests. Roughly 465 tests, about 15 seconds. If a ladder audit fails, re-read the six places above; a missing `migrationVersions` entry or a missing `reportProgress()` twin is the usual cause. + +- [ ] **Step 10: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(db): add trip_day_weather table at schema v168 + +One row per trip day, holding fetched historical weather for days whose +dives supply none. A separate table rather than columns on an existing +row so an automatic derived write never conflicts with hand-entered data +under row-level HLC resolution." +``` + +--- + +### Task 2: Repository + +**Files:** +- Create: `lib/features/trips/data/repositories/trip_day_weather_repository.dart` +- Modify: `lib/features/trips/data/repositories/trip_repository.dart:263-267` (the `deleteTrip` transaction) +- Test: `test/features/trips/data/repositories/trip_day_weather_repository_test.dart` + +**Interfaces:** +- Consumes: `TripDayWeather` domain entity and the `_db.tripDayWeather` Drift table from Task 1. +- Produces: `TripDayWeatherRepository` with + - `Stream watchWeatherChanges()` + - `Future> getForTrip(String tripId)` keyed by `date.millisecondsSinceEpoch` + - `Future upsert(TripDayWeather weather)` + - `Future deleteByTripId(String tripId)` + +- [ ] **Step 1: Write the failing repository test** + +Create `test/features/trips/data/repositories/trip_day_weather_repository_test.dart`. Read `test/features/trips/data/repositories/itinerary_day_repository_test.dart` first (if it exists) or another repository test in `test/features/trips/data/repositories/` to copy the exact `DatabaseService` test harness those tests use, since these repositories reach `DatabaseService.instance.database` rather than taking a database argument. + +```dart +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + late TripDayWeatherRepository repository; + + // Use the same in-memory DatabaseService setup the sibling trip repository + // tests use, and insert a parent trip row named 'trip-1' before each test + // (trip_id is a non-nullable FK and beforeOpen enables foreign keys). + + TripDayWeather sample({ + String id = 'w1', + String tripId = 'trip-1', + DateTime? date, + double? airTemp = 21.5, + }) { + final day = date ?? DateTime(2026, 3, 8); + return TripDayWeather( + id: id, + tripId: tripId, + date: day, + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: CloudCover.clear, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ); + } + + test('upsert then read back, keyed by date millis', () async { + await repository.upsert(sample()); + + final stored = await repository.getForTrip('trip-1'); + + expect(stored, hasLength(1)); + final row = stored[DateTime(2026, 3, 8).millisecondsSinceEpoch]!; + expect(row.airTemp, 21.5); + expect(row.cloudCover, CloudCover.clear); + expect(row.weatherSource, WeatherSource.openMeteo); + }); + + test('upserting the same day twice keeps one row', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', airTemp: 25)); + + final stored = await repository.getForTrip('trip-1'); + + expect(stored, hasLength(1)); + expect( + stored[DateTime(2026, 3, 8).millisecondsSinceEpoch]!.airTemp, + 25, + ); + }); + + test('getForTrip is scoped to one trip', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: 'trip-2')); + + expect(await repository.getForTrip('trip-1'), hasLength(1)); + }); + + test('deleteByTripId removes only that trip rows', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: 'trip-2')); + + await repository.deleteByTripId('trip-1'); + + expect(await repository.getForTrip('trip-1'), isEmpty); + expect(await repository.getForTrip('trip-2'), hasLength(1)); + }); +} +``` + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/data/repositories/trip_day_weather_repository_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_repository.dart`. + +- [ ] **Step 3: Write the repository** + +Create `lib/features/trips/data/repositories/trip_day_weather_repository.dart`. `ItineraryDayRepository` is the shape to copy exactly: the `DatabaseService.instance.database` getter, the `SyncRepository()` field, the logger, `markRecordPending` after every write, `logDeletion` for every delete, and `SyncEventBus.notifyLocalChange()` at the end of each mutating method. + +```dart +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + as domain; + +/// Reads and writes stored per-day trip weather. +/// +/// The unique index on (trip_id, date) is what makes an upsert idempotent: +/// two devices that both fetch the same day converge on one row rather than +/// accumulating duplicates. +class TripDayWeatherRepository { + AppDatabase get _db => DatabaseService.instance.database; + final SyncRepository _syncRepository = SyncRepository(); + final _log = LoggerService.forClass(TripDayWeatherRepository); + + /// Emits whenever `trip_day_weather` changes, so the display provider + /// refreshes after a backfill write or a sync import. + Stream watchWeatherChanges() => + _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); + + /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + Future> getForTrip(String tripId) async { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + return {for (final row in rows) row.date: _mapRow(row)}; + } + + /// Insert or replace one day's weather. + Future upsert(domain.TripDayWeather weather) async { + try { + final now = DateTime.now().millisecondsSinceEpoch; + + // Replace any existing row for this day rather than relying on the id: + // the day is the identity, and a peer may have written its own id for + // the same day. + final existing = + await (_db.select(_db.tripDayWeather)..where( + (t) => + t.tripId.equals(weather.tripId) & + t.date.equals(weather.date.millisecondsSinceEpoch), + )) + .getSingleOrNull(); + final id = existing?.id ?? weather.id; + + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherCompanion( + id: Value(id), + tripId: Value(weather.tripId), + date: Value(weather.date.millisecondsSinceEpoch), + latitude: Value(weather.latitude), + longitude: Value(weather.longitude), + airTemp: Value(weather.airTemp), + cloudCover: Value(weather.cloudCover?.name), + precipitation: Value(weather.precipitation?.name), + windSpeed: Value(weather.windSpeed), + windDirection: Value(weather.windDirection?.name), + humidity: Value(weather.humidity), + surfacePressure: Value(weather.surfacePressure), + weatherCode: Value(weather.weatherCode), + weatherSource: Value(weather.weatherSource.name), + fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), + createdAt: Value( + existing?.createdAt ?? weather.createdAt.millisecondsSinceEpoch, + ), + updatedAt: Value(now), + ), + ); + + await _syncRepository.markRecordPending( + entityType: 'tripDayWeather', + recordId: id, + localUpdatedAt: now, + ); + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to store weather for trip ${weather.tripId}', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Delete every stored day for a trip, logging each id for sync. + Future deleteByTripId(String tripId) async { + try { + final existing = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + if (existing.isEmpty) return; + + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).go(); + + for (final row in existing) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: row.id, + ); + } + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to delete weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + domain.TripDayWeather _mapRow(TripDayWeatherData row) { + return domain.TripDayWeather( + id: row.id, + tripId: row.tripId, + date: DateTime.fromMillisecondsSinceEpoch(row.date), + latitude: row.latitude, + longitude: row.longitude, + airTemp: row.airTemp, + cloudCover: row.cloudCover == null + ? null + : CloudCover.values.byName(row.cloudCover!), + precipitation: row.precipitation == null + ? null + : Precipitation.values.byName(row.precipitation!), + windSpeed: row.windSpeed, + windDirection: row.windDirection == null + ? null + : CurrentDirection.values.byName(row.windDirection!), + humidity: row.humidity, + surfacePressure: row.surfacePressure, + weatherCode: row.weatherCode, + weatherSource: WeatherSource.values.byName(row.weatherSource), + fetchedAt: DateTime.fromMillisecondsSinceEpoch(row.fetchedAt), + createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), + updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), + ); + } +} +``` + +If the generated row class is not named `TripDayWeatherData`, check `database.g.dart` for the actual name and use it. + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/data/repositories/trip_day_weather_repository_test.dart +``` + +Expected: PASS. + +- [ ] **Step 5: Wire the trip-delete cascade** + +In `lib/features/trips/data/repositories/trip_repository.dart`, inside the `deleteTrip` transaction (line 263), add the new repository next to the others: + +```dart + await LiveaboardDetailsRepository().deleteByTripId(id); + await ItineraryDayRepository().deleteByTripId(id); + await TripChecklistRepository().deleteByTripId(id); + await TripDayWeatherRepository().deleteByTripId(id); +``` + +Add the import for `trip_day_weather_repository.dart` in the same file's local import group. + +- [ ] **Step 6: Add a deletion test and run the trip repository suite** + +Append to the repository test file: + +```dart + test('deleting a trip takes its weather rows with it', () async { + await repository.upsert(sample()); + + await TripRepository().deleteTrip('trip-1'); + + expect(await repository.getForTrip('trip-1'), isEmpty); + }); +``` + +Import `TripRepository` at the top of the test file. Then: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/ +``` + +Expected: PASS, with no regressions in the existing trip tests. + +- [ ] **Step 7: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): add TripDayWeatherRepository + +Upsert is keyed by (trip, date) rather than by id so two devices that +both fetch the same day converge on one row. Deleting a trip takes its +weather rows with it, logged for sync like every other child record." +``` + +--- + +### Task 3: Sync registration + +**Files:** +- Modify: `lib/core/data/repositories/sync_repository.dart:49` (the `hlcTargets` map) +- Modify: `lib/core/services/sync/sync_data_serializer.dart` (13 sites, listed below) +- Modify: `lib/core/services/sync/sync_service.dart:1209`, `:1959`, `:2120` +- Test: `test/core/services/sync/trip_day_weather_sync_test.dart` + +**Interfaces:** +- Consumes: the `trip_day_weather` table from Task 1. +- Produces: entity type key `'tripDayWeather'` registered end to end, and `SyncData.tripDayWeather` as a `List>` field. + +Every site below is a copy of what `'itineraryDays'` does. Three structural tests already in the suite will fail if any site is missed: `sync_hlc_target_registration_test` (asserts every table with an `hlc` column is in `hlcTargets`), the `entityHasUpdatedAt covers exactly the SyncData entities` test, and the merge-order parity test. + +- [ ] **Step 1: Write the failing round-trip test** + +Create `test/core/services/sync/trip_day_weather_sync_test.dart`, modeled directly on `test/core/services/sync/site_features_sync_test.dart` (the closest analogue: a synced child record with a non-nullable FK to its parent). + +```dart +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/sync/sync_data_serializer.dart'; +import 'package:submersion/core/services/sync/sync_service.dart'; + +import '../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late SyncDataSerializer serializer; + + setUp(() async { + db = await setUpTestDatabase(); + serializer = SyncDataSerializer(); + await db + .into(db.trips) + .insert( + TripsCompanion.insert( + id: 'trip-1', + name: 'Bonaire', + createdAt: 1, + updatedAt: 1, + ), + ); + await db + .into(db.tripDayWeather) + .insert( + TripDayWeatherCompanion.insert( + id: 'w-1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8).millisecondsSinceEpoch, + latitude: 12.16, + longitude: -68.28, + airTemp: const Value(24.0), + cloudCover: const Value('clear'), + fetchedAt: 1, + createdAt: 1, + updatedAt: 1, + ), + ); + }); + + tearDown(tearDownTestDatabase); + + test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { + final record = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(record, isNotNull); + expect(record!['airTemp'], 24.0); + expect(record['cloudCover'], 'clear'); + + // A remote edit merges over the local row (LWW payload apply). + await serializer.upsertRecord('tripDayWeather', { + ...record, + 'airTemp': 26.0, + 'updatedAt': 2, + }); + final merged = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(merged!['airTemp'], 26.0); + + expect(await serializer.recordIdsFor('tripDayWeather'), contains('w-1')); + + await serializer.deleteRecord('tripDayWeather', 'w-1'); + expect(await serializer.fetchRecord('tripDayWeather', 'w-1'), isNull); + }); + + test('the delta export filters on the row own hlc', () async { + await (db.update(db.tripDayWeather)..where((t) => t.id.equals('w-1'))) + .write( + const TripDayWeatherCompanion( + hlc: Value('2026-08-16T00:00:00.000-0000'), + ), + ); + + Future changesetCount(String? watermark) async { + final payload = await serializer.exportChangeset( + deviceId: 'device-1', + hlcWatermark: watermark, + deletions: const [], + ); + return payload.data.tripDayWeather.length; + } + + // A base carries the row; a watermark newer than it excludes it; an + // older watermark includes it. + expect(await changesetCount(null), 1); + expect(await changesetCount('2026-08-17T00:00:00.000-0000'), 0); + expect(await changesetCount('2026-08-15T00:00:00.000-0000'), 1); + }); + + test('tripDayWeather is registered as an hlc target', () { + expect(SyncRepository.hlcTargets.containsKey('tripDayWeather'), isTrue); + expect( + SyncRepository.hlcTargets['tripDayWeather']!.table, + 'trip_day_weather', + ); + }); + + test('tripDayWeather carries an updatedAt flag', () { + expect(SyncService.entityHasUpdatedAt['tripDayWeather'], isTrue); + }); +} +``` + +Two things to check against generated code rather than assume. First, the keys in the `fetchRecord` map come from Drift's generated `toJson()`, which uses Dart field names (`airTemp`, `cloudCover`), not SQL column names; if an assertion fails on a key, confirm the real name in `database.g.dart`. Second, `TripsCompanion.insert` may require more than `id`/`name`/`createdAt`/`updatedAt`; read the `Trips` table definition and supply whatever else is non-nullable and undefaulted. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/services/sync/trip_day_weather_sync_test.dart +``` + +Expected: FAIL, because `hlcTargets` has no `tripDayWeather` key and `SyncData` has no such field. + +- [ ] **Step 3: Register the HLC target** + +In `lib/core/data/repositories/sync_repository.dart`, after the `'itineraryDays'` entry (line 49): + +```dart + 'tripDayWeather': (table: 'trip_day_weather', pk: 'id'), +``` + +- [ ] **Step 4: Add all 13 serializer sites** + +In `lib/core/services/sync/sync_data_serializer.dart`, add a `tripDayWeather` line immediately after each `itineraryDays` line at these locations: + +1. Line 249, the `SyncData` field list: `final List> tripDayWeather;` +2. Line 324, the constructor: `this.tripDayWeather = const [],` +3. Line 400, `toJson`: `'tripDayWeather': tripDayWeather,` +4. Line 477, `fromJson`: `tripDayWeather: _parseList(json['tripDayWeather']),` +5. Line 762, the table tuple list: + +```dart + ( + key: 'tripDayWeather', + table: _db.tripDayWeather, + blob: false, + full: null, + ), +``` + +6. Line 1268, the export block: + +```dart + tripDayWeather: await _safeExport( + 'tripDayWeather', + () => _exportTripDayWeather(hlcSince), + ), +``` + +7. Line 1707, single-record fetch: + +```dart + case 'tripDayWeather': + final row = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).getSingleOrNull(); + return row?.toJson(); +``` + +8. Line 2032, batch fetch: + +```dart + case 'tripDayWeather': + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.isIn(idList))).get(); + return {for (final r in rows) r.id: r.toJson()}; +``` + +9. Line 2592, single upsert: + +```dart + case 'tripDayWeather': + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherData.fromJson(data).toCompanion(false), + ); + return; +``` + +10. Line 3196, batch upsert: + +```dart + case 'tripDayWeather': + await _db.batch( + (b) => b.insertAllOnConflictUpdate( + _db.tripDayWeather, + records + .map((r) => TripDayWeatherData.fromJson(r).toCompanion(false)) + .toList(), + ), + ); + return; +``` + +11. Line 3681, the plain id projection: `case 'tripDayWeather': return plain(_db.tripDayWeather, _db.tripDayWeather.id);` +12. Line 3914, table lookup: `case 'tripDayWeather': return _db.tripDayWeather;` +13. Line 4222, delete by id: + +```dart + case 'tripDayWeather': + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).go(); + return; +``` + +Then add the export helper next to `_exportItineraryDays` (line 4892), copying its body exactly: + +```dart + Future>> _exportTripDayWeather( + String? hlcSince, + ) async { + final query = _db.select(_db.tripDayWeather); + if (hlcSince != null) { + query.where((t) => t.hlc.isBiggerThanValue(hlcSince)); + } + final rows = await query.get(); + return rows.map((r) => r.toJson()).toList(); + } +``` + +Read `_exportItineraryDays` in full before copying: if it does anything else (such as a diver scope filter), mirror that too. + +Use the generated row class name from `database.g.dart` in sites 9 and 10; the plan assumes `TripDayWeatherData`. + +- [ ] **Step 5: Add the three sync_service sites** + +In `lib/core/services/sync/sync_service.dart`: + +1. After the `itineraryDays` merge-order record at line 1209: + +```dart + ( + type: 'tripDayWeather', + records: data.tripDayWeather, + hasUpdatedAt: true, + ), +``` + +2. In `entityHasUpdatedAt` after line 1959: `'tripDayWeather': true,` +3. In the FK parent map after line 2120: + +```dart + 'tripDayWeather': [(field: 'tripId', parent: 'trips', nullable: false)], +``` + +- [ ] **Step 6: Run the sync suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/core/services/sync/ +``` + +Expected: PASS, including the new round-trip test and the three structural tests. A failure naming an entity coverage mismatch means one of the 17 sites is missing. + +- [ ] **Step 7: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(sync): replicate trip day weather + +Registers tripDayWeather 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." +``` + +--- + +### Task 4: Backfill rules (pure domain logic) + +**Files:** +- Create: `lib/features/trips/domain/services/trip_day_weather_backfill.dart` +- Test: `test/features/trips/domain/services/trip_day_weather_backfill_test.dart` + +**Interfaces:** +- Consumes: `TripStory`, `TripStoryDay`, `TripStoryMapGeometry` (all in `features/trips/domain/entities/`), and the `Map` shape `TripDayWeatherRepository.getForTrip` returns. +- Produces: + +```dart +class TripDayWeatherTarget extends Equatable { + final DateTime date; // local midnight + final double latitude; + final double longitude; + DateTime get localNoon; // date at 12:00, the sample hour to request +} + +class TripDayWeatherBackfill { + static List targetsFor({ + required TripStory story, + required Map stored, + }); +} +``` + +This task is pure: no database, no network, no Riverpod. That is the point. All four skip rules are testable as a plain function. + +- [ ] **Step 1: Write the failing test** + +Create `test/features/trips/domain/services/trip_day_weather_backfill_test.dart`: + +```dart +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; + +void main() { + // Build the smallest story that exercises one rule at a time. Copy the + // Trip/Dive fixture helpers from + // test/features/trips/domain/entities/trip_story_day_test.dart rather than + // hand-rolling new ones. + + Trip trip() => Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 14), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ); + + TripStory storyWith( + List days, { + List points = const [], + }) { + return TripStory( + trip: trip(), + days: days, + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: TripStoryMapGeometry(points: points), + ); + } + + Dive diveWith({double? airTemp}) => + Dive(id: 'd1', dateTime: DateTime(2026, 3, 8, 9), airTemp: airTemp); + + TripStoryDay day({ + required int index, + TripStoryDayKind kind = TripStoryDayKind.past, + List dives = const [], + }) { + return TripStoryDay( + date: DateTime(2026, 3, 8 + index), + dayNumber: index + 1, + kind: kind, + dives: dives, + ); + } + + TripStoryMapPoint pointFor(int dayIndex) => TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: dayIndex, + label: 'Site', + ); + + test('a past day with no dives and a nearby point is a target', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 8)); + expect(targets.single.latitude, 12.16); + expect(targets.single.localNoon, DateTime(2026, 3, 8, 12)); + }); + + test('a day whose dives carry weather is skipped', () { + // Build a dive with airTemp set so TripStoryDay.weather is non-null. + final story = storyWith( + [day(index: 0, dives: [diveWith(airTemp: 26)])], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a future day is skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.future)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day with a stored row is skipped', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final stored = { + DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + ); + }); + + test('a day with no map point anywhere in the story is skipped', () { + final story = storyWith([day(index: 0)]); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day borrows the nearest day point when it has none of its own', () { + // nearestPointForDay already walks outward, so day 1 with a point only on + // day 0 is still a target, at day 0 coordinates. + final story = storyWith( + [day(index: 0, dives: [diveWith(airTemp: 26)]), day(index: 1)], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 9)); + expect(targets.single.latitude, 12.16); + }); +} +``` + +Add the imports these fixtures need: `Trip` from `features/trips/domain/entities/trip.dart` and `Dive` from `features/dive_log/domain/entities/dive.dart`. If `Trip`'s constructor has gained a required parameter, read the entity and supply it. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/domain/services/trip_day_weather_backfill_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_backfill.dart`. + +- [ ] **Step 3: Write the service** + +Create `lib/features/trips/domain/services/trip_day_weather_backfill.dart`: + +```dart +import 'package:equatable/equatable.dart'; + +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// One day that needs a weather lookup, with the coordinates to look it up at. +class TripDayWeatherTarget extends Equatable { + /// Local midnight for the day. + final DateTime date; + final double latitude; + final double longitude; + + const TripDayWeatherTarget({ + required this.date, + required this.latitude, + required this.longitude, + }); + + /// The hour to sample. Noon local reads as "the day's weather" far better + /// than the API's default midnight boundary. + DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); + + @override + List get props => [date, latitude, longitude]; +} + +/// Decides which trip days need a weather lookup. +/// +/// Pure by design: no database, no network. Every skip rule is a plain +/// condition over the built story and the rows already stored. +class TripDayWeatherBackfill { + const TripDayWeatherBackfill._(); + + static List targetsFor({ + required TripStory story, + required Map stored, + }) { + final targets = []; + + for (var index = 0; index < story.days.length; index++) { + final day = story.days[index]; + + // A dive that logged weather is the better source; never override it. + if (day.weather != null) continue; + + // A historical archive has nothing for a day that has not happened. + if (day.kind == TripStoryDayKind.future) continue; + + final date = DateTime(day.date.year, day.date.month, day.date.day); + if (stored.containsKey(date.millisecondsSinceEpoch)) continue; + + // nearestPointForDay walks outward from the day, so a dive-free day + // between two dived days borrows the closer one's coordinates. + final point = story.mapGeometry.nearestPointForDay(index); + if (point == null) continue; + + targets.add( + TripDayWeatherTarget( + date: date, + latitude: point.latitude, + longitude: point.longitude, + ), + ); + } + + return targets; + } +} +``` + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/domain/services/trip_day_weather_backfill_test.dart +``` + +Expected: PASS, all six tests. + +- [ ] **Step 5: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): decide which trip days need a weather lookup + +Pure rules over the built story and the rows already stored: dive-logged +weather wins, future days have no archive, stored days are done, and a +day with no mappable point anywhere in the trip has nowhere to ask." +``` + +--- + +### Task 5: Providers and the fetch loop + +**Files:** +- Create: `lib/features/trips/presentation/providers/trip_day_weather_providers.dart` +- Test: `test/features/trips/presentation/providers/trip_day_weather_providers_test.dart` + +**Interfaces:** +- Consumes: `TripDayWeatherRepository` (Task 2), `TripDayWeatherBackfill.targetsFor` (Task 4), the existing `weatherServiceProvider` from `lib/features/weather/presentation/providers/weather_providers.dart`, and `tripStoryProvider(tripId)` from `trip_story_providers.dart`. +- Produces: + - `tripDayWeatherRepositoryProvider` -> `Provider` + - `tripDayWeatherProvider` -> `FutureProvider.family, String>` keyed by trip id, values keyed by `date.millisecondsSinceEpoch` + - `tripDayWeatherBackfillProvider` -> `FutureProvider.family` keyed by trip id + +**Why the backfill provider does not watch the weather table:** the display provider subscribes to the table tick, but the backfill must not, or every row it writes would invalidate it and start another pass. It reads stored rows directly from the repository instead. Its only reactive dependency is the story itself, so adding dives to a trip re-evaluates what still needs fetching. + +- [ ] **Step 1: Write the failing provider test** + +Create `test/features/trips/presentation/providers/trip_day_weather_providers_test.dart`. The existing `surface_day_weather_provider_test.dart` (deleted in Task 6) is where the `MockClient` weather-response fixture comes from; lift it rather than inventing one, because it encodes the exact Open-Meteo response shape `WeatherMapper` expects. + +```dart +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +/// A full Open-Meteo hourly payload for one day. Copied from the deleted +/// surface_day_weather_provider_test. +http.Response weatherResponse({double noonTemp = 29.0, int cloud = 95}) => + http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T09:00', '2026-03-08T12:00'], + 'temperature_2m': [24.0, noonTemp], + 'relative_humidity_2m': [80.0, 70.0], + 'precipitation': [0.0, 0.0], + 'cloud_cover': [10.0, cloud], + 'wind_speed_10m': [8.0, 12.0], + 'wind_direction_10m': [30.0, 45.0], + 'surface_pressure': [1012.0, 1011.0], + 'weathercode': [0, 0], + }, + }), + 200, + ); + +/// Records upserts instead of touching a database. +class FakeTripDayWeatherRepository implements TripDayWeatherRepository { + FakeTripDayWeatherRepository({this.stored = const {}}); + + final Map stored; + final List upserts = []; + + @override + Future> getForTrip(String tripId) async => stored; + + @override + Future upsert(TripDayWeather weather) async => upserts.add(weather); + + @override + Future deleteByTripId(String tripId) async {} + + @override + Stream watchWeatherChanges() => const Stream.empty(); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + /// Two past days with no dives, and one map point on day 0 that day 1 + /// borrows through nearestPointForDay. + TripStory twoDayStory() => TripStory( + trip: Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ), + days: [ + TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + TripStoryDay( + date: DateTime(2026, 3, 9), + dayNumber: 2, + kind: TripStoryDayKind.past, + ), + ], + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: const TripStoryMapGeometry( + points: [ + TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: 0, + label: 'Site', + ), + ], + ), + ); + + ProviderContainer containerWith({ + required http.Client client, + required FakeTripDayWeatherRepository repository, + }) { + final container = ProviderContainer( + overrides: [ + weatherHttpClientProvider.overrideWithValue(client), + tripDayWeatherRepositoryProvider.overrideWithValue(repository), + tripStoryProvider('trip-1').overrideWith((ref) async => twoDayStory()), + ], + ); + addTearDown(container.dispose); + return container; + } + + test('fetches each target once and stores the result', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((request) async { + calls++; + expect(request.url.queryParameters['timezone'], 'auto'); + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + expect(repository.upserts, hasLength(2)); + expect(repository.upserts.first.airTemp, 29.0); + expect(repository.upserts.first.latitude, 12.16); + expect(repository.upserts.first.tripId, 'trip-1'); + }); + + test('a failed fetch writes no row', () async { + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => http.Response('', 500)), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a result with nothing renderable writes no row', () async { + // Humidity and pressure only: the day header could render none of it, and + // storing it would suppress the retry once the archive catches up. + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient( + (_) async => http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T12:00'], + 'temperature_2m': [null], + 'relative_humidity_2m': [70.0], + 'precipitation': [null], + 'cloud_cover': [null], + 'wind_speed_10m': [null], + 'wind_direction_10m': [null], + 'surface_pressure': [1011.0], + 'weathercode': [null], + }, + }), + 200, + ), + ), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a day already stored is not fetched', () async { + final repository = FakeTripDayWeatherRepository( + stored: { + DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }, + ); + var calls = 0; + final container = containerWith( + client: MockClient((_) async { + calls++; + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 1); + expect(repository.upserts, hasLength(1)); + expect(repository.upserts.single.date, DateTime(2026, 3, 9)); + }); + + test('fetches run one at a time', () async { + final gate = Completer(); + var started = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + started++; + if (started == 1) await gate.future; + return weatherResponse(); + }), + repository: repository, + ); + + final pending = container.read( + tripDayWeatherBackfillProvider('trip-1').future, + ); + await Future.delayed(Duration.zero); + + // The second day must not be in flight while the first is pending. + expect(started, 1); + + gate.complete(); + await pending; + expect(started, 2); + }); +} +``` + +Add the `Trip` import (`features/trips/domain/entities/trip.dart`) for the fixture. Note that `tripStoryProvider` is a `family`, so the override must name the same key the backfill reads: `tripStoryProvider('trip-1')`. + +- [ ] **Step 2: Run it and watch it fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +``` + +Expected: FAIL with an unresolved import of `trip_day_weather_providers.dart`. + +- [ ] **Step 3: Write the providers** + +Create `lib/features/trips/presentation/providers/trip_day_weather_providers.dart`: + +```dart +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/providers/provider.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +final tripDayWeatherRepositoryProvider = Provider( + (ref) => TripDayWeatherRepository(), +); + +/// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. +/// +/// 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 +/// ever happened. +final tripDayWeatherProvider = + FutureProvider.family, String>(( + ref, + tripId, + ) async { + final repository = ref.watch(tripDayWeatherRepositoryProvider); + ref.invalidateSelfWhen(repository.watchWeatherChanges()); + return repository.getForTrip(tripId); + }); + +/// Fills the gaps: fetches historical weather for trip days that have none +/// stored and no dive to supply it, then writes what it finds. +/// +/// Deliberately does NOT watch [tripDayWeatherProvider]. Watching the rows it +/// writes would invalidate this provider on every write and start another +/// pass; it reads the stored rows straight from the repository instead. The +/// story is its only reactive input, so assigning dives to a trip re-evaluates +/// what is still missing. +/// +/// Not auto-disposed: one pass per trip per provider container lifetime. +final tripDayWeatherBackfillProvider = FutureProvider.family(( + ref, + tripId, +) async { + final story = await ref.watch(tripStoryProvider(tripId).future); + final repository = ref.watch(tripDayWeatherRepositoryProvider); + final service = ref.watch(weatherServiceProvider); + + final stored = await repository.getForTrip(tripId); + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: stored, + ); + if (targets.isEmpty) return; + + const uuid = Uuid(); + + // Sequential on purpose: a two-week trip would otherwise open with a burst + // of parallel requests, and rows landing one at a time let headers fill in + // progressively. + for (final target in targets) { + final weather = await service.fetchWeather( + latitude: target.latitude, + longitude: target.longitude, + date: target.date, + entryTime: target.localNoon, + useLocationTimezone: true, + ); + if (weather == null) continue; + + final now = DateTime.now(); + final row = TripDayWeather( + id: uuid.v4(), + tripId: tripId, + date: target.date, + latitude: target.latitude, + longitude: target.longitude, + airTemp: weather.airTemp, + cloudCover: weather.cloudCover, + precipitation: weather.precipitation, + windSpeed: weather.windSpeed, + windDirection: weather.windDirection, + humidity: weather.humidity, + surfacePressure: weather.surfacePressure, + weatherCode: weather.weatherCode, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + + // A row the header could render nothing from is worse than no row: it + // would suppress the retry that a later archive update would satisfy. + if (!row.hasRenderableWeather) continue; + + await repository.upsert(row); + } +}); +``` + +- [ ] **Step 4: Run the test and watch it pass** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +``` + +Expected: PASS, all five tests. + +- [ ] **Step 5: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): fetch and store trip day weather once + +The display provider rides the table tick; the backfill deliberately does +not, so the rows it writes cannot invalidate it into another pass. A miss +or an unrenderable result writes nothing and is retried on a later view." +``` + +--- + +### Task 6: Read path, and delete the per-view fetch + +**Files:** +- Modify: `lib/features/trips/presentation/widgets/story/trip_story_day_header.dart:34-70` (drop the provider watch, take stored weather as a parameter) +- Modify: `lib/features/trips/presentation/widgets/story/trip_story_view.dart:10` (import), `:236-243` (build stored weather instead of a fetch request), and the `TripStoryDayHeader` construction in `_daySliver` +- Delete: `lib/features/trips/presentation/providers/surface_day_weather_provider.dart` +- Delete: `test/features/trips/presentation/providers/surface_day_weather_provider_test.dart` +- Modify: `test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart` (the four weather tests at lines 267-330) +- Modify: `test/features/trips/presentation/widgets/story/trip_story_view_test.dart` (add provider overrides) + +**Interfaces:** +- Consumes: `tripDayWeatherProvider` and `tripDayWeatherBackfillProvider` (Task 5). +- Produces: `TripStoryDayHeader({required TripStoryDay day, TripStoryDayWeather? storedWeather})`. The `surfaceWeatherRequest` parameter and the `SurfaceDayWeatherRequest` type are gone. + +**Note for whoever implements this:** adding a provider dependency to a widget breaks every existing test that pumps it without an override. `trip_story_view_test.dart` pumps the whole view and will need both new providers overridden. Run that file's tests before assuming the change is complete. + +- [ ] **Step 1: Update the header tests first** + +In `test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart`, replace the four tests that override `surfaceDayWeatherProvider` (lines 267-330) with tests that pass the weather in directly. The header no longer fetches, so there is no loading state to test and the "pending future" test at line 305 has no meaning; replace it with the absence case. + +```dart + testWidgets('shows stored weather in the existing badge', (tester) async { + await pumpHeader( + tester, + surfaceDay(), + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), + ); + + expect(find.byIcon(Icons.wb_sunny_outlined), findsOneWidget); + expect(find.text('22°C'), findsOneWidget); + }); + + testWidgets('stored temperature respects Fahrenheit', (tester) async { + final settings = MockSettingsNotifier(); + await settings.setTemperatureUnit(TemperatureUnit.fahrenheit); + await pumpHeader( + tester, + surfaceDay(), + settingsNotifier: settings, + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('71.6°F'), findsOneWidget); + }); + + testWidgets('without stored weather stays badge-free', (tester) async { + await pumpHeader(tester, surfaceDay()); + + expect(find.textContaining('°'), findsNothing); + expect(find.byType(CircularProgressIndicator), findsNothing); + }); + + testWidgets('dive-logged weather wins over stored weather', (tester) async { + // A day whose dive logged 26C must render 26, not the stored 22. + await pumpHeader( + tester, + dayWithDive(airTemp: 26), + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('26°C'), findsOneWidget); + expect(find.text('22°C'), findsNothing); + }); +``` + +Update the `pumpHeader` helper in that file: replace its `surfaceWeatherRequest` parameter with `storedWeather`, and drop the `extra` overrides those tests passed. Use the file's existing fixture for a day with a dive in the last test. + +- [ ] **Step 2: Run the header tests and watch them fail** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +``` + +Expected: FAIL to compile, because `TripStoryDayHeader` has no `storedWeather` parameter. + +- [ ] **Step 3: Simplify the header** + +In `trip_story_day_header.dart`: + +- Replace the `surfaceWeatherRequest` field and constructor parameter with `final TripStoryDayWeather? storedWeather;` / `this.storedWeather`. +- Delete the `surface_day_weather_provider.dart` import. +- Replace the three lines at 66-69: + +```dart + final request = day.isSurface ? surfaceWeatherRequest : null; + final fetchedWeather = request == null + ? null + : ref.watch(surfaceDayWeatherProvider(request)).asData?.value; + final weather = day.weather ?? fetchedWeather; +``` + +with: + +```dart + // Dive-logged weather always wins: it is what the diver recorded, and a + // fetched day summary is only ever a stand-in for days that logged none. + final weather = day.weather ?? storedWeather; +``` + +Update the class docstring: the header no longer fetches anything, and the note about which days get a badge should say "days with logged or stored weather". + +The widget still needs `ref` for `settingsProvider`, so it stays a `ConsumerWidget`. + +- [ ] **Step 4: Wire the view** + +In `trip_story_view.dart`: + +- Replace the `surface_day_weather_provider.dart` import with `trip_day_weather_providers.dart`. +- In the widget's `build`, watch both providers once for the whole trip: + +```dart + // One read for the whole story; the backfill is a fire-and-forget pass + // whose writes come back through the provider above. + final storedWeather = + ref.watch(tripDayWeatherProvider(widget.story.trip.id)).asData?.value ?? + const {}; + ref.watch(tripDayWeatherBackfillProvider(widget.story.trip.id)); +``` + +Pass `storedWeather` down to `_daySliver`, since that method builds the header. + +- Replace the `weatherPoint` / `surfaceWeatherRequest` block at lines 236-243 with a map lookup: + +```dart + final dayDate = DateTime(day.date.year, day.date.month, day.date.day); + final stored = storedWeather[dayDate.millisecondsSinceEpoch]; +``` + +- Construct the header with `storedWeather: stored?.toStoryWeather()`. + +Check whether `_daySliver` is called from a place that must now thread the map through; if the method is on the state class, reading the value in `build` and passing it as a parameter keeps the data flow explicit. + +- [ ] **Step 5: Delete the replaced provider** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + git rm lib/features/trips/presentation/providers/surface_day_weather_provider.dart \ + test/features/trips/presentation/providers/surface_day_weather_provider_test.dart +``` + +Then confirm nothing still references it: + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + grep -rn "surfaceDayWeather\|SurfaceDayWeatherRequest" lib test +``` + +Expected: no output. + +- [ ] **Step 6: Override the new providers in the view test** + +In `test/features/trips/presentation/widgets/story/trip_story_view_test.dart`, add to the overrides that file already builds: + +```dart + tripDayWeatherProvider( + tripId, + ).overrideWith((ref) async => const {}), + tripDayWeatherBackfillProvider(tripId).overrideWith((ref) async {}), +``` + +Without these, the view reaches a real repository and a real HTTP client under `flutter test`. + +- [ ] **Step 7: Run the trips suite** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test test/features/trips/ +``` + +Expected: PASS. A hang here usually means a widget test hit a real database or HTTP client; check the overrides. + +- [ ] **Step 8: Format and commit** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + dart format . && \ + git add -A && \ + git commit -m "feat(trips): read trip day weather from the database + +The day header no longer fetches: it renders whatever weather it is +handed, with dive-logged weather still winning. surfaceDayWeatherProvider +and its per-view Open-Meteo call are deleted." +``` + +--- + +### Task 7: Whole-project verification + +**Files:** none created; this task proves the branch is shippable. + +- [ ] **Step 1: Format the whole project** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + dart format . +``` + +- [ ] **Step 2: Analyze the whole project** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + flutter analyze +``` + +Expected: "No issues found!". CI treats infos as fatal, so an info-level finding must be fixed too. Do not pipe this through `grep`: a pipe masks the exit code and hides failures. The "Analyzing ..." line is the receipt that it ran in the right tree. + +- [ ] **Step 3: Run the full test suite once** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + echo "PWD: $(pwd)" && \ + flutter test 2>&1 | tail -40 +``` + +Expected: all tests pass. Notes: +- One full run is sufficient before opening a PR; do not run the suite repeatedly. +- Do not overlap this with another local test run, including in another worktree: concurrent runs produce spurious lone failures. +- If the output ends with no summary line and exit code 0, the run was killed by a sibling session rather than passing. Re-run it. +- If a single file fails here but passes when run alone, it is a known cross-test interaction, not a regression in this branch. Confirm by running that file alone before investigating. + +- [ ] **Step 4: Re-check the schema rung** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + grep -n "currentSchemaVersion = " lib/core/database/database.dart | head -1 +``` + +Expected: 168, and still above every claim found by the Task 1 scan. If another branch has landed on 168 since, renumber: the six places from Task 1 Step 6 plus the test filename, then re-run `flutter test test/core/database/`. + +- [ ] **Step 5: Commit anything the format pass touched** + +```bash +cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ + git status --short && \ + git add -A && \ + git commit -m "chore: format" || echo "nothing to commit" +``` + +--- + +## What this plan does not do + +- **Range batching.** The Open-Meteo archive endpoint accepts `start_date`/`end_date`, so days sharing a coordinate could collapse into one request. Every day is fetched exactly once ever, so this is an optimization, not a fix. Noted in the spec as a follow-up. +- **A manual refresh action.** Nothing in the UI re-fetches a stored day. Deleting the row is the only way to force a refetch, and no UI exposes that. +- **Weather on dive days.** Dives already store their own, and `TripStoryDay.weather` composes it. diff --git a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md index 6991c63a2b..a6f474555e 100644 --- a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md +++ b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md @@ -65,7 +65,7 @@ New table `TripDayWeather` in `lib/core/database/database.dart`: | --- | --- | --- | | `id` | text, pk | uuid v4 | | `tripId` | text | references `Trips(#id)` | -| `date` | int | unix seconds, local midnight, same convention as `trip_itinerary_days.date` | +| `date` | int | epoch **milliseconds** at local midnight, matching what `ItineraryDayRepository` writes for `trip_itinerary_days.date` (the column comment there says "Unix timestamp", but the repository writes `millisecondsSinceEpoch`) | | `latitude` | real | the coordinate the lookup used | | `longitude` | real | the coordinate the lookup used | | `airTemp` | real, nullable | celsius | @@ -77,9 +77,9 @@ New table `TripDayWeather` in `lib/core/database/database.dart`: | `surfacePressure` | real, nullable | bar | | `weatherCode` | int, nullable | raw WMO code, so prose renders in the diver's locale at display time | | `weatherSource` | text | defaults to `openMeteo` | -| `fetchedAt` | int | unix seconds | -| `createdAt` | int | unix seconds | -| `updatedAt` | int | unix seconds | +| `fetchedAt` | int | epoch milliseconds | +| `createdAt` | int | epoch milliseconds | +| `updatedAt` | int | epoch milliseconds | | `hlc` | text, nullable | matches every other synced table | Unique index on (`tripId`, `date`). @@ -105,7 +105,7 @@ auto-merge with no conflict marker. The claim touches the six places the ladder requires: the `currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the -`_assertTripDayWeatherTable()` helper docstring, the `if (from < 168)` +`_assertTripDayWeatherSchema()` helper docstring, the `if (from < 168)` onUpgrade guard and its `reportProgress()` twin, the `beforeOpen` backstop comment, and the `migration_v168_trip_day_weather_test.dart` filename with its version assertions. The ladder is non-contiguous by design (v162 is @@ -120,13 +120,18 @@ an older build simply ignores it. A new `TripDayWeatherRepository` in `lib/features/trips/data/repositories/trip_day_weather_repository.dart`: -- `watchForTrip(String tripId)` streams the trip's rows keyed by date. -- `getForTrip(String tripId)` reads them once. -- `upsert(TripDayWeather)` writes one row, stamping `updatedAt` and `hlc` the - way the sibling trip repositories do. -- `deleteForTrip(String tripId)` removes them, called from the trip-delete +- `watchWeatherChanges()` emits on every table change, so the display + provider refreshes after a backfill write or a sync import. +- `getForTrip(String tripId)` reads a trip's rows, keyed by + `date.millisecondsSinceEpoch`. +- `upsert(TripDayWeather)` writes one row, keyed on (trip, date) rather than + on id so two devices that both fetch the same day converge on one row. +- `deleteByTripId(String tripId)` removes them, called from the trip-delete path alongside itinerary days. +The method names follow `ItineraryDayRepository`, which is the sibling to +copy. + A backfill service in `lib/features/trips/domain/services/trip_day_weather_backfill.dart` decides what to fetch. Given a built `TripStory` and the rows already stored, a day is @@ -139,9 +144,10 @@ fetched only when all of these hold: Each qualifying day is fetched through the existing `WeatherService.fetchWeather` at local noon with `useLocationTimezone: true`, -matching what `surfaceDayWeatherProvider` does today. Requests run with a -small concurrency cap rather than all at once, since widening from surface -days to all dive-free days raises the first-view request count. +matching what `surfaceDayWeatherProvider` does today. Requests run +sequentially rather than all at once, since widening from surface days to all +dive-free days raises the first-view request count. Sequential also means rows +land progressively, so headers fill in as results arrive. **A fetch that returns null, or returns a `WeatherData` with no usable field, writes no row and is retried on the next view.** This mirrors the rule already @@ -213,8 +219,8 @@ Tests come first, per the project's TDD rule. when two branches write the same number. Re-grep after every merge from main and re-run `test/core/database/`. - **Request volume on first view.** Widening from surface days to all - dive-free days increases first-view requests for a trip. Bounded by the - concurrency cap, and one-time per day per trip. + dive-free days increases first-view requests for a trip. Bounded by + sequential fetching, and one-time per day per trip. - **Coordinate drift.** Stored rows record the coordinate used. If a site later moves, the stored weather is not re-fetched. Accepted: the row records what it was fetched for, and a day's weather is not materially sensitive to From bbd827ac6872ed18bb68644f2e54c36c16d1d92f Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:14:19 -0400 Subject: [PATCH 03/18] feat(db): add trip_day_weather table at schema v168 One row per trip day, holding fetched historical weather for days whose dives supply none. A separate table rather than columns on an existing row so an automatic derived write never conflicts with hand-entered data under row-level HLC resolution. The unique index on (trip_id, date) makes the day the identity, so two devices that both fetch it converge. --- lib/core/database/database.dart | 109 ++++++++++++- .../domain/entities/trip_day_weather.dart | 152 ++++++++++++++++++ .../migration_v168_trip_day_weather_test.dart | 140 ++++++++++++++++ 3 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 lib/features/trips/domain/entities/trip_day_weather.dart create mode 100644 test/core/database/migration_v168_trip_day_weather_test.dart diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 99b322ede9..d6ef5413cd 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -136,6 +136,60 @@ class TripItineraryDays extends Table { Set get primaryKey => {id}; } +/// Fetched historical weather for one trip day, stored for days whose dives +/// supply no weather of their own (surface days and dive-free itinerary days). +/// +/// A separate table rather than columns on `trips` or `trip_itinerary_days` on +/// purpose: HLC conflicts resolve per row, so 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. Weather owns its own +/// row and its own clock. +/// +/// Metric storage throughout (celsius, m/s, bar); conversion to the diver's +/// units happens at display time. +class TripDayWeather extends Table { + // coverage:ignore-start + TextColumn get id => text()(); + TextColumn get tripId => text().references(Trips, #id)(); + + /// Local midnight for the day, as epoch milliseconds (the convention + /// TripItineraryDays.date is written with). + IntColumn get date => integer()(); + + /// The coordinates the lookup actually used, so a row records what it was + /// fetched for even if the trip's sites later move. + RealColumn get latitude => real()(); + RealColumn get longitude => real()(); + + RealColumn get airTemp => real().nullable()(); // celsius + TextColumn get cloudCover => text().nullable()(); // enum: CloudCover.name + TextColumn get precipitation => + text().nullable()(); // enum: Precipitation.name + RealColumn get windSpeed => real().nullable()(); // m/s + TextColumn get windDirection => + text().nullable()(); // enum: CurrentDirection.name + RealColumn get humidity => real().nullable()(); // 0-100 + RealColumn get surfacePressure => real().nullable()(); // bar + + /// Raw WMO weather code, kept so the description renders in the diver's + /// locale at display time rather than frozen as English prose at fetch time. + IntColumn get weatherCode => integer().nullable()(); + + TextColumn get weatherSource => + text().withDefault(const Constant('openMeteo'))(); + IntColumn get fetchedAt => integer()(); + IntColumn get createdAt => integer()(); + IntColumn get updatedAt => integer()(); + + /// Hybrid Logical Clock for cross-device conflict resolution + /// (nullable: rows written before HLC rollout fall back to updatedAt). + TextColumn get hlc => text().nullable()(); + // coverage:ignore-end + + @override + Set get primaryKey => {id}; +} + /// Reusable checklist templates for trip planning (issue #164) class ChecklistTemplates extends Table { // coverage:ignore-start @@ -3141,6 +3195,7 @@ String legacyDataSourceId(String diveId) => '$kLegacyDataSourceIdPrefix$diveId'; // Liveaboard tracking (v2.0) LiveaboardDetailRecords, TripItineraryDays, + TripDayWeather, ChecklistTemplates, ChecklistTemplateItems, TripChecklistItems, @@ -3180,7 +3235,7 @@ class AppDatabase extends _$AppDatabase { /// The current schema version as a static constant so that pre-open checks /// (e.g. version-mismatch guard) can reference it without an instance. - static const int currentSchemaVersion = 164; + static const int currentSchemaVersion = 168; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3476,6 +3531,11 @@ class AppDatabase extends _$AppDatabase { // media item in the dive when its capture time is wrong (issue #1090). // Renumbered from 162, which #731 landed past while this branch was open. 164, + // v168: trip_day_weather, fetched historical weather for trip days whose + // dives supply none. 165, 166, and 167 are claimed by open PRs (#1290, + // #1300, #1276), so this ladder is non-contiguous by design; the audit + // asserts monotonic, unique, and scalar == max, never contiguous. + 168, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -3929,6 +3989,41 @@ class AppDatabase extends _$AppDatabase { /// v129: quality_findings table for the Data Quality Assistant. /// Idempotent so it is safe to call from both onUpgrade and the /// beforeOpen backstop. + /// v168: fetched per-day trip weather. + /// + /// Idempotent, so it doubles as the beforeOpen backstop for a database + /// stranded at 168 by a parallel branch that never created the table. + Future _assertTripDayWeatherSchema() async { + await customStatement(''' + CREATE TABLE IF NOT EXISTS trip_day_weather ( + id TEXT NOT NULL PRIMARY KEY, + trip_id TEXT NOT NULL REFERENCES trips (id), + date INTEGER NOT NULL, + latitude REAL NOT NULL, + longitude REAL NOT NULL, + air_temp REAL, + cloud_cover TEXT, + precipitation TEXT, + wind_speed REAL, + wind_direction TEXT, + humidity REAL, + surface_pressure REAL, + weather_code INTEGER, + weather_source TEXT NOT NULL DEFAULT 'openMeteo', + fetched_at INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + hlc TEXT + ) + '''); + // The day is the identity: two devices that both fetch it must converge + // on one row rather than accumulating duplicates. + await customStatement( + 'CREATE UNIQUE INDEX IF NOT EXISTS idx_trip_day_weather_trip_date ' + 'ON trip_day_weather (trip_id, date)', + ); + } + Future _assertQualityFindingsSchema() async { await customStatement(''' CREATE TABLE IF NOT EXISTS quality_findings ( @@ -8615,6 +8710,12 @@ class AppDatabase extends _$AppDatabase { await _assertMediaManualElapsedColumn(); } if (from < 164) await reportProgress(); + // v168: trip_day_weather, fetched per-day weather for trip days whose + // dives supply none. + if (from < 168) { + await _assertTripDayWeatherSchema(); + } + if (from < 168) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8819,6 +8920,12 @@ class AppDatabase extends _$AppDatabase { // media row mapper reads it on every hydration. await _assertMediaManualElapsedColumn(); + // v168 backstop: re-assert trip_day_weather (same parallel-branch + // version-collision self-heal). The helper is CREATE TABLE IF NOT + // EXISTS plus CREATE UNIQUE INDEX IF NOT EXISTS, so it is a no-op on + // every open after the first. + await _assertTripDayWeatherSchema(); + // v145 backstop: re-assert the gps_tracks provenance and trim columns. await _assertGpsTrackColumns(); diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart new file mode 100644 index 0000000000..f9175b7520 --- /dev/null +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -0,0 +1,152 @@ +import 'package:equatable/equatable.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; + +/// Stored historical weather for one trip day. +/// +/// Written only for days whose dives supply no weather of their own; a day +/// with dive-logged weather renders that instead, because it is what the +/// diver actually recorded. +/// +/// Metric throughout (celsius, m/s, bar). Conversion to the diver's units +/// happens at display time. +class TripDayWeather extends Equatable { + final String id; + final String tripId; + + /// Local midnight for the day this describes. + final DateTime date; + + /// The coordinates the lookup used. + final double latitude; + final double longitude; + + final double? airTemp; // celsius + final CloudCover? cloudCover; + final Precipitation? precipitation; + final double? windSpeed; // m/s + final CurrentDirection? windDirection; + final double? humidity; // 0-100 + final double? surfacePressure; // bar + + /// Raw WMO weather code (0 clear, 61 rain, 95 thunderstorm, ...), kept so + /// the description can be rendered in the diver's locale at display time. + final int? weatherCode; + + final WeatherSource weatherSource; + final DateTime fetchedAt; + final DateTime createdAt; + final DateTime updatedAt; + + const TripDayWeather({ + required this.id, + required this.tripId, + required this.date, + required this.latitude, + required this.longitude, + this.airTemp, + this.cloudCover, + this.precipitation, + this.windSpeed, + this.windDirection, + this.humidity, + this.surfacePressure, + this.weatherCode, + this.weatherSource = WeatherSource.openMeteo, + required this.fetchedAt, + required this.createdAt, + required this.updatedAt, + }); + + /// True when at least one field the day header can render is present. + /// + /// A row with only wind and humidity would render as nothing and would + /// suppress the retry that a later archive update would satisfy, so it is + /// not worth storing. + bool get hasRenderableWeather => + airTemp != null || cloudCover != null || precipitation != null; + + /// The compact view model the day header consumes. + TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); + + TripDayWeather copyWith({ + String? id, + String? tripId, + DateTime? date, + double? latitude, + double? longitude, + Object? airTemp = _undefined, + Object? cloudCover = _undefined, + Object? precipitation = _undefined, + Object? windSpeed = _undefined, + Object? windDirection = _undefined, + Object? humidity = _undefined, + Object? surfacePressure = _undefined, + Object? weatherCode = _undefined, + WeatherSource? weatherSource, + DateTime? fetchedAt, + DateTime? createdAt, + DateTime? updatedAt, + }) { + return TripDayWeather( + id: id ?? this.id, + tripId: tripId ?? this.tripId, + date: date ?? this.date, + latitude: latitude ?? this.latitude, + longitude: longitude ?? this.longitude, + airTemp: airTemp == _undefined ? this.airTemp : airTemp as double?, + cloudCover: cloudCover == _undefined + ? this.cloudCover + : cloudCover as CloudCover?, + precipitation: precipitation == _undefined + ? this.precipitation + : precipitation as Precipitation?, + windSpeed: windSpeed == _undefined + ? this.windSpeed + : windSpeed as double?, + windDirection: windDirection == _undefined + ? this.windDirection + : windDirection as CurrentDirection?, + humidity: humidity == _undefined ? this.humidity : humidity as double?, + surfacePressure: surfacePressure == _undefined + ? this.surfacePressure + : surfacePressure as double?, + weatherCode: weatherCode == _undefined + ? this.weatherCode + : weatherCode as int?, + weatherSource: weatherSource ?? this.weatherSource, + fetchedAt: fetchedAt ?? this.fetchedAt, + createdAt: createdAt ?? this.createdAt, + updatedAt: updatedAt ?? this.updatedAt, + ); + } + + @override + List get props => [ + id, + tripId, + date, + latitude, + longitude, + airTemp, + cloudCover, + precipitation, + windSpeed, + windDirection, + humidity, + surfacePressure, + weatherCode, + weatherSource, + fetchedAt, + createdAt, + updatedAt, + ]; +} + +// Sentinel value for distinguishing null from undefined in copyWith +const _undefined = Object(); diff --git a/test/core/database/migration_v168_trip_day_weather_test.dart b/test/core/database/migration_v168_trip_day_weather_test.dart new file mode 100644 index 0000000000..bf505f7ad1 --- /dev/null +++ b/test/core/database/migration_v168_trip_day_weather_test.dart @@ -0,0 +1,140 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v168 adds `trip_day_weather`: fetched historical weather for trip days +/// whose dives supply none of their own (surface days and dive-free itinerary +/// days). Its own table rather than columns on `trips` or `trip_itinerary_days` +/// because HLC conflicts resolve per row, and an automatic derived write must +/// not race the diver's hand edits on the same row. +NativeDatabase _dbAt164() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 164'); + rawDb.execute(''' + CREATE TABLE trips ( + id TEXT NOT NULL PRIMARY KEY, + name TEXT NOT NULL, + start_date INTEGER NOT NULL, + end_date INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute( + "INSERT INTO trips (id, name, start_date, end_date, created_at, " + "updated_at) VALUES ('t1', 'Bonaire', 0, 0, 0, 0)", + ); + }, + ); +} + +Future> _columnsOf(AppDatabase db, String table) async { + final cols = await db.customSelect("PRAGMA table_info('$table')").get(); + return cols.map((c) => c.read('name')).toSet(); +} + +void main() { + test('v168 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(168)); + expect(AppDatabase.migrationVersions, contains(168)); + }); + + test('a fresh database has trip_day_weather with every column', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + expect( + await _columnsOf(db, 'trip_day_weather'), + containsAll({ + 'id', + 'trip_id', + 'date', + 'latitude', + 'longitude', + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + 'weather_source', + 'fetched_at', + 'created_at', + 'updated_at', + 'hlc', + }), + ); + }); + + test('the weather payload columns are all nullable', () async { + // A fetch that resolves only some fields must still store a row; every + // reading is independently optional. + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('trip_day_weather')") + .get(); + for (final name in [ + 'air_temp', + 'cloud_cover', + 'precipitation', + 'wind_speed', + 'wind_direction', + 'humidity', + 'surface_pressure', + 'weather_code', + ]) { + final column = cols.firstWhere((c) => c.read('name') == name); + expect(column.read('notnull'), 0, reason: '$name must be nullable'); + } + }); + + test('one row per trip and date', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + await db.customStatement('PRAGMA foreign_keys = OFF'); + + Future insert(String id) => db.customStatement( + 'INSERT INTO trip_day_weather ' + '(id, trip_id, date, latitude, longitude, weather_source, ' + 'fetched_at, created_at, updated_at) ' + "VALUES ('$id', 't1', 1000, 1.0, 2.0, 'openMeteo', 1, 1, 1)", + ); + + await insert('a'); + // Two devices that both fetch the same day must converge on one row + // rather than accumulating duplicates. + await expectLater(insert('b'), throwsA(anything)); + }); + + test('a database at v164 gains the table and keeps its rows', () async { + final db = AppDatabase(_dbAt164()); + addTearDown(db.close); + + expect(await _columnsOf(db, 'trip_day_weather'), isNotEmpty); + final trip = await db + .customSelect("SELECT name FROM trips WHERE id = 't1'") + .getSingle(); + expect(trip.read('name'), 'Bonaire'); + }); + + test('a database stranded at a parallel-branch v168 gains the table via ' + 'beforeOpen', () async { + // Stamped AT 168 but without the table: the onUpgrade block never runs, + // so only the beforeOpen backstop can create it. + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + expect(await _columnsOf(db, 'trip_day_weather'), isNotEmpty); + }); +} From 8170b651a05e1eaf86f6ac65d4fe121c66dcc482 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:17:05 -0400 Subject: [PATCH 04/18] feat(trips): add TripDayWeatherRepository Upsert is keyed by (trip, date) rather than by id, reusing an existing row's id when the day already has one, so two devices that both fetch the same day converge instead of colliding on the unique index. Deleting a trip takes its weather rows with it, logged for sync like every other child record. --- .../trip_day_weather_repository.dart | 161 ++++++++++++++++ .../data/repositories/trip_repository.dart | 2 + .../trip_day_weather_repository_test.dart | 179 ++++++++++++++++++ 3 files changed, 342 insertions(+) create mode 100644 lib/features/trips/data/repositories/trip_day_weather_repository.dart create mode 100644 test/features/trips/data/repositories/trip_day_weather_repository_test.dart diff --git a/lib/features/trips/data/repositories/trip_day_weather_repository.dart b/lib/features/trips/data/repositories/trip_day_weather_repository.dart new file mode 100644 index 0000000000..399645d5d8 --- /dev/null +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -0,0 +1,161 @@ +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + as domain; + +/// Reads and writes stored per-day trip weather. +/// +/// The day is the identity, not the row id: `upsert` replaces any existing +/// row for the same (trip, date), so two devices that both fetch the same day +/// converge on one row rather than accumulating duplicates. +class TripDayWeatherRepository { + AppDatabase get _db => DatabaseService.instance.database; + final SyncRepository _syncRepository = SyncRepository(); + final _log = LoggerService.forClass(TripDayWeatherRepository); + + /// Emits whenever `trip_day_weather` changes, so the display provider + /// refreshes after a backfill write or a sync import. + Stream watchWeatherChanges() => + _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); + + /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + Future> getForTrip(String tripId) async { + try { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + return {for (final row in rows) row.date: _mapRow(row)}; + } catch (e, stackTrace) { + _log.error( + 'Failed to read weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Insert or replace one day's weather. + Future upsert(domain.TripDayWeather weather) async { + try { + final now = DateTime.now().millisecondsSinceEpoch; + final dateMillis = weather.date.millisecondsSinceEpoch; + + // Reuse the stored row's id when the day already has one: a peer may + // have written its own uuid for this day, and replacing it under a new + // id would violate the unique index and orphan the peer's sync record. + final existing = + await (_db.select(_db.tripDayWeather)..where( + (t) => + t.tripId.equals(weather.tripId) & t.date.equals(dateMillis), + )) + .getSingleOrNull(); + final id = existing?.id ?? weather.id; + + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherCompanion( + id: Value(id), + tripId: Value(weather.tripId), + date: Value(dateMillis), + latitude: Value(weather.latitude), + longitude: Value(weather.longitude), + airTemp: Value(weather.airTemp), + cloudCover: Value(weather.cloudCover?.name), + precipitation: Value(weather.precipitation?.name), + windSpeed: Value(weather.windSpeed), + windDirection: Value(weather.windDirection?.name), + humidity: Value(weather.humidity), + surfacePressure: Value(weather.surfacePressure), + weatherCode: Value(weather.weatherCode), + weatherSource: Value(weather.weatherSource.name), + fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), + createdAt: Value( + existing?.createdAt ?? weather.createdAt.millisecondsSinceEpoch, + ), + updatedAt: Value(now), + ), + ); + + await _syncRepository.markRecordPending( + entityType: 'tripDayWeather', + recordId: id, + localUpdatedAt: now, + ); + SyncEventBus.notifyLocalChange(); + } catch (e, stackTrace) { + _log.error( + 'Failed to store weather for trip: ${weather.tripId}', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + /// Delete every stored day for a trip, logging each id for sync. + Future deleteByTripId(String tripId) async { + try { + final existing = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + if (existing.isEmpty) return; + + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).go(); + + for (final row in existing) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: row.id, + ); + } + SyncEventBus.notifyLocalChange(); + + _log.info('Deleted ${existing.length} weather days for trip: $tripId'); + } catch (e, stackTrace) { + _log.error( + 'Failed to delete weather for trip: $tripId', + error: e, + stackTrace: stackTrace, + ); + rethrow; + } + } + + domain.TripDayWeather _mapRow(TripDayWeatherData row) { + return domain.TripDayWeather( + id: row.id, + tripId: row.tripId, + date: DateTime.fromMillisecondsSinceEpoch(row.date), + latitude: row.latitude, + longitude: row.longitude, + airTemp: row.airTemp, + cloudCover: row.cloudCover == null + ? null + : CloudCover.values.byName(row.cloudCover!), + precipitation: row.precipitation == null + ? null + : Precipitation.values.byName(row.precipitation!), + windSpeed: row.windSpeed, + windDirection: row.windDirection == null + ? null + : CurrentDirection.values.byName(row.windDirection!), + humidity: row.humidity, + surfacePressure: row.surfacePressure, + weatherCode: row.weatherCode, + weatherSource: WeatherSource.values.byName(row.weatherSource), + fetchedAt: DateTime.fromMillisecondsSinceEpoch(row.fetchedAt), + createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), + updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), + ); + } +} diff --git a/lib/features/trips/data/repositories/trip_repository.dart b/lib/features/trips/data/repositories/trip_repository.dart index b1d3478910..0f97eb5cca 100644 --- a/lib/features/trips/data/repositories/trip_repository.dart +++ b/lib/features/trips/data/repositories/trip_repository.dart @@ -12,6 +12,7 @@ import 'package:submersion/features/checklists/data/repositories/trip_checklist_ import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; import 'package:submersion/features/trips/data/repositories/itinerary_day_repository.dart'; import 'package:submersion/features/trips/data/repositories/liveaboard_details_repository.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; import 'package:submersion/features/trips/domain/entities/dive_candidate.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart' as domain; @@ -265,6 +266,7 @@ class TripRepository { await LiveaboardDetailsRepository().deleteByTripId(id); await ItineraryDayRepository().deleteByTripId(id); await TripChecklistRepository().deleteByTripId(id); + await TripDayWeatherRepository().deleteByTripId(id); // Remove trip association from dives (nullable FK) await _db.customUpdate( diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart new file mode 100644 index 0000000000..3686260aa5 --- /dev/null +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -0,0 +1,179 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/data/repositories/trip_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late TripDayWeatherRepository repository; + late TripRepository tripRepository; + late String testTripId; + late String otherTripId; + + final day1 = DateTime(2026, 3, 8); + final day2 = DateTime(2026, 3, 9); + + Trip createTestTrip({String name = 'Test Trip'}) { + final now = DateTime.now(); + return Trip( + id: '', + name: name, + startDate: day1, + endDate: DateTime(2026, 3, 14), + createdAt: now, + updatedAt: now, + ); + } + + TripDayWeather sample({ + String id = 'w1', + String? tripId, + DateTime? date, + double? airTemp = 21.5, + CloudCover? cloudCover = CloudCover.clear, + }) { + final now = DateTime(2026, 3, 15); + return TripDayWeather( + id: id, + tripId: tripId ?? testTripId, + date: date ?? day1, + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: cloudCover, + windSpeed: 6.5, + windDirection: CurrentDirection.north, + humidity: 70, + surfacePressure: 1.011, + weatherCode: 0, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + } + + setUp(() async { + await setUpTestDatabase(); + repository = TripDayWeatherRepository(); + tripRepository = TripRepository(); + + // Two trips, to prove the queries are scoped. trip_id is a non-nullable + // FK and beforeOpen turns foreign keys on. + testTripId = (await tripRepository.createTrip( + createTestTrip(name: 'Weather Test Trip'), + )).id; + otherTripId = (await tripRepository.createTrip( + createTestTrip(name: 'Other Trip'), + )).id; + }); + + tearDown(() async { + await tearDownTestDatabase(); + }); + + group('TripDayWeatherRepository', () { + test('getForTrip is empty before anything is stored', () async { + expect(await repository.getForTrip(testTripId), isEmpty); + }); + + test('upsert then read back, keyed by date millis', () async { + await repository.upsert(sample()); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + final row = stored[day1.millisecondsSinceEpoch]!; + expect(row.airTemp, 21.5); + expect(row.cloudCover, CloudCover.clear); + expect(row.windDirection, CurrentDirection.north); + expect(row.weatherCode, 0); + expect(row.weatherSource, WeatherSource.openMeteo); + expect(row.latitude, 12.16); + expect(row.date, day1); + }); + + test('a null payload field round-trips as null', () async { + await repository.upsert(sample(airTemp: null, cloudCover: null)); + + final row = (await repository.getForTrip( + testTripId, + ))[day1.millisecondsSinceEpoch]!; + + expect(row.airTemp, isNull); + expect(row.cloudCover, isNull); + expect(row.precipitation, isNull); + }); + + test('upserting the same day twice keeps one row', () async { + await repository.upsert(sample()); + // A different id for the same day: the day is the identity, so this + // must replace rather than accumulate. + await repository.upsert(sample(id: 'w2', airTemp: 25)); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + }); + + test('two different days both persist', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', date: day2, airTemp: 19)); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(2)); + expect(stored[day2.millisecondsSinceEpoch]!.airTemp, 19); + }); + + test('getForTrip is scoped to one trip', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + expect(await repository.getForTrip(testTripId), hasLength(1)); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('deleteByTripId removes only that trip rows', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + await repository.deleteByTripId(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('deleteByTripId on a trip with no weather is a no-op', () async { + await repository.deleteByTripId(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + }); + + test('deleting a trip takes its weather rows with it', () async { + await repository.upsert(sample()); + await repository.upsert(sample(id: 'w2', tripId: otherTripId)); + + await tripRepository.deleteTrip(testTripId); + + expect(await repository.getForTrip(testTripId), isEmpty); + expect(await repository.getForTrip(otherTripId), hasLength(1)); + }); + + test('watchWeatherChanges emits after a write', () async { + final emissions = []; + final subscription = repository.watchWeatherChanges().listen( + emissions.add, + ); + addTearDown(subscription.cancel); + + await repository.upsert(sample()); + await Future.delayed(Duration.zero); + + expect(emissions, isNotEmpty); + }); + }); +} From 150944630719e08ff9caf2c57a342b367343435c Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:23:21 -0400 Subject: [PATCH 05/18] feat(sync): replicate trip day weather Registers tripDayWeather 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. The FK completeness guard needed the table too, or its foreign key would have gone unverified. --- .../data/repositories/sync_repository.dart | 1 + .../services/sync/sync_data_serializer.dart | 56 +++++++++ lib/core/services/sync/sync_service.dart | 7 ++ .../sync_parent_refs_completeness_test.dart | 1 + .../sync/trip_day_weather_sync_test.dart | 107 ++++++++++++++++++ 5 files changed, 172 insertions(+) create mode 100644 test/core/services/sync/trip_day_weather_sync_test.dart diff --git a/lib/core/data/repositories/sync_repository.dart b/lib/core/data/repositories/sync_repository.dart index c7f928a507..2ff9115bdc 100644 --- a/lib/core/data/repositories/sync_repository.dart +++ b/lib/core/data/repositories/sync_repository.dart @@ -47,6 +47,7 @@ class SyncRepository { 'trips': (table: 'trips', pk: 'id'), 'liveaboardDetails': (table: 'liveaboard_detail_records', pk: 'id'), 'itineraryDays': (table: 'trip_itinerary_days', pk: 'id'), + 'tripDayWeather': (table: 'trip_day_weather', pk: 'id'), 'checklistTemplates': (table: 'checklist_templates', pk: 'id'), 'checklistTemplateItems': (table: 'checklist_template_items', pk: 'id'), 'tripChecklistItems': (table: 'trip_checklist_items', pk: 'id'), diff --git a/lib/core/services/sync/sync_data_serializer.dart b/lib/core/services/sync/sync_data_serializer.dart index a942e36284..43e2b57bd4 100644 --- a/lib/core/services/sync/sync_data_serializer.dart +++ b/lib/core/services/sync/sync_data_serializer.dart @@ -247,6 +247,7 @@ class SyncData { final List> trips; final List> liveaboardDetails; final List> itineraryDays; + final List> tripDayWeather; final List> checklistTemplates; final List> checklistTemplateItems; final List> tripChecklistItems; @@ -322,6 +323,7 @@ class SyncData { this.trips = const [], this.liveaboardDetails = const [], this.itineraryDays = const [], + this.tripDayWeather = const [], this.checklistTemplates = const [], this.checklistTemplateItems = const [], this.tripChecklistItems = const [], @@ -398,6 +400,7 @@ class SyncData { 'trips': trips, 'liveaboardDetails': liveaboardDetails, 'itineraryDays': itineraryDays, + 'tripDayWeather': tripDayWeather, 'checklistTemplates': checklistTemplates, 'checklistTemplateItems': checklistTemplateItems, 'tripChecklistItems': tripChecklistItems, @@ -475,6 +478,7 @@ class SyncData { trips: _parseList(json['trips']), liveaboardDetails: _parseList(json['liveaboardDetails']), itineraryDays: _parseList(json['itineraryDays']), + tripDayWeather: _parseList(json['tripDayWeather']), checklistTemplates: _parseList(json['checklistTemplates']), checklistTemplateItems: _parseList(json['checklistTemplateItems']), tripChecklistItems: _parseList(json['tripChecklistItems']), @@ -764,6 +768,7 @@ class SyncDataSerializer { blob: false, full: null, ), + (key: 'tripDayWeather', table: _db.tripDayWeather, blob: false, full: null), ( key: 'checklistTemplates', table: _db.checklistTemplates, @@ -1269,6 +1274,10 @@ class SyncDataSerializer { 'itineraryDays', () => _exportItineraryDays(hlcSince), ), + tripDayWeather: await _safeExport( + 'tripDayWeather', + () => _exportTripDayWeather(hlcSince), + ), checklistTemplates: await _safeExport( 'checklistTemplates', () => _exportChecklistTemplates(hlcSince), @@ -1709,6 +1718,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.equals(recordId))).getSingleOrNull(); return row?.toJson(); + case 'tripDayWeather': + final row = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).getSingleOrNull(); + return row?.toJson(); case 'checklistTemplates': final row = await (_db.select( _db.checklistTemplates, @@ -2034,6 +2048,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.isIn(idList))).get(); return {for (final r in rows) r.id: r.toJson()}; + case 'tripDayWeather': + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.isIn(idList))).get(); + return {for (final r in rows) r.id: r.toJson()}; case 'checklistTemplates': final rows = await (_db.select( _db.checklistTemplates, @@ -2596,6 +2615,13 @@ class SyncDataSerializer { TripItineraryDay.fromJson(data).toCompanion(false), ); return; + case 'tripDayWeather': + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherData.fromJson(data).toCompanion(false), + ); + return; case 'checklistTemplates': await _db .into(_db.checklistTemplates) @@ -3203,6 +3229,16 @@ class SyncDataSerializer { ), ); return; + case 'tripDayWeather': + await _db.batch( + (b) => b.insertAllOnConflictUpdate( + _db.tripDayWeather, + records + .map((r) => TripDayWeatherData.fromJson(r).toCompanion(false)) + .toList(), + ), + ); + return; case 'checklistTemplates': await _db.batch( (b) => b.insertAllOnConflictUpdate( @@ -3680,6 +3716,8 @@ class SyncDataSerializer { ); case 'itineraryDays': return plain(_db.tripItineraryDays, _db.tripItineraryDays.id); + case 'tripDayWeather': + return plain(_db.tripDayWeather, _db.tripDayWeather.id); case 'checklistTemplates': return plain(_db.checklistTemplates, _db.checklistTemplates.id); case 'checklistTemplateItems': @@ -3913,6 +3951,8 @@ class SyncDataSerializer { return _db.liveaboardDetailRecords; case 'itineraryDays': return _db.tripItineraryDays; + case 'tripDayWeather': + return _db.tripDayWeather; case 'checklistTemplates': return _db.checklistTemplates; case 'checklistTemplateItems': @@ -4224,6 +4264,11 @@ class SyncDataSerializer { _db.tripItineraryDays, )..where((t) => t.id.equals(recordId))).go(); return; + case 'tripDayWeather': + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.equals(recordId))).go(); + return; case 'checklistTemplates': await (_db.delete( _db.checklistTemplates, @@ -4900,6 +4945,17 @@ class SyncDataSerializer { return rows.map((r) => r.toJson()).toList(); } + Future>> _exportTripDayWeather( + String? hlcSince, + ) async { + final query = _db.select(_db.tripDayWeather); + if (hlcSince != null) { + query.where((t) => t.hlc.isBiggerThanValue(hlcSince)); + } + final rows = await query.get(); + return rows.map((r) => r.toJson()).toList(); + } + Future>> _exportChecklistTemplates( String? hlcSince, ) async { diff --git a/lib/core/services/sync/sync_service.dart b/lib/core/services/sync/sync_service.dart index 5fd1999a7f..89ea5b0d73 100644 --- a/lib/core/services/sync/sync_service.dart +++ b/lib/core/services/sync/sync_service.dart @@ -1210,6 +1210,11 @@ class SyncService { records: data.itineraryDays, hasUpdatedAt: true, ), + ( + type: 'tripDayWeather', + records: data.tripDayWeather, + hasUpdatedAt: true, + ), ( type: 'checklistTemplates', records: data.checklistTemplates, @@ -1957,6 +1962,7 @@ class SyncService { 'trips': true, 'liveaboardDetails': true, 'itineraryDays': true, + 'tripDayWeather': true, 'checklistTemplates': true, 'checklistTemplateItems': true, 'tripChecklistItems': true, @@ -2118,6 +2124,7 @@ class SyncService { 'siteFeatures': [(field: 'siteId', parent: 'diveSites', nullable: false)], 'liveaboardDetails': [(field: 'tripId', parent: 'trips', nullable: false)], 'itineraryDays': [(field: 'tripId', parent: 'trips', nullable: false)], + 'tripDayWeather': [(field: 'tripId', parent: 'trips', nullable: false)], 'checklistTemplateItems': [ (field: 'templateId', parent: 'checklistTemplates', nullable: false), ], diff --git a/test/core/services/sync/sync_parent_refs_completeness_test.dart b/test/core/services/sync/sync_parent_refs_completeness_test.dart index 7b97c84141..0626edf0e3 100644 --- a/test/core/services/sync/sync_parent_refs_completeness_test.dart +++ b/test/core/services/sync/sync_parent_refs_completeness_test.dart @@ -24,6 +24,7 @@ void main() { 'trips': 'trips', 'liveaboard_detail_records': 'liveaboardDetails', 'trip_itinerary_days': 'itineraryDays', + 'trip_day_weather': 'tripDayWeather', 'checklist_templates': 'checklistTemplates', 'checklist_template_items': 'checklistTemplateItems', 'trip_checklist_items': 'tripChecklistItems', diff --git a/test/core/services/sync/trip_day_weather_sync_test.dart b/test/core/services/sync/trip_day_weather_sync_test.dart new file mode 100644 index 0000000000..781fb8f41c --- /dev/null +++ b/test/core/services/sync/trip_day_weather_sync_test.dart @@ -0,0 +1,107 @@ +import 'package:drift/drift.dart' show Value; +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/sync/sync_data_serializer.dart'; +import 'package:submersion/core/services/sync/sync_service.dart'; + +import '../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late SyncDataSerializer serializer; + + setUp(() async { + db = await setUpTestDatabase(); + serializer = SyncDataSerializer(); + await db + .into(db.trips) + .insert( + TripsCompanion.insert( + id: 'trip-1', + name: 'Bonaire', + startDate: 0, + endDate: 0, + createdAt: 1, + updatedAt: 1, + ), + ); + await db + .into(db.tripDayWeather) + .insert( + TripDayWeatherCompanion.insert( + id: 'w-1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8).millisecondsSinceEpoch, + latitude: 12.16, + longitude: -68.28, + airTemp: const Value(24.0), + cloudCover: const Value('clear'), + fetchedAt: 1, + createdAt: 1, + updatedAt: 1, + ), + ); + }); + + tearDown(tearDownTestDatabase); + + test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { + final record = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(record, isNotNull); + expect(record!['airTemp'], 24.0); + expect(record['cloudCover'], 'clear'); + + // A remote edit merges over the local row (LWW payload apply). + await serializer.upsertRecord('tripDayWeather', { + ...record, + 'airTemp': 26.0, + 'updatedAt': 2, + }); + final merged = await serializer.fetchRecord('tripDayWeather', 'w-1'); + expect(merged!['airTemp'], 26.0); + + expect(await serializer.recordIdsFor('tripDayWeather'), contains('w-1')); + + await serializer.deleteRecord('tripDayWeather', 'w-1'); + expect(await serializer.fetchRecord('tripDayWeather', 'w-1'), isNull); + }); + + test('the delta export filters on the row own hlc', () async { + await (db.update( + db.tripDayWeather, + )..where((t) => t.id.equals('w-1'))).write( + const TripDayWeatherCompanion(hlc: Value('2026-08-16T00:00:00.000-0000')), + ); + + Future changesetCount(String? watermark) async { + final payload = await serializer.exportChangeset( + deviceId: 'device-1', + hlcWatermark: watermark, + deletions: const [], + ); + return payload.data.tripDayWeather.length; + } + + // A base carries the row; a watermark newer than it excludes it; an older + // watermark includes it. + expect(await changesetCount(null), 1); + expect(await changesetCount('2026-08-17T00:00:00.000-0000'), 0); + expect(await changesetCount('2026-08-15T00:00:00.000-0000'), 1); + }); + + test('tripDayWeather is registered as an hlc target', () { + // An omission here is silent: _stampHlc no-ops on an unknown entity type, + // the column stays NULL, and the incremental export's hlc > watermark + // filter then excludes the row from every changeset forever. + expect(SyncRepository.hlcTargets.containsKey('tripDayWeather'), isTrue); + expect( + SyncRepository.hlcTargets['tripDayWeather']!.table, + 'trip_day_weather', + ); + }); + + test('tripDayWeather carries an updatedAt flag', () { + expect(SyncService.entityHasUpdatedAt['tripDayWeather'], isTrue); + }); +} From cb1b6289a18582e2a33039396a48a887bf2b105a Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:25:00 -0400 Subject: [PATCH 06/18] feat(trips): decide which trip days need a weather lookup Pure rules over the built story and the rows already stored: dive-logged weather wins, future days have no archive to read, stored days are done, and a day with no mappable point anywhere in the trip has nowhere to ask. Dates normalize to midnight so a stray time component cannot make a day refetch on every view. --- .../services/trip_day_weather_backfill.dart | 76 ++++++ .../trip_day_weather_backfill_test.dart | 222 ++++++++++++++++++ 2 files changed, 298 insertions(+) create mode 100644 lib/features/trips/domain/services/trip_day_weather_backfill.dart create mode 100644 test/features/trips/domain/services/trip_day_weather_backfill_test.dart diff --git a/lib/features/trips/domain/services/trip_day_weather_backfill.dart b/lib/features/trips/domain/services/trip_day_weather_backfill.dart new file mode 100644 index 0000000000..05222ec407 --- /dev/null +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -0,0 +1,76 @@ +import 'package:equatable/equatable.dart'; + +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +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. + final DateTime date; + final double latitude; + final double longitude; + + const TripDayWeatherTarget({ + required this.date, + required this.latitude, + required this.longitude, + }); + + /// The hour to sample. Noon local reads as "the day's weather" far better + /// than the archive's default midnight boundary, which straddles two days. + DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); + + @override + List get props => [date, latitude, longitude]; +} + +/// Decides which trip days need a weather lookup. +/// +/// Pure by design: no database, no network, no clock. Every skip rule is a +/// plain condition over the built story and the rows already stored, which is +/// what makes each one directly testable. +class TripDayWeatherBackfill { + const TripDayWeatherBackfill._(); + + static List targetsFor({ + required TripStory story, + required Map stored, + }) { + final targets = []; + + for (var index = 0; index < story.days.length; index++) { + final day = story.days[index]; + + // A dive that logged weather is the better source: it is what the diver + // recorded. Never override it with a fetched summary. + if (day.weather != null) continue; + + // A historical archive has nothing for a day that has not happened. + if (day.kind == TripStoryDayKind.future) continue; + + // Normalize to midnight: stored rows are keyed on midnight millis, so a + // stray time component would never match and the day would refetch on + // every view. + final date = DateTime(day.date.year, day.date.month, day.date.day); + if (stored.containsKey(date.millisecondsSinceEpoch)) continue; + + // nearestPointForDay walks outward from the day, so a dive-free day + // between two dived days borrows the closer one's coordinates. A trip + // with no mappable point anywhere has nowhere to ask. + final point = story.mapGeometry.nearestPointForDay(index); + if (point == null) continue; + + targets.add( + TripDayWeatherTarget( + date: date, + latitude: point.latitude, + longitude: point.longitude, + ), + ); + } + + return targets; + } +} diff --git a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart new file mode 100644 index 0000000000..70e6563e72 --- /dev/null +++ b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart @@ -0,0 +1,222 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; + +void main() { + Trip trip() => Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 14), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ); + + TripStory storyWith( + List days, { + List points = const [], + }) { + return TripStory( + trip: trip(), + days: days, + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: TripStoryMapGeometry(points: points), + ); + } + + Dive diveWith({double? airTemp}) => + Dive(id: 'd1', dateTime: DateTime(2026, 3, 8, 9), airTemp: airTemp); + + TripStoryDay day({ + required int index, + TripStoryDayKind kind = TripStoryDayKind.past, + List dives = const [], + }) { + return TripStoryDay( + date: DateTime(2026, 3, 8 + index), + dayNumber: index + 1, + kind: kind, + dives: dives, + ); + } + + TripStoryMapPoint pointFor(int dayIndex) => TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: dayIndex, + label: 'Site', + ); + + TripDayWeather storedFor(DateTime date) => TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: date, + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ); + + group('TripDayWeatherBackfill.targetsFor', () { + test('a past day with no dives and a nearby point is a target', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 8)); + expect(targets.single.latitude, 12.16); + expect(targets.single.longitude, -68.28); + // Noon local reads as "the day's weather" far better than the API's + // default midnight boundary. + expect(targets.single.localNoon, DateTime(2026, 3, 8, 12)); + }); + + test('a day whose dives carry weather is skipped', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(airTemp: 26)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a dive-free itinerary day is a target, not just a surface day', () { + // The day has dives with no weather at all, so nothing supplies it. + final story = storyWith( + [ + day(index: 0, dives: [diveWith()]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }); + + test('a future day is skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.future)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('today is not skipped', () { + final story = storyWith( + [day(index: 0, kind: TripStoryDayKind.today)], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }); + + test('a day with a stored row is skipped', () { + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final stored = { + DateTime(2026, 3, 8).millisecondsSinceEpoch: storedFor( + DateTime(2026, 3, 8), + ), + }; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + ); + }); + + test('a day with no map point anywhere in the story is skipped', () { + final story = storyWith([day(index: 0)]); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a day borrows the nearest day point when it has none of its own', () { + // nearestPointForDay walks outward, so day 1 with a point only on day 0 + // is still a target, at day 0's coordinates. + final story = storyWith( + [ + day(index: 0, dives: [diveWith(airTemp: 26)]), + day(index: 1), + ], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets, hasLength(1)); + expect(targets.single.date, DateTime(2026, 3, 9)); + expect(targets.single.latitude, 12.16); + }); + + test('targets come back in day order', () { + final story = storyWith( + [day(index: 0), day(index: 1), day(index: 2)], + points: [pointFor(1)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets.map((t) => t.date).toList(), [ + DateTime(2026, 3, 8), + DateTime(2026, 3, 9), + DateTime(2026, 3, 10), + ]); + }); + + test('a day date with a time component is normalized to midnight', () { + // The story day's date should already be date-only, but a stored row is + // keyed on midnight millis, so a stray time would never match and the + // day would refetch forever. + final story = storyWith( + [ + TripStoryDay( + date: DateTime(2026, 3, 8, 17, 30), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + ], + points: [pointFor(0)], + ); + + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: const {}, + ); + + expect(targets.single.date, DateTime(2026, 3, 8)); + }); + }); +} From 7378397752d23450dd0eb929daa3d5b2770a8d5d Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:27:51 -0400 Subject: [PATCH 07/18] feat(trips): fetch and store trip day weather once The display provider rides the table tick; the backfill deliberately does not, so the rows it writes cannot invalidate it into another pass. A miss writes nothing and is retried on a later view, which is what makes this correct against the archive's publication lag. hasRenderableWeather excludes Precipitation.none: WeatherMapper never returns null precipitation, defaulting a missing reading to none, and weatherIconFor gives none no glyph. Counting it would have stored rows that render as nothing and permanently suppress the retry. --- .../domain/entities/trip_day_weather.dart | 18 +- .../providers/trip_day_weather_providers.dart | 97 +++++++ .../entities/trip_day_weather_test.dart | 107 ++++++++ .../trip_day_weather_providers_test.dart | 244 ++++++++++++++++++ 4 files changed, 461 insertions(+), 5 deletions(-) create mode 100644 lib/features/trips/presentation/providers/trip_day_weather_providers.dart create mode 100644 test/features/trips/domain/entities/trip_day_weather_test.dart create mode 100644 test/features/trips/presentation/providers/trip_day_weather_providers_test.dart diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index f9175b7520..a9ae4be434 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -59,13 +59,21 @@ class TripDayWeather extends Equatable { required this.updatedAt, }); - /// True when at least one field the day header can render is present. + /// True when the day header's badge would actually show something. /// - /// A row with only wind and humidity would render as nothing and would - /// suppress the retry that a later archive update would satisfy, so it is - /// not worth storing. + /// Wind, humidity, and pressure are stored but never drawn, so a row + /// carrying only those renders as nothing and would suppress the retry that + /// a later archive update would satisfy. + /// + /// [Precipitation.none] does not count. `WeatherMapper.mapPrecipitation` + /// returns non-null always, defaulting a missing reading to `none`, so + /// `none` cannot be read as evidence that the fetch resolved anything. It + /// also earns no glyph of its own in `weatherIconFor`, which falls through + /// to cloud cover. bool get hasRenderableWeather => - airTemp != null || cloudCover != null || precipitation != null; + airTemp != null || + cloudCover != null || + (precipitation != null && precipitation != Precipitation.none); /// The compact view model the day header consumes. TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( diff --git a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart new file mode 100644 index 0000000000..2c550b5168 --- /dev/null +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -0,0 +1,97 @@ +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/providers/provider.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/services/trip_day_weather_backfill.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +final tripDayWeatherRepositoryProvider = Provider( + (ref) => TripDayWeatherRepository(), +); + +/// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. +/// +/// 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 +/// ever happened. +final tripDayWeatherProvider = + FutureProvider.family, String>(( + ref, + tripId, + ) async { + final repository = ref.watch(tripDayWeatherRepositoryProvider); + ref.invalidateSelfWhen(repository.watchWeatherChanges()); + return repository.getForTrip(tripId); + }); + +/// Fills the gaps: fetches historical weather for trip days that have none +/// stored and no dive to supply it, then writes what it finds. +/// +/// Deliberately does NOT watch [tripDayWeatherProvider]. Watching the rows it +/// writes would invalidate this provider on every write and start another +/// pass, so it reads the stored rows straight from the repository instead. +/// The story is its only reactive input, which means assigning dives to a +/// trip re-evaluates what is still missing. +/// +/// Not auto-disposed: one pass per trip per provider container lifetime. +final tripDayWeatherBackfillProvider = FutureProvider.family(( + ref, + tripId, +) async { + final story = await ref.watch(tripStoryProvider(tripId).future); + final repository = ref.watch(tripDayWeatherRepositoryProvider); + final service = ref.watch(weatherServiceProvider); + + final stored = await repository.getForTrip(tripId); + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: stored, + ); + if (targets.isEmpty) return; + + const uuid = Uuid(); + + // Sequential on purpose: a two-week trip would otherwise open with a burst + // of parallel requests, and rows landing one at a time let the day headers + // fill in progressively. + for (final target in targets) { + final weather = await service.fetchWeather( + latitude: target.latitude, + longitude: target.longitude, + date: target.date, + entryTime: target.localNoon, + useLocationTimezone: true, + ); + // A miss writes nothing and is retried on the next view, which is what + // makes this correct against the archive's few-day publication lag. + if (weather == null) continue; + + final now = DateTime.now(); + final row = TripDayWeather( + id: uuid.v4(), + tripId: tripId, + date: target.date, + latitude: target.latitude, + longitude: target.longitude, + airTemp: weather.airTemp, + cloudCover: weather.cloudCover, + precipitation: weather.precipitation, + windSpeed: weather.windSpeed, + windDirection: weather.windDirection, + humidity: weather.humidity, + surfacePressure: weather.surfacePressure, + weatherCode: weather.weatherCode, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + + // A row the header could render nothing from is worse than no row: it + // would suppress the retry that a later archive update would satisfy. + if (!row.hasRenderableWeather) continue; + + await repository.upsert(row); + } +}); diff --git a/test/features/trips/domain/entities/trip_day_weather_test.dart b/test/features/trips/domain/entities/trip_day_weather_test.dart new file mode 100644 index 0000000000..5b9334fd53 --- /dev/null +++ b/test/features/trips/domain/entities/trip_day_weather_test.dart @@ -0,0 +1,107 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; + +void main() { + TripDayWeather weather({ + double? airTemp, + CloudCover? cloudCover, + Precipitation? precipitation, + double? windSpeed, + double? humidity, + }) { + final now = DateTime(2026, 3, 9); + return TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + windSpeed: windSpeed, + humidity: humidity, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); + } + + group('hasRenderableWeather', () { + test('air temperature alone counts', () { + expect(weather(airTemp: 24).hasRenderableWeather, isTrue); + }); + + test('cloud cover alone counts', () { + expect( + weather(cloudCover: CloudCover.overcast).hasRenderableWeather, + isTrue, + ); + }); + + test('active precipitation alone counts', () { + expect( + weather(precipitation: Precipitation.rain).hasRenderableWeather, + isTrue, + ); + }); + + test('precipitation none alone does NOT count', () { + // WeatherMapper.mapPrecipitation never returns null: a missing reading + // becomes Precipitation.none, so `none` is not evidence that the fetch + // resolved anything. weatherIconFor gives it no glyph either. + expect( + weather(precipitation: Precipitation.none).hasRenderableWeather, + isFalse, + ); + }); + + test('wind and humidity alone do NOT count', () { + // Stored, but never drawn in the day header badge. + expect( + weather( + windSpeed: 6.5, + humidity: 70, + precipitation: Precipitation.none, + ).hasRenderableWeather, + isFalse, + ); + }); + + test('an empty result does not count', () { + expect(weather().hasRenderableWeather, isFalse); + }); + }); + + group('toStoryWeather', () { + test('carries only the three fields the header renders', () { + final story = weather( + airTemp: 24, + cloudCover: CloudCover.clear, + precipitation: Precipitation.none, + windSpeed: 6.5, + humidity: 70, + ).toStoryWeather(); + + expect(story.airTemp, 24); + expect(story.cloudCover, CloudCover.clear); + expect(story.precipitation, Precipitation.none); + }); + }); + + group('copyWith', () { + test('clears a nullable field when passed null explicitly', () { + final cleared = weather(airTemp: 24).copyWith(airTemp: null); + + expect(cleared.airTemp, isNull); + }); + + test('leaves an untouched field alone', () { + final same = weather(airTemp: 24).copyWith(latitude: 0); + + expect(same.airTemp, 24); + expect(same.latitude, 0); + }); + }); +} diff --git a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart new file mode 100644 index 0000000000..27d1646aea --- /dev/null +++ b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart @@ -0,0 +1,244 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; +import 'package:submersion/features/trips/domain/entities/trip.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story.dart'; +import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_story_providers.dart'; +import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; + +/// A full Open-Meteo hourly payload for one day, in the shape WeatherMapper +/// expects (copied from the deleted surface_day_weather_provider_test). +http.Response weatherResponse({double noonTemp = 29.0, num cloud = 95}) => + http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T09:00', '2026-03-08T12:00'], + 'temperature_2m': [24.0, noonTemp], + 'relative_humidity_2m': [80.0, 70.0], + 'precipitation': [0.0, 0.0], + 'cloud_cover': [10.0, cloud], + 'wind_speed_10m': [8.0, 12.0], + 'wind_direction_10m': [30.0, 45.0], + 'surface_pressure': [1012.0, 1011.0], + 'weathercode': [0, 0], + }, + }), + 200, + ); + +/// Nothing the day header could render: humidity and pressure only. +http.Response unrenderableResponse() => http.Response( + jsonEncode({ + 'hourly': { + 'time': ['2026-03-08T12:00'], + 'temperature_2m': [null], + 'relative_humidity_2m': [70.0], + 'precipitation': [null], + 'cloud_cover': [null], + 'wind_speed_10m': [null], + 'wind_direction_10m': [null], + 'surface_pressure': [1011.0], + 'weathercode': [null], + }, + }), + 200, +); + +/// Records upserts instead of touching a database. +class FakeTripDayWeatherRepository implements TripDayWeatherRepository { + FakeTripDayWeatherRepository({this.stored = const {}}); + + final Map stored; + final List upserts = []; + + @override + Future> getForTrip(String tripId) async => stored; + + @override + Future upsert(TripDayWeather weather) async => upserts.add(weather); + + @override + Future deleteByTripId(String tripId) async {} + + @override + Stream watchWeatherChanges() => const Stream.empty(); +} + +void main() { + /// Two past days with no dives, and one map point on day 0 that day 1 + /// borrows through nearestPointForDay. + TripStory twoDayStory() => TripStory( + trip: Trip( + id: 'trip-1', + name: 'Bonaire', + startDate: DateTime(2026, 3, 8), + endDate: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 1), + updatedAt: DateTime(2026, 3, 1), + ), + days: [ + TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 1, + kind: TripStoryDayKind.past, + ), + TripStoryDay( + date: DateTime(2026, 3, 9), + dayNumber: 2, + kind: TripStoryDayKind.past, + ), + ], + checklist: const TripStoryChecklistSummary(done: 0, total: 0), + mapGeometry: const TripStoryMapGeometry( + points: [ + TripStoryMapPoint( + latitude: 12.16, + longitude: -68.28, + dayIndex: 0, + label: 'Site', + ), + ], + ), + ); + + ProviderContainer containerWith({ + required http.Client client, + required FakeTripDayWeatherRepository repository, + }) { + final container = ProviderContainer( + overrides: [ + weatherHttpClientProvider.overrideWithValue(client), + tripDayWeatherRepositoryProvider.overrideWithValue(repository), + tripStoryProvider('trip-1').overrideWith((ref) async => twoDayStory()), + ], + ); + addTearDown(container.dispose); + return container; + } + + group('tripDayWeatherBackfillProvider', () { + test('fetches each target once and stores the result', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((request) async { + calls++; + // Local noon at the coordinate, not the archive's GMT midnight. + expect(request.url.queryParameters['timezone'], 'auto'); + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + expect(repository.upserts, hasLength(2)); + final first = repository.upserts.first; + expect(first.tripId, 'trip-1'); + expect(first.date, DateTime(2026, 3, 8)); + expect(first.airTemp, 29.0); + expect(first.cloudCover, CloudCover.overcast); + expect(first.latitude, 12.16); + expect(first.longitude, -68.28); + expect(first.weatherSource, WeatherSource.openMeteo); + // The full payload is stored even though the header renders only part. + expect(first.humidity, 70.0); + expect(first.surfacePressure, isNotNull); + expect(repository.upserts[1].date, DateTime(2026, 3, 9)); + }); + + test('a failed fetch writes no row', () async { + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => http.Response('', 500)), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a result with nothing renderable writes no row', () async { + // Storing it would suppress the retry that a later archive update + // would satisfy. + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async => unrenderableResponse()), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(repository.upserts, isEmpty); + }); + + test('a day already stored is not fetched', () async { + final repository = FakeTripDayWeatherRepository( + stored: { + DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: 'trip-1', + date: DateTime(2026, 3, 8), + latitude: 12.16, + longitude: -68.28, + airTemp: 21, + fetchedAt: DateTime(2026, 3, 9), + createdAt: DateTime(2026, 3, 9), + updatedAt: DateTime(2026, 3, 9), + ), + }, + ); + var calls = 0; + final container = containerWith( + client: MockClient((_) async { + calls++; + return weatherResponse(); + }), + repository: repository, + ); + + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 1); + expect(repository.upserts, hasLength(1)); + expect(repository.upserts.single.date, DateTime(2026, 3, 9)); + }); + + test('fetches run one at a time', () async { + final gate = Completer(); + var started = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + started++; + if (started == 1) await gate.future; + return weatherResponse(); + }), + repository: repository, + ); + + final pending = container.read( + tripDayWeatherBackfillProvider('trip-1').future, + ); + await Future.delayed(Duration.zero); + + // A two-week trip must not open with a burst of parallel requests. + expect(started, 1); + + gate.complete(); + await pending; + expect(started, 2); + }); + }); +} From beb30ef55f41d4c4cc1cc590261ee5cf7ffe7e68 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 16:32:48 -0400 Subject: [PATCH 08/18] feat(trips): read trip day weather from the database The day header no longer fetches: it renders whatever weather it is handed, with dive-logged weather still winning. The story view reads the trip's stored rows once and threads each day's row down, watching above the LayoutBuilder because that builder runs at layout time, not build time. surfaceDayWeatherProvider and its per-view Open-Meteo call are deleted. The two new providers default to inert in getBaseOverrides so no widget test reaches a real repository or the network. --- .../surface_day_weather_provider.dart | 58 ----------- .../widgets/story/trip_story_day_header.dart | 25 +++-- .../widgets/story/trip_story_view.dart | 45 +++++---- .../surface_day_weather_provider_test.dart | 98 ------------------- .../story/trip_story_day_header_test.dart | 86 +++++++--------- .../widgets/story/trip_story_view_test.dart | 73 +++++++++----- test/helpers/mock_providers.dart | 11 +++ 7 files changed, 135 insertions(+), 261 deletions(-) delete mode 100644 lib/features/trips/presentation/providers/surface_day_weather_provider.dart delete mode 100644 test/features/trips/presentation/providers/surface_day_weather_provider_test.dart diff --git a/lib/features/trips/presentation/providers/surface_day_weather_provider.dart b/lib/features/trips/presentation/providers/surface_day_weather_provider.dart deleted file mode 100644 index daedc7a4bb..0000000000 --- a/lib/features/trips/presentation/providers/surface_day_weather_provider.dart +++ /dev/null @@ -1,58 +0,0 @@ -import 'package:equatable/equatable.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; - -import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; - -/// One historical-weather lookup for a contentless trip day. -/// -/// Value equality makes this a stable Riverpod family key, so scrolling a day -/// header out of the sliver tree and back in reuses the same in-memory result. -class SurfaceDayWeatherRequest extends Equatable { - final DateTime date; - final double latitude; - final double longitude; - - const SurfaceDayWeatherRequest({ - required this.date, - required this.latitude, - required this.longitude, - }); - - DateTime get localNoon => DateTime(date.year, date.month, date.day, 12); - - @override - List get props => [date, latitude, longitude]; -} - -/// Best-effort historical weather for a trip surface day. -/// -/// Intentionally not auto-disposed: a trip story can repeatedly mount and -/// unmount day slivers while scrolling, and each request should be fetched at -/// most once during the provider container's lifetime. -final surfaceDayWeatherProvider = - FutureProvider.family(( - ref, - request, - ) async { - final weather = await ref - .watch(weatherServiceProvider) - .fetchWeather( - latitude: request.latitude, - longitude: request.longitude, - date: DateTime( - request.date.year, - request.date.month, - request.date.day, - ), - entryTime: request.localNoon, - useLocationTimezone: true, - ); - if (weather == null) return null; - - return TripStoryDayWeather( - airTemp: weather.airTemp, - cloudCover: weather.cloudCover, - precipitation: weather.precipitation, - ); - }); diff --git a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart index fc5f3d30b1..bea6ef77ee 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart @@ -9,7 +9,6 @@ import 'package:submersion/features/settings/presentation/providers/settings_pro import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; import 'package:submersion/features/trips/presentation/helpers/day_type_l10n.dart'; import 'package:submersion/features/trips/presentation/helpers/weather_icon.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; import 'package:submersion/l10n/arb/app_localizations.dart'; import 'package:submersion/l10n/l10n_extension.dart'; @@ -18,8 +17,9 @@ import 'package:submersion/l10n/l10n_extension.dart'; /// (surfaceContainer, one step above the page surface) so the sticky headers /// read as chapter anchors and day cards scroll underneath cleanly. The badge /// echoes the map's primary-colored day pins, tying header and pin together. -/// Days with logged weather get a trailing badge: conditions icon plus air -/// temperature in the diver's temperature unit. +/// Days with logged or stored weather get a trailing badge: conditions icon +/// plus air temperature in the diver's temperature unit. The header never +/// fetches: it renders whatever weather it is handed. /// /// Every day of the trip gets one of these, including surface days (which /// carry no card body at all and are nothing but this header). Presenting a @@ -36,13 +36,12 @@ class TripStoryDayHeader extends ConsumerWidget { static const double minHeight = 52; final TripStoryDay day; - final SurfaceDayWeatherRequest? surfaceWeatherRequest; - const TripStoryDayHeader({ - super.key, - required this.day, - this.surfaceWeatherRequest, - }); + /// Weather stored for this day, passed down by the story view from one + /// per-trip read. Null when nothing is stored yet. + final TripStoryDayWeather? storedWeather; + + const TripStoryDayHeader({super.key, required this.day, this.storedWeather}); @override Widget build(BuildContext context, WidgetRef ref) { @@ -62,11 +61,9 @@ class TripStoryDayHeader extends ConsumerWidget { ...day.siteNames, ].map((part) => part.trim()).where((part) => part.isNotEmpty).toList(); - final request = day.isSurface ? surfaceWeatherRequest : null; - final fetchedWeather = request == null - ? null - : ref.watch(surfaceDayWeatherProvider(request)).asData?.value; - final weather = day.weather ?? fetchedWeather; + // Dive-logged weather always wins: it is what the diver recorded, and a + // stored day summary is only ever a stand-in for days that logged none. + final weather = day.weather ?? storedWeather; final units = UnitFormatter(ref.watch(settingsProvider)); final weatherBadge = _weatherBadge(context, theme, units, weather); diff --git a/lib/features/trips/presentation/widgets/story/trip_story_view.dart b/lib/features/trips/presentation/widgets/story/trip_story_view.dart index 66676226b4..a426887884 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_view.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_view.dart @@ -7,7 +7,8 @@ import 'package:latlong2/latlong.dart'; import 'package:submersion/features/checklists/presentation/widgets/trip_checklist_section.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart'; import 'package:submersion/features/trips/domain/entities/trip_story.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_flight_countdown_card.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_card.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_header.dart'; @@ -154,6 +155,16 @@ class _TripStoryViewState extends ConsumerState @override Widget build(BuildContext context) { + final tripId = widget.story.trip.id; + // One read for the whole story. Watched here rather than inside the + // LayoutBuilder below, whose builder runs at layout time, not build time. + final storedWeather = + ref.watch(tripDayWeatherProvider(tripId)).asData?.value ?? + const {}; + // Fire and forget: the backfill's writes come back through the provider + // above via the table tick, so a row landing re-renders its day header. + ref.watch(tripDayWeatherBackfillProvider(tripId)); + return LayoutBuilder( builder: (context, constraints) { final wide = constraints.maxWidth >= _wideBreakpoint; @@ -172,7 +183,7 @@ class _TripStoryViewState extends ConsumerState siteCount: _siteCount, ), ), - ..._contentSlivers(), + ..._contentSlivers(storedWeather), ], ), ); @@ -195,7 +206,9 @@ class _TripStoryViewState extends ConsumerState Expanded( child: NotificationListener( onNotification: _onScroll, - child: CustomScrollView(slivers: _contentSlivers()), + child: CustomScrollView( + slivers: _contentSlivers(storedWeather), + ), ), ), ], @@ -229,18 +242,16 @@ class _TripStoryViewState extends ConsumerState /// One day chapter: a SliverMainAxisGroup whose pinned header sticks below /// the map until the next day's group pushes it out. Every day gets the same /// header, surface days included - theirs simply has no body under it. - Widget _daySliver(TripStory story, int index, int? todayIndex) { + Widget _daySliver( + TripStory story, + int index, + int? todayIndex, + Map storedWeather, + ) { final day = story.days[index]; - final weatherPoint = day.isSurface - ? story.mapGeometry.nearestPointForDay(index) - : null; - final surfaceWeatherRequest = weatherPoint == null - ? null - : SurfaceDayWeatherRequest( - date: day.date, - latitude: weatherPoint.latitude, - longitude: weatherPoint.longitude, - ); + // 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]; final showTodayDivider = todayIndex != null && index == todayIndex; const divider = SliverPadding( padding: EdgeInsets.symmetric(horizontal: 16), @@ -274,7 +285,7 @@ class _TripStoryViewState extends ConsumerState PinnedHeaderSliver( child: TripStoryDayHeader( day: day, - surfaceWeatherRequest: surfaceWeatherRequest, + storedWeather: stored?.toStoryWeather(), ), ), body, @@ -282,7 +293,7 @@ class _TripStoryViewState extends ConsumerState ); } - List _contentSlivers() { + List _contentSlivers(Map storedWeather) { final story = widget.story; final trip = story.trip; final todayIndex = story.todayIndex; @@ -314,7 +325,7 @@ class _TripStoryViewState extends ConsumerState sliver: SliverToBoxAdapter(child: TripVesselSection(tripId: trip.id)), ), for (var index = 0; index < story.days.length; index++) - _daySliver(story, index, todayIndex), + _daySliver(story, index, todayIndex, storedWeather), if (trip.notes.isNotEmpty) SliverPadding( padding: const EdgeInsets.fromLTRB(16, 0, 16, 16), diff --git a/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart b/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart deleted file mode 100644 index 1f7eda2944..0000000000 --- a/test/features/trips/presentation/providers/surface_day_weather_provider_test.dart +++ /dev/null @@ -1,98 +0,0 @@ -import 'dart:convert'; - -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; -import 'package:submersion/core/constants/enums.dart'; -import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; -import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; - -http.Response weatherResponse() => http.Response( - jsonEncode({ - 'hourly': { - 'time': ['2024-06-15T09:00', '2024-06-15T12:00'], - 'temperature_2m': [24.0, 29.0], - 'relative_humidity_2m': [80.0, 70.0], - 'precipitation': [0.0, 1.0], - 'cloud_cover': [10.0, 95.0], - 'wind_speed_10m': [8.0, 12.0], - 'wind_direction_10m': [30.0, 45.0], - 'surface_pressure': [1012.0, 1011.0], - 'weathercode': [0, 61], - }, - }), - 200, -); - -void main() { - final request = SurfaceDayWeatherRequest( - date: DateTime(2024, 6, 15), - latitude: 12.1, - longitude: -68.2, - ); - - test('fetches local noon and maps compact trip weather', () async { - final client = MockClient((http.Request httpRequest) async { - expect(httpRequest.url.queryParameters['timezone'], 'auto'); - expect(httpRequest.url.queryParameters['start_date'], '2024-06-15'); - return weatherResponse(); - }); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - final weather = await container.read( - surfaceDayWeatherProvider(request).future, - ); - - expect( - weather, - const TripStoryDayWeather( - airTemp: 29, - cloudCover: CloudCover.overcast, - precipitation: Precipitation.lightRain, - ), - ); - }); - - test('caches one request for an equal family key', () async { - var calls = 0; - final client = MockClient((_) async { - calls++; - return weatherResponse(); - }); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - await container.read(surfaceDayWeatherProvider(request).future); - await container.read( - surfaceDayWeatherProvider( - SurfaceDayWeatherRequest( - date: DateTime(2024, 6, 15), - latitude: 12.1, - longitude: -68.2, - ), - ).future, - ); - - expect(calls, 1); - }); - - test('propagates unavailable weather as null', () async { - final client = MockClient((_) async => http.Response('', 500)); - final container = ProviderContainer( - overrides: [weatherHttpClientProvider.overrideWithValue(client)], - ); - addTearDown(container.dispose); - - expect( - await container.read(surfaceDayWeatherProvider(request).future), - isNull, - ); - }); -} diff --git a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart index a7c2789da2..82855221d8 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart @@ -10,7 +10,6 @@ import 'package:submersion/features/dive_log/domain/entities/dive.dart'; import 'package:submersion/features/dive_sites/domain/entities/dive_site.dart'; import 'package:submersion/features/trips/domain/entities/itinerary_day.dart'; import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; -import 'package:submersion/features/trips/presentation/providers/surface_day_weather_provider.dart'; import 'package:submersion/features/trips/presentation/widgets/story/trip_story_day_header.dart'; import 'package:submersion/l10n/arb/app_localizations.dart'; @@ -33,7 +32,7 @@ Future pumpHeader( TripStoryDay day, { double textScale = 1.0, MockSettingsNotifier? settingsNotifier, - SurfaceDayWeatherRequest? surfaceWeatherRequest, + TripStoryDayWeather? storedWeather, List extra = const [], }) async { // The header dates itself with DateFormat.MMMEd(), which resolves against @@ -65,7 +64,7 @@ Future pumpHeader( ).copyWith(textScaler: TextScaler.linear(textScale)), child: TripStoryDayHeader( day: day, - surfaceWeatherRequest: surfaceWeatherRequest, + storedWeather: storedWeather, ), ), ), @@ -200,12 +199,6 @@ void main() { dayNumber: 2, kind: TripStoryDayKind.past, ); - final request = SurfaceDayWeatherRequest( - date: DateTime(2026, 3, 8), - latitude: 12.1, - longitude: -68.2, - ); - testWidgets('gets the same badge and title line as any other day', ( tester, ) async { @@ -264,73 +257,66 @@ void main() { expect(find.byType(Icon), findsNothing); }); - testWidgets('shows fetched weather in the existing badge', (tester) async { + testWidgets('shows stored weather in the existing badge', (tester) async { await pumpHeader( tester, surfaceDay(), - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider(request).overrideWith( - (ref) async => const TripStoryDayWeather( - airTemp: 22, - cloudCover: CloudCover.clear, - ), - ), - ], + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), ); - await tester.pump(); expect(find.byIcon(Icons.wb_sunny_outlined), findsOneWidget); expect(find.text('22°C'), findsOneWidget); }); - testWidgets('fetched temperature respects Fahrenheit', (tester) async { + testWidgets('stored temperature respects Fahrenheit', (tester) async { final settings = MockSettingsNotifier(); await settings.setTemperatureUnit(TemperatureUnit.fahrenheit); await pumpHeader( tester, surfaceDay(), settingsNotifier: settings, - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider( - request, - ).overrideWith((ref) async => const TripStoryDayWeather(airTemp: 22)), - ], + storedWeather: const TripStoryDayWeather(airTemp: 22), ); - await tester.pump(); expect(find.text('71.6°F'), findsOneWidget); }); - testWidgets('loading and failed weather stay badge-free', (tester) async { - final pending = Completer(); - await pumpHeader( - tester, - surfaceDay(), - surfaceWeatherRequest: request, - extra: [ - surfaceDayWeatherProvider( - request, - ).overrideWith((ref) => pending.future), - ], - ); - - expect(find.textContaining('°'), findsNothing); - expect(find.byType(CircularProgressIndicator), findsNothing); - - pending.completeError(Exception('weather unavailable')); - await tester.pump(); + testWidgets('without stored weather stays badge-free', (tester) async { + // The header no longer fetches anything, so there is no loading state + // and no spinner to guard against: absent weather is simply no badge. + await pumpHeader(tester, surfaceDay()); expect(find.textContaining('°'), findsNothing); expect(find.byType(CircularProgressIndicator), findsNothing); }); - testWidgets('without a request stays badge-free', (tester) async { - await pumpHeader(tester, surfaceDay()); + testWidgets('dive-logged weather wins over stored weather', (tester) async { + // What the diver recorded outranks a fetched day summary. + final day = TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 2, + kind: TripStoryDayKind.past, + dives: [ + Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + airTemp: 26, + site: const DiveSite(id: 'site-a', name: 'Blue Corner'), + ), + ], + ); - expect(find.textContaining('°'), findsNothing); - expect(find.byType(CircularProgressIndicator), findsNothing); + await pumpHeader( + tester, + day, + storedWeather: const TripStoryDayWeather(airTemp: 22), + ); + + expect(find.text('26°C'), findsOneWidget); + expect(find.text('22°C'), findsNothing); }); }); diff --git a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart index 55df7be3ec..f07b2fddc0 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart @@ -6,6 +6,7 @@ import 'package:go_router/go_router.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; import 'package:submersion/core/providers/provider.dart'; import 'package:submersion/features/checklists/domain/entities/trip_checklist_item.dart'; import 'package:submersion/features/dive_log/domain/entities/dive.dart'; @@ -78,12 +79,14 @@ Future pumpView( List extra = const [], Size viewSize = const Size(800, 2600), http.Client? weatherHttpClient, + Map? tripDayWeather, }) async { tester.view.physicalSize = viewSize; tester.view.devicePixelRatio = 1.0; addTearDown(tester.view.reset); final overrides = await getBaseOverrides( weatherHttpClient: weatherHttpClient, + tripDayWeather: tripDayWeather, ); final stats = TripWithStats(trip: story.trip, diveCount: 2); final router = GoRouter( @@ -405,9 +408,50 @@ void main() { expect(find.text('Surface day'), findsOneWidget); }); - testWidgets('only the surface day fetches from the nearest trip point', ( + testWidgets('the surface day renders stored weather', (tester) async { + final trip = _trip( + start: DateTime(2026, 3, 25), + end: DateTime(2026, 3, 27), + ); + final story = _story( + trip, + dives: [ + _diveAt('d1', DateTime(2026, 3, 25, 9), 12.10, -68.20), + _diveAt('d3', DateTime(2026, 3, 27, 9), 13.30, -69.40), + ], + today: DateTime(2026, 6, 1), + ); + final surfaceDate = DateTime(2026, 3, 26); + final now = DateTime(2026, 3, 28); + + await pumpView( + tester, + story, + tripDayWeather: { + surfaceDate.millisecondsSinceEpoch: TripDayWeather( + id: 'w1', + tripId: trip.id, + date: surfaceDate, + latitude: 12.10, + longitude: -68.20, + airTemp: 26, + cloudCover: CloudCover.clear, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ), + }, + ); + await tester.pump(); + + expect(find.text('26°C'), findsOneWidget); + }); + + testWidgets('a day with nothing stored renders no weather badge', ( tester, ) async { + // The view no longer falls back to a network fetch while rendering: an + // unstored day is simply badge-free until the backfill writes its row. final trip = _trip( start: DateTime(2026, 3, 25), end: DateTime(2026, 3, 27), @@ -421,34 +465,15 @@ void main() { today: DateTime(2026, 6, 1), ); var calls = 0; - final client = MockClient((request) async { + final client = MockClient((_) async { calls++; - expect(request.url.queryParameters['latitude'], '12.1'); - expect(request.url.queryParameters['longitude'], '-68.2'); - expect(request.url.queryParameters['start_date'], '2026-03-26'); - expect(request.url.queryParameters['timezone'], 'auto'); - return http.Response( - jsonEncode({ - 'hourly': { - 'time': ['2026-03-26T12:00'], - 'temperature_2m': [26.0], - 'relative_humidity_2m': [70.0], - 'precipitation': [0.0], - 'cloud_cover': [10.0], - 'wind_speed_10m': [8.0], - 'wind_direction_10m': [30.0], - 'surface_pressure': [1012.0], - 'weathercode': [0], - }, - }), - 200, - ); + return http.Response('', 500); }); await pumpView(tester, story, weatherHttpClient: client); await tester.pump(); - expect(calls, 1); - expect(find.text('26°C'), findsOneWidget); + expect(calls, 0); + expect(find.textContaining('°C'), findsNothing); }); } diff --git a/test/helpers/mock_providers.dart b/test/helpers/mock_providers.dart index f7140f0603..a011905a98 100644 --- a/test/helpers/mock_providers.dart +++ b/test/helpers/mock_providers.dart @@ -27,6 +27,8 @@ import 'package:submersion/features/pre_dive/domain/entities/pre_dive_session.da import 'package:submersion/features/pre_dive/presentation/providers/pre_dive_providers.dart'; import 'package:submersion/core/utils/coordinates/coordinate_format.dart'; import 'package:submersion/features/settings/presentation/providers/settings_providers.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; +import 'package:submersion/features/trips/presentation/providers/trip_day_weather_providers.dart'; import 'package:submersion/features/weather/presentation/providers/weather_providers.dart'; typedef Override = riverpod.Override; @@ -537,6 +539,7 @@ Future> getBaseOverrides({ MockSettingsNotifier? settingsNotifier, http.Client? weatherHttpClient, PreDiveSession? linkedPreDiveSession, + Map? tripDayWeather, }) async { SharedPreferences.setMockInitialValues({}); final prefs = await SharedPreferences.getInstance(); @@ -567,5 +570,13 @@ Future> getBaseOverrides({ weatherHttpClientProvider.overrideWithValue( weatherHttpClient ?? MockClient((_) async => http.Response('', 500)), ), + // Stored trip day weather reaches the real repository and a database + // widget tests do not have; the backfill would additionally walk the + // story and fetch. Both default to inert here, so a test that cares about + // the badge overrides tripDayWeatherProvider with its own rows. + tripDayWeatherProvider.overrideWith( + (ref, tripId) async => tripDayWeather ?? const {}, + ), + tripDayWeatherBackfillProvider.overrideWith((ref, tripId) async {}), ]; } From 2c5bf1c4eb8da3fb8bb1a61d22c16aeda6b6a901 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 18:37:47 -0400 Subject: [PATCH 09/18] chore(trips): mark the weather backfill as tick-exempt The change-tick architecture guard flags any provider that reads a repository without subscribing to a tick. This one must not subscribe: it renders nothing, and the rows it writes would invalidate the pass that wrote them. The marker has to sit within twelve lines of the declaration for the scanner to see it. --- .../providers/trip_day_weather_providers.dart | 15 ++++++++------- .../widgets/story/trip_story_day_header_test.dart | 2 -- .../widgets/story/trip_story_view_test.dart | 2 -- 3 files changed, 8 insertions(+), 11 deletions(-) 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 2c550b5168..f337c35611 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -26,16 +26,17 @@ final tripDayWeatherProvider = return repository.getForTrip(tripId); }); +// no-tick: a side-effecting pass, not a cached query. It renders nothing (the +// value is void), and subscribing to the weather tick would make every row it +// writes invalidate the pass that wrote it. Its rows reach the UI through +// [tripDayWeatherProvider], which does subscribe. A stale read costs nothing: +// a row it misses is simply not refetched, and a stored day is skipped anyway. /// Fills the gaps: fetches historical weather for trip days that have none /// stored and no dive to supply it, then writes what it finds. /// -/// Deliberately does NOT watch [tripDayWeatherProvider]. Watching the rows it -/// writes would invalidate this provider on every write and start another -/// pass, so it reads the stored rows straight from the repository instead. -/// The story is its only reactive input, which means assigning dives to a -/// trip re-evaluates what is still missing. -/// -/// Not auto-disposed: one pass per trip per provider container lifetime. +/// The story is its only reactive input, so assigning dives to a trip +/// re-evaluates what is still missing. Not auto-disposed: one pass per trip +/// per provider container lifetime. final tripDayWeatherBackfillProvider = FutureProvider.family(( ref, tripId, diff --git a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart index 82855221d8..a6c4adc02e 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart @@ -1,5 +1,3 @@ -import 'dart:async'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:intl/intl.dart'; diff --git a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart index f07b2fddc0..b7792afa8e 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart @@ -1,5 +1,3 @@ -import 'dart:convert'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:go_router/go_router.dart'; From 8c880541597e11bd540beb89194c3f8336d12b68 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 20:30:33 -0400 Subject: [PATCH 10/18] chore(db): renumber trip_day_weather from schema v168 to v171 v168 was already claimed and pushed by PR #1237 (issue #638, buddies.is_favorite). That claim was resolved locally and not yet pushed when this branch picked its number, so the open-PR diff scan this plan prescribes could not see it. Two branches writing the same scalar auto-merge with no conflict marker, so the collision would have surfaced only as a database silently skipping a rung. Moves all six coupled sites together plus the test filename: the scalar, the migrationVersions entry, the _assertTripDayWeatherSchema docstring, the onUpgrade guard, its reportProgress twin, and the beforeOpen backstop comment. The migration test is renamed to migration_v171_trip_day_weather_test.dart with its greaterThanOrEqualTo and contains assertions updated. Its stranded-database fixture moves from 168 to 171: 168 is now a real rung owned by #1237, so a database stamped there upgrades normally and no longer exercises the backstop. The design doc and plan are updated too, including the reasoning passages that were wrong rather than merely stale: the plan recorded #1237 as a stale v161 claim when it was a live v168 one. Both now say to scan every worktree's working-tree scalar alongside open PR diffs, and to re-run both immediately before pushing rather than only when picking the number. Ladder is left non-contiguous by design: 165 #1290, 166 #1300, 167 #1276, 168 #1237, 169 the dive-computer gear-twin branch, 170 #1322. Verified: ladder monotonic, unique, scalar == max, 168 absent; helper defined exactly once and referenced three times; guard and twin both at 171; flutter analyze clean; flutter test test/core/database/ 471 passed. --- .../2026-08-26-trip-day-weather-storage.md | 44 +++++++++---------- ...6-08-26-trip-day-weather-storage-design.md | 8 ++-- lib/core/database/database.dart | 27 +++++++----- ...migration_v171_trip_day_weather_test.dart} | 14 +++--- 4 files changed, 49 insertions(+), 44 deletions(-) rename test/core/database/{migration_v168_trip_day_weather_test.dart => migration_v171_trip_day_weather_test.dart} (92%) diff --git a/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md index 018f4cfe69..68f6f4d0ba 100644 --- a/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md +++ b/docs/superpowers/plans/2026-08-26-trip-day-weather-storage.md @@ -13,7 +13,7 @@ ## Global Constraints - **Worktree:** all work happens in `/Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather` on branch `worktree-trip-day-weather`. The Bash working directory does NOT reliably persist across turns: put `cd &&` in the same compound command as the work and echo `pwd`, or use absolute paths. A relative-path write from the wrong cwd silently edits the main checkout. -- **Schema version is 168.** Verified by scanning open PR diffs: main is at 164, and 165/166/167 are claimed by PRs #1300, #1290, #1276. Re-verify with the scan in Task 1 before writing the number. Do NOT raise `minimumCompatibleSchemaVersion` (stays 160): a new table is additive. +- **Schema version is 171.** Renumbered from 168: that number was already claimed and pushed by PR #1237 (issue #638), but the claim was local and unpushed when this branch picked its number, so an open-PR scan could not see it. 165-170 are claimed by #1290, #1300, #1276, #1237, the gear-twin branch, and #1322. Re-verify with BOTH scans (open-PR diffs AND every worktree's working-tree scalar) immediately before pushing, not just when picking the number. Do NOT raise `minimumCompatibleSchemaVersion` (stays 160): a new table is additive. - **No em-dashes** (`—`, U+2014) in any output: code, comments, docs, commit messages. En-dashes and " - " as prose punctuation are equally forbidden. A hyphen inside a compound word or CLI flag is fine. - **No emojis** in code, comments, or documentation. - **Timestamps are epoch MILLISECONDS** in these tables (`DateTime.millisecondsSinceEpoch`), matching `ItineraryDayRepository`. The `// Unix timestamp` comment on `trip_itinerary_days.date` is misleading; the repository writes milliseconds. @@ -25,19 +25,19 @@ --- -### Task 1: Schema, entity, and the v168 migration +### Task 1: Schema, entity, and the v171 migration **Files:** - Create: `lib/features/trips/domain/entities/trip_day_weather.dart` - Modify: `lib/core/database/database.dart` (table class near `TripItineraryDays` at line 117; `@DriftDatabase(tables: [...])` list; `currentSchemaVersion` at line 3183; `migrationVersions` list; a new `_assertTripDayWeatherSchema()` helper next to `_assertQualityFindingsSchema()` at line 3932; the `onUpgrade` ladder tail at line 8614; the `beforeOpen` backstop block around line 8823) -- Test: `test/core/database/migration_v168_trip_day_weather_test.dart` +- Test: `test/core/database/migration_v171_trip_day_weather_test.dart` **Interfaces:** - Consumes: nothing (first task). - Produces: - Drift table `TripDayWeather` -> generated row class `TripDayWeatherData`, accessor `_db.tripDayWeather`, companion `TripDayWeatherCompanion`. - Domain entity `TripDayWeather` (in the `features/trips` namespace; import it `as domain` wherever the Drift row class is also in scope, per the project's import-alias convention) with fields `id`, `tripId`, `date`, `latitude`, `longitude`, `airTemp`, `cloudCover`, `precipitation`, `windSpeed`, `windDirection`, `humidity`, `surfacePressure`, `weatherCode`, `weatherSource`, `fetchedAt`, `createdAt`, `updatedAt`; `copyWith`; and `TripStoryDayWeather toStoryWeather()`. - - `AppDatabase.currentSchemaVersion == 168`. + - `AppDatabase.currentSchemaVersion == 171`. - [ ] **Step 1: Re-verify the schema version claim** @@ -51,11 +51,11 @@ cd /Users/ericgriffin/repos/submersion-app/submersion && \ done ``` -Expected: claims at 165 (#1290), 166 (#1300), 167 (#1276), plus stale claims from #1237 and #603 that are far below main and do not count. Combined with `currentSchemaVersion = 164` on this branch, the next free rung is **168**. If the scan shows a claim at 168, use the next free number above every claim and substitute it everywhere `168` appears in this plan, including the test filename. +Expected: claims at 165 (#1290), 166 (#1300), 167 (#1276), 168 (#1237), 169 (the dive-computer gear-twin branch, local only), 170 (#1322), plus a stale claim from #603 that is far below main and does not count. NOTE: the #1237 claim read as stale v161 when this plan was written and was in fact a live v168, because its renumber was resolved locally and not yet pushed. An open-PR scan cannot see an unpushed claim; also scan every worktree's working-tree scalar. The next free rung is **171**. - [ ] **Step 2: Write the failing migration test** -Create `test/core/database/migration_v168_trip_day_weather_test.dart`. Model it on the other `migration_v*_test.dart` files in that directory: read one first to copy the exact in-memory database setup helper they use. +Create `test/core/database/migration_v171_trip_day_weather_test.dart`. Model it on the other `migration_v*_test.dart` files in that directory: read one first to copy the exact in-memory database setup helper they use. ```dart import 'package:drift/drift.dart'; @@ -66,13 +66,13 @@ import 'package:submersion/core/database/database.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); - group('v168 trip_day_weather', () { - test('the ladder claims 168', () { + group('v171 trip_day_weather', () { + test('the ladder claims 171', () { expect( AppDatabase.currentSchemaVersion, - greaterThanOrEqualTo(168), + greaterThanOrEqualTo(171), ); - expect(AppDatabase.migrationVersions, contains(168)); + expect(AppDatabase.migrationVersions, contains(171)); }); test('a new database has the trip_day_weather table with every column', () async { @@ -149,10 +149,10 @@ Note on the last test: check how the sibling `migration_v*_test.dart` files buil ```bash cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ echo "PWD: $(pwd)" && \ - flutter test test/core/database/migration_v168_trip_day_weather_test.dart + flutter test test/core/database/migration_v171_trip_day_weather_test.dart ``` -Expected: FAIL. `migrationVersions` does not contain 168, and `PRAGMA table_info(trip_day_weather)` returns no rows. +Expected: FAIL. `migrationVersions` does not contain 171, and `PRAGMA table_info(trip_day_weather)` returns no rows. - [ ] **Step 4: Add the Drift table** @@ -219,7 +219,7 @@ Then add `TripDayWeather,` to the `tables: [...]` list in the `@DriftDatabase` a Next to `_assertQualityFindingsSchema()` (line 3932), which is the pattern to copy, add: ```dart - /// v168: fetched per-day trip weather. Idempotent, so it doubles as the + /// v171: fetched per-day trip weather. Idempotent, so it doubles as the /// beforeOpen backstop for a database that took the rung before the unique /// index existed. Future _assertTripDayWeatherSchema() async { @@ -254,24 +254,24 @@ Next to `_assertQualityFindingsSchema()` (line 3932), which is the pattern to co - [ ] **Step 6: Claim the rung in all six places** -1. `static const int currentSchemaVersion = 168;` (was 164, line 3183). -2. Append `168,` to the end of the `migrationVersions` list. The ladder is non-contiguous by design (162 is permanently skipped and reserved rungs may be missing); do not "fix" the gaps. -3. The helper docstring above already names v168. +1. `static const int currentSchemaVersion = 171;` (was 164, line 3183). +2. Append `171,` to the end of the `migrationVersions` list. The ladder is non-contiguous by design (162 is permanently skipped and reserved rungs may be missing); do not "fix" the gaps. +3. The helper docstring above already names v171. 4. In `onUpgrade`, after the `if (from < 164) await reportProgress();` pair at line 8617, add both halves: ```dart - // v168: trip_day_weather, fetched per-day weather for trip days whose + // v171: trip_day_weather, fetched per-day weather for trip days whose // dives supply none. - if (from < 168) { + if (from < 171) { await _assertTripDayWeatherSchema(); } - if (from < 168) await reportProgress(); + if (from < 171) await reportProgress(); ``` 5. In `beforeOpen`, alongside the other backstops (around line 8823), add: ```dart - // v168 backstop: re-assert the trip day weather table (the helper is + // v171 backstop: re-assert the trip day weather table (the helper is // CREATE TABLE IF NOT EXISTS, so it is safe on every open). await _assertTripDayWeatherSchema(); ``` @@ -456,7 +456,7 @@ Expected: PASS, including the pre-existing ladder audit tests. Roughly 465 tests cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day-weather && \ dart format . && \ git add -A && \ - git commit -m "feat(db): add trip_day_weather table at schema v168 + git commit -m "feat(db): add trip_day_weather table at schema v171 One row per trip day, holding fetched historical weather for days whose dives supply none. A separate table rather than columns on an existing @@ -2011,7 +2011,7 @@ cd /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/trip-day grep -n "currentSchemaVersion = " lib/core/database/database.dart | head -1 ``` -Expected: 168, and still above every claim found by the Task 1 scan. If another branch has landed on 168 since, renumber: the six places from Task 1 Step 6 plus the test filename, then re-run `flutter test test/core/database/`. +Expected: 171, and still above every claim found by the Task 1 scan. Re-run BOTH scans immediately before pushing, not just when picking the number: a claim can land in between. If another branch has taken 171 since, renumber: the six places from Task 1 Step 6 plus the test filename and this plan, then re-run `flutter test test/core/database/`. - [ ] **Step 5: Commit anything the format pass touched** diff --git a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md index a6f474555e..7173932653 100644 --- a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md +++ b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md @@ -97,7 +97,7 @@ per the project convention, plus a mapping to the existing ## Schema version -**v168.** Derived by scanning open PR diffs for the scalar, not by grepping +**v171.** Renumbered from 168, which PR #1237 held. Derived by scanning open PR diffs for the scalar, not by grepping main: main is at v164, and v165, v166, v167 are claimed by PRs #1300, #1290, and #1276 respectively. Re-verify at implementation time, and re-grep the scalar after any merge from main, because two branches writing the same number @@ -105,9 +105,9 @@ auto-merge with no conflict marker. The claim touches the six places the ladder requires: the `currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the -`_assertTripDayWeatherSchema()` helper docstring, the `if (from < 168)` +`_assertTripDayWeatherSchema()` helper docstring, the `if (from < 171)` onUpgrade guard and its `reportProgress()` twin, the `beforeOpen` backstop -comment, and the `migration_v168_trip_day_weather_test.dart` filename with its +comment, and the `migration_v171_trip_day_weather_test.dart` filename with its version assertions. The ladder is non-contiguous by design (v162 is permanently skipped, and reserved rungs may be missing), so the migration audit asserts monotonic, unique, and scalar equals max, never contiguous. @@ -204,7 +204,7 @@ Tests come first, per the project's TDD rule. future day, row already stored, no coordinate available), plus the two negative-result cases that must write no row (service returns null; service returns an all-null `WeatherData`). -- **Migration:** `migration_v168_trip_day_weather_test.dart`, including a +- **Migration:** `migration_v171_trip_day_weather_test.dart`, including a stranded-database fixture at the previous `PRAGMA user_version`, plus the existing ladder audit in `test/core/database/`. - **Sync:** a round trip proving a row exports and re-imports intact. diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index d6ef5413cd..9e8d40a42e 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -3235,7 +3235,7 @@ class AppDatabase extends _$AppDatabase { /// The current schema version as a static constant so that pre-open checks /// (e.g. version-mismatch guard) can reference it without an instance. - static const int currentSchemaVersion = 168; + static const int currentSchemaVersion = 171; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3531,11 +3531,16 @@ class AppDatabase extends _$AppDatabase { // media item in the dive when its capture time is wrong (issue #1090). // Renumbered from 162, which #731 landed past while this branch was open. 164, - // v168: trip_day_weather, fetched historical weather for trip days whose - // dives supply none. 165, 166, and 167 are claimed by open PRs (#1290, - // #1300, #1276), so this ladder is non-contiguous by design; the audit + // v171: trip_day_weather, fetched historical weather for trip days whose + // dives supply none. Renumbered from 168, which PR #1237 (issue #638, + // buddies.is_favorite) had already claimed and pushed; that claim was + // local and unpushed when this branch picked its number, so an open-PR + // scan could not see it. + // 165 through 170 are deliberately absent, not missing: 165 #1290, + // 166 #1300, 167 #1276, 168 #1237, 169 the dive-computer gear-twin + // branch, 170 #1322. This ladder is non-contiguous by design; the audit // asserts monotonic, unique, and scalar == max, never contiguous. - 168, + 171, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -3989,10 +3994,10 @@ class AppDatabase extends _$AppDatabase { /// v129: quality_findings table for the Data Quality Assistant. /// Idempotent so it is safe to call from both onUpgrade and the /// beforeOpen backstop. - /// v168: fetched per-day trip weather. + /// v171: fetched per-day trip weather. /// /// Idempotent, so it doubles as the beforeOpen backstop for a database - /// stranded at 168 by a parallel branch that never created the table. + /// stranded at 171 by a parallel branch that never created the table. Future _assertTripDayWeatherSchema() async { await customStatement(''' CREATE TABLE IF NOT EXISTS trip_day_weather ( @@ -8710,12 +8715,12 @@ class AppDatabase extends _$AppDatabase { await _assertMediaManualElapsedColumn(); } if (from < 164) await reportProgress(); - // v168: trip_day_weather, fetched per-day weather for trip days whose + // v171: trip_day_weather, fetched per-day weather for trip days whose // dives supply none. - if (from < 168) { + if (from < 171) { await _assertTripDayWeatherSchema(); } - if (from < 168) await reportProgress(); + if (from < 171) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8920,7 +8925,7 @@ class AppDatabase extends _$AppDatabase { // media row mapper reads it on every hydration. await _assertMediaManualElapsedColumn(); - // v168 backstop: re-assert trip_day_weather (same parallel-branch + // v171 backstop: re-assert trip_day_weather (same parallel-branch // version-collision self-heal). The helper is CREATE TABLE IF NOT // EXISTS plus CREATE UNIQUE INDEX IF NOT EXISTS, so it is a no-op on // every open after the first. diff --git a/test/core/database/migration_v168_trip_day_weather_test.dart b/test/core/database/migration_v171_trip_day_weather_test.dart similarity index 92% rename from test/core/database/migration_v168_trip_day_weather_test.dart rename to test/core/database/migration_v171_trip_day_weather_test.dart index bf505f7ad1..6ce941840d 100644 --- a/test/core/database/migration_v168_trip_day_weather_test.dart +++ b/test/core/database/migration_v171_trip_day_weather_test.dart @@ -3,7 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/database/database.dart'; -/// v168 adds `trip_day_weather`: fetched historical weather for trip days +/// v171 adds `trip_day_weather`: fetched historical weather for trip days /// whose dives supply none of their own (surface days and dive-free itinerary /// days). Its own table rather than columns on `trips` or `trip_itinerary_days` /// because HLC conflicts resolve per row, and an automatic derived write must @@ -36,9 +36,9 @@ Future> _columnsOf(AppDatabase db, String table) async { } void main() { - test('v168 is in the migration ladder', () { - expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(168)); - expect(AppDatabase.migrationVersions, contains(168)); + test('v171 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(171)); + expect(AppDatabase.migrationVersions, contains(171)); }); test('a fresh database has trip_day_weather with every column', () async { @@ -123,13 +123,13 @@ void main() { expect(trip.read('name'), 'Bonaire'); }); - test('a database stranded at a parallel-branch v168 gains the table via ' + test('a database stranded at a parallel-branch v171 gains the table via ' 'beforeOpen', () async { - // Stamped AT 168 but without the table: the onUpgrade block never runs, + // Stamped AT 171 but without the table: the onUpgrade block never runs, // so only the beforeOpen backstop can create it. final nativeDb = NativeDatabase.memory( setup: (rawDb) { - rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute('PRAGMA user_version = 171'); }, ); final db = AppDatabase(nativeDb); From 6328b3ce7f5b18c3c0e25e605ca94ed5f6f5cb05 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 21:27:55 -0400 Subject: [PATCH 11/18] fix(trips): treat unrenderable dive weather as no weather Addresses two review comments on PR #1319. A dive whose weather lookup resolved nothing still stores Precipitation.none, because WeatherMapper never returns null precipitation. TripStoryDay.weather was therefore non-null for such a day, so the backfill skipped it and the header drew nothing: a permanently badge-free day. The renderability rule now lives on TripStoryDayWeather and is shared by three callers, the backfill skip decision, the header's precedence, and the entity's is-this-worth-storing check, so the three cannot drift apart. The repository also normalizes the day to local midnight on both read and write. It owns the (trip, date) uniqueness invariant, and a caller passing a time component, or a row arriving through sync from a peer, would otherwise store a second row for the same calendar day that no midnight-keyed lookup could see. --- .../trip_day_weather_repository.dart | 18 +++++- .../domain/entities/trip_day_weather.dart | 13 ++-- .../trips/domain/entities/trip_story_day.dart | 13 ++++ .../services/trip_day_weather_backfill.dart | 8 ++- .../widgets/story/trip_story_day_header.dart | 12 +++- .../trip_day_weather_repository_test.dart | 27 ++++++++ .../trip_day_weather_backfill_test.dart | 63 ++++++++++++++++++- .../story/trip_story_day_header_test.dart | 31 +++++++++ 8 files changed, 168 insertions(+), 17 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 399645d5d8..89bb58c64a 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -19,6 +19,17 @@ class TripDayWeatherRepository { final SyncRepository _syncRepository = SyncRepository(); final _log = LoggerService.forClass(TripDayWeatherRepository); + /// Local midnight for [date], as epoch milliseconds. + /// + /// The day is the identity, so normalizing here is what actually enforces + /// the (trip, date) uniqueness intent. A caller that passes a DateTime with + /// a time component would otherwise store a second row for the same + /// calendar day, invisible to every midnight-keyed lookup and refetched on + /// every view. Reads normalize too, because a row can also arrive through + /// sync from a peer, bypassing this class entirely. + static int _dayKey(DateTime date) => + DateTime(date.year, date.month, date.day).millisecondsSinceEpoch; + /// Emits whenever `trip_day_weather` changes, so the display provider /// refreshes after a backfill write or a sync import. Stream watchWeatherChanges() => @@ -30,7 +41,10 @@ class TripDayWeatherRepository { final rows = await (_db.select( _db.tripDayWeather, )..where((t) => t.tripId.equals(tripId))).get(); - return {for (final row in rows) row.date: _mapRow(row)}; + return { + for (final row in rows) + _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)): _mapRow(row), + }; } catch (e, stackTrace) { _log.error( 'Failed to read weather for trip: $tripId', @@ -45,7 +59,7 @@ class TripDayWeatherRepository { Future upsert(domain.TripDayWeather weather) async { try { final now = DateTime.now().millisecondsSinceEpoch; - final dateMillis = weather.date.millisecondsSinceEpoch; + final dateMillis = _dayKey(weather.date); // Reuse the stored row's id when the day already has one: a peer may // have written its own uuid for this day, and replacing it under a new diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index a9ae4be434..fd1412e491 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -65,15 +65,10 @@ class TripDayWeather extends Equatable { /// carrying only those renders as nothing and would suppress the retry that /// a later archive update would satisfy. /// - /// [Precipitation.none] does not count. `WeatherMapper.mapPrecipitation` - /// returns non-null always, defaulting a missing reading to `none`, so - /// `none` cannot be read as evidence that the fetch resolved anything. It - /// also earns no glyph of its own in `weatherIconFor`, which falls through - /// to cloud cover. - bool get hasRenderableWeather => - airTemp != null || - cloudCover != null || - (precipitation != null && precipitation != Precipitation.none); + /// Delegates to [TripStoryDayWeather.isRenderable] so the rule that decides + /// what is worth storing is the same one that decides what the header can + /// draw, and the same one the backfill uses to judge a day's dive weather. + bool get hasRenderableWeather => toStoryWeather().isRenderable; /// The compact view model the day header consumes. TripStoryDayWeather toStoryWeather() => TripStoryDayWeather( diff --git a/lib/features/trips/domain/entities/trip_story_day.dart b/lib/features/trips/domain/entities/trip_story_day.dart index 000e73b5da..70d272153a 100644 --- a/lib/features/trips/domain/entities/trip_story_day.dart +++ b/lib/features/trips/domain/entities/trip_story_day.dart @@ -133,6 +133,19 @@ class TripStoryDayWeather extends Equatable { this.precipitation, }); + /// True when the day header's badge would actually draw something. + /// + /// [Precipitation.none] does not count, and that is the whole point of this + /// getter. `WeatherMapper.mapPrecipitation` never returns null: a missing + /// reading becomes `none`, so a dive whose weather lookup resolved nothing + /// still stores `none`. `weatherIconFor` gives `none` no glyph of its own, + /// so such a day renders as blank. Treating it as "this day has weather" + /// would leave the day badge-free forever. + bool get isRenderable => + airTemp != null || + cloudCover != null || + (precipitation != null && precipitation != Precipitation.none); + @override List get props => [airTemp, cloudCover, precipitation]; } 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 05222ec407..0842cfa18f 100644 --- a/lib/features/trips/domain/services/trip_day_weather_backfill.dart +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -45,7 +45,13 @@ class TripDayWeatherBackfill { // A dive that logged weather is the better source: it is what the diver // recorded. Never override it with a fetched summary. - if (day.weather != null) continue; + // + // Renderability, not mere presence, is the test. A dive whose weather + // lookup resolved nothing still stores Precipitation.none, because + // WeatherMapper never returns null precipitation, and that renders as a + // blank badge. Skipping on presence alone would leave such a day + // permanently badge-free. + if (day.weather?.isRenderable ?? false) continue; // A historical archive has nothing for a day that has not happened. if (day.kind == TripStoryDayKind.future) continue; diff --git a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart index bea6ef77ee..cf55cc3427 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_day_header.dart @@ -61,9 +61,15 @@ class TripStoryDayHeader extends ConsumerWidget { ...day.siteNames, ].map((part) => part.trim()).where((part) => part.isNotEmpty).toList(); - // Dive-logged weather always wins: it is what the diver recorded, and a - // stored day summary is only ever a stand-in for days that logged none. - final weather = day.weather ?? storedWeather; + // Dive-logged weather wins: it is what the diver recorded, and a stored + // day summary is only ever a stand-in for days that logged none. It wins + // only when it can actually draw something, though: a dive whose weather + // lookup resolved nothing still carries Precipitation.none, and letting + // that mask a stored row would render the day blank. + final diveWeather = day.weather; + final weather = (diveWeather != null && diveWeather.isRenderable) + ? diveWeather + : (storedWeather ?? diveWeather); final units = UnitFormatter(ref.watch(settingsProvider)); final weatherBadge = _weatherBadge(context, theme, units, weather); diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart index 3686260aa5..503be868e0 100644 --- a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -119,6 +119,33 @@ void main() { expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); }); + test('a date with a time component is stored under local midnight', () { + // The repository owns the (trip, date) uniqueness invariant, so it + // normalizes rather than trusting every caller to. A row keyed on a + // stray time would be invisible to midnight-keyed lookups and would + // refetch forever. + return () async { + await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30))); + + final stored = await repository.getForTrip(testTripId); + + expect(stored.keys.single, day1.millisecondsSinceEpoch); + expect(stored[day1.millisecondsSinceEpoch]!.date, day1); + }(); + }); + + test('the same day at two times of day stays one row', () async { + await repository.upsert(sample(date: DateTime(2026, 3, 8, 6))); + await repository.upsert( + sample(id: 'w2', date: DateTime(2026, 3, 8, 23), airTemp: 25), + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + }); + test('two different days both persist', () async { await repository.upsert(sample()); await repository.upsert(sample(id: 'w2', date: day2, airTemp: 19)); diff --git a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart index 70e6563e72..44e1152522 100644 --- a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart +++ b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart @@ -1,4 +1,5 @@ import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/constants/enums.dart'; import 'package:submersion/features/dive_log/domain/entities/dive.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart'; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; @@ -28,8 +29,17 @@ void main() { ); } - Dive diveWith({double? airTemp}) => - Dive(id: 'd1', dateTime: DateTime(2026, 3, 8, 9), airTemp: airTemp); + Dive diveWith({ + double? airTemp, + CloudCover? cloudCover, + Precipitation? precipitation, + }) => Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + airTemp: airTemp, + cloudCover: cloudCover, + precipitation: precipitation, + ); TripStoryDay day({ required int index, @@ -196,6 +206,55 @@ void main() { ]); }); + test( + 'a dive carrying only Precipitation.none does NOT count as weather', + () { + // WeatherMapper never returns null precipitation, so a dive whose + // lookup resolved nothing still stores `none`. The header draws no + // glyph for it, so treating it as "this day has weather" would skip the + // backfill and leave the day permanently badge-free. + final story = storyWith( + [ + day(index: 0, dives: [diveWith(precipitation: Precipitation.none)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + hasLength(1), + ); + }, + ); + + test('a dive carrying active precipitation DOES count as weather', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(precipitation: Precipitation.rain)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + + test('a dive carrying only cloud cover counts as weather', () { + final story = storyWith( + [ + day(index: 0, dives: [diveWith(cloudCover: CloudCover.overcast)]), + ], + points: [pointFor(0)], + ); + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: const {}), + isEmpty, + ); + }); + test('a day date with a time component is normalized to midnight', () { // The story day's date should already be date-only, but a stored row is // keyed on midnight millis, so a stray time would never match and the diff --git a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart index a6c4adc02e..0fde9196fa 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_day_header_test.dart @@ -291,6 +291,37 @@ void main() { expect(find.byType(CircularProgressIndicator), findsNothing); }); + testWidgets('stored weather wins when the dive weather renders nothing', ( + tester, + ) async { + // A dive whose lookup resolved nothing still carries + // Precipitation.none, which draws no glyph. The stored row is the only + // thing that can render, so it must not be masked. + final day = TripStoryDay( + date: DateTime(2026, 3, 8), + dayNumber: 2, + kind: TripStoryDayKind.past, + dives: [ + Dive( + id: 'd1', + dateTime: DateTime(2026, 3, 8, 9), + precipitation: Precipitation.none, + ), + ], + ); + + await pumpHeader( + tester, + day, + storedWeather: const TripStoryDayWeather( + airTemp: 22, + cloudCover: CloudCover.clear, + ), + ); + + expect(find.text('22°C'), findsOneWidget); + }); + testWidgets('dive-logged weather wins over stored weather', (tester) async { // What the diver recorded outranks a fetched day summary. final day = TripStoryDay( From 5cd3e2e362541170893c6f6d0ee0e1dae88cfd42 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 22:02:17 -0400 Subject: [PATCH 12/18] fix(trips): derive the trip day weather row id from trip and day A v4 id per device meant two devices that both fetched the same day inserted two rows. The serializer upserts by primary key, so the peer's differing id missed the ON CONFLICT target and hit the unique (trip_id, date) index instead: SqliteException 2067, thrown inside the merge transaction, which aborts the whole sync pull rather than one row. Reproduced in a test before fixing. The id is now a UUIDv5 over (tripId, dayMillis), following the same convention as importedDiveComputerId and qualityFindingId, whose docstring already warned that a unique constraint on a replicated table turns an inbound insert into a throw rather than a merge. The repository derives it and ignores whatever id a caller passes. _mapRow also normalizes the entity's date, not just the map key, so a row carrying a time component cannot hand a time-bearing date to downstream logic. --- .../trip_day_weather_repository.dart | 33 ++++++--- .../domain/entities/trip_day_weather.dart | 20 ++++++ .../providers/trip_day_weather_providers.dart | 8 +-- .../sync/trip_day_weather_sync_test.dart | 67 +++++++++++++++++-- 4 files changed, 105 insertions(+), 23 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 89bb58c64a..cd8e73af6d 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -8,6 +8,8 @@ import 'package:submersion/core/services/logger_service.dart'; import 'package:submersion/core/services/sync/sync_event_bus.dart'; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' as domain; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' + show tripDayWeatherRowId; /// Reads and writes stored per-day trip weather. /// @@ -61,16 +63,19 @@ class TripDayWeatherRepository { final now = DateTime.now().millisecondsSinceEpoch; final dateMillis = _dayKey(weather.date); - // Reuse the stored row's id when the day already has one: a peer may - // have written its own uuid for this day, and replacing it under a new - // id would violate the unique index and orphan the peer's sync record. - final existing = - await (_db.select(_db.tripDayWeather)..where( - (t) => - t.tripId.equals(weather.tripId) & t.date.equals(dateMillis), - )) - .getSingleOrNull(); - final id = existing?.id ?? weather.id; + // The id is derived from (trip, day), never taken from the caller, so + // every device writing this day produces the same primary key and sync + // merges by id instead of colliding on the unique index. + final id = tripDayWeatherRowId( + tripId: weather.tripId, + dayMillis: dateMillis, + ); + + // Only to preserve createdAt across an update; insertOnConflictUpdate + // would otherwise overwrite it with this write's timestamp. + final existing = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.id.equals(id))).getSingleOrNull(); await _db .into(_db.tripDayWeather) @@ -149,7 +154,13 @@ class TripDayWeatherRepository { return domain.TripDayWeather( id: row.id, tripId: row.tripId, - date: DateTime.fromMillisecondsSinceEpoch(row.date), + // Normalized, matching the map key getForTrip returns it under: a row + // written by an older build or an out-of-date peer can still carry a + // time component, and handing that back would put time-bearing dates + // into downstream logic. + date: DateTime.fromMillisecondsSinceEpoch( + _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)), + ), latitude: row.latitude, longitude: row.longitude, airTemp: row.airTemp, diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index fd1412e491..ed63935617 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -1,8 +1,28 @@ import 'package:equatable/equatable.dart'; +import 'package:uuid/uuid.dart'; import 'package:submersion/core/constants/enums.dart'; import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; +/// Fixed namespace for deterministic trip-day-weather ids (UUIDv5). +/// Never change: the ids already stored depend on it. +const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; + +/// Deterministic row id for one trip day. +/// +/// The day is the identity, so the id must be derived from it rather than +/// minted per device. Two devices that both fetch the same day would +/// otherwise insert two rows, and the unique (trip_id, date) index turns the +/// second one into an inbound-sync failure rather than a merge: the +/// serializer upserts by primary key, so a differing id misses the conflict +/// target entirely and hits the index instead. That throws inside the merge +/// transaction and aborts the whole sync pull. +/// +/// [dayMillis] must already be normalized to local midnight; the repository +/// does that before calling here. +String tripDayWeatherRowId({required String tripId, required int dayMillis}) => + const Uuid().v5(kTripDayWeatherNamespace, '$tripId|$dayMillis'); + /// Stored historical weather for one trip day. /// /// Written only for days whose dives supply no weather of their own; a day 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 f337c35611..e16b69db6b 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -1,5 +1,3 @@ -import 'package:uuid/uuid.dart'; - import 'package:submersion/core/providers/provider.dart'; import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; @@ -52,8 +50,6 @@ final tripDayWeatherBackfillProvider = FutureProvider.family(( ); if (targets.isEmpty) return; - const uuid = Uuid(); - // Sequential on purpose: a two-week trip would otherwise open with a burst // of parallel requests, and rows landing one at a time let the day headers // fill in progressively. @@ -71,7 +67,9 @@ final tripDayWeatherBackfillProvider = FutureProvider.family(( final now = DateTime.now(); final row = TripDayWeather( - id: uuid.v4(), + // Ignored by the repository, which derives the id from (trip, day) so + // every device converges on one row. + id: '', tripId: tripId, date: target.date, latitude: target.latitude, diff --git a/test/core/services/sync/trip_day_weather_sync_test.dart b/test/core/services/sync/trip_day_weather_sync_test.dart index 781fb8f41c..66719a2aee 100644 --- a/test/core/services/sync/trip_day_weather_sync_test.dart +++ b/test/core/services/sync/trip_day_weather_sync_test.dart @@ -4,6 +4,7 @@ import 'package:submersion/core/data/repositories/sync_repository.dart'; import 'package:submersion/core/database/database.dart'; import 'package:submersion/core/services/sync/sync_data_serializer.dart'; import 'package:submersion/core/services/sync/sync_service.dart'; +import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart'; import '../../../helpers/test_database.dart'; @@ -11,6 +12,9 @@ void main() { late AppDatabase db; late SyncDataSerializer serializer; + final dayMillis = DateTime(2026, 3, 8).millisecondsSinceEpoch; + final rowId = tripDayWeatherRowId(tripId: 'trip-1', dayMillis: dayMillis); + setUp(() async { db = await setUpTestDatabase(); serializer = SyncDataSerializer(); @@ -30,7 +34,10 @@ void main() { .into(db.tripDayWeather) .insert( TripDayWeatherCompanion.insert( - id: 'w-1', + id: tripDayWeatherRowId( + tripId: 'trip-1', + dayMillis: DateTime(2026, 3, 8).millisecondsSinceEpoch, + ), tripId: 'trip-1', date: DateTime(2026, 3, 8).millisecondsSinceEpoch, latitude: 12.16, @@ -47,7 +54,7 @@ void main() { tearDown(tearDownTestDatabase); test('tripDayWeather export, fetch, upsert, and delete round-trip', () async { - final record = await serializer.fetchRecord('tripDayWeather', 'w-1'); + final record = await serializer.fetchRecord('tripDayWeather', rowId); expect(record, isNotNull); expect(record!['airTemp'], 24.0); expect(record['cloudCover'], 'clear'); @@ -58,19 +65,19 @@ void main() { 'airTemp': 26.0, 'updatedAt': 2, }); - final merged = await serializer.fetchRecord('tripDayWeather', 'w-1'); + final merged = await serializer.fetchRecord('tripDayWeather', rowId); expect(merged!['airTemp'], 26.0); - expect(await serializer.recordIdsFor('tripDayWeather'), contains('w-1')); + expect(await serializer.recordIdsFor('tripDayWeather'), contains(rowId)); - await serializer.deleteRecord('tripDayWeather', 'w-1'); - expect(await serializer.fetchRecord('tripDayWeather', 'w-1'), isNull); + await serializer.deleteRecord('tripDayWeather', rowId); + expect(await serializer.fetchRecord('tripDayWeather', rowId), isNull); }); test('the delta export filters on the row own hlc', () async { await (db.update( db.tripDayWeather, - )..where((t) => t.id.equals('w-1'))).write( + )..where((t) => t.id.equals(rowId))).write( const TripDayWeatherCompanion(hlc: Value('2026-08-16T00:00:00.000-0000')), ); @@ -90,6 +97,52 @@ void main() { expect(await changesetCount('2026-08-15T00:00:00.000-0000'), 1); }); + test('a peer row for the same day merges instead of throwing', () async { + // Two devices that both fetch the same day must converge. A v4 id per + // device would insert a second row and violate the unique (trip_id, date) + // index, and because the merge runs in a transaction that aborts the + // whole sync pull, not just this row. + // The peer derives the same id from the same (trip, day). + await serializer.upsertRecord('tripDayWeather', { + 'id': tripDayWeatherRowId(tripId: 'trip-1', dayMillis: dayMillis), + 'tripId': 'trip-1', + 'date': dayMillis, + 'latitude': 12.16, + 'longitude': -68.28, + 'airTemp': 26.0, + 'weatherSource': 'openMeteo', + 'fetchedAt': 2, + 'createdAt': 2, + 'updatedAt': 2, + }); + + final rows = await db.select(db.tripDayWeather).get(); + expect(rows, hasLength(1)); + expect(rows.single.airTemp, 26.0); + }); + + 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, + ), + ), + ); + }); + test('tripDayWeather is registered as an hlc target', () { // An omission here is silent: _stampHlc no-ops on an unknown entity type, // the column stays NULL, and the incremental export's hlc > watermark From 47c12a59d3d1a3a9a53467841a0fe452cfcbd7a6 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 01:25:41 -0400 Subject: [PATCH 13/18] fix(trips): reconcile trip day weather rows upsert did not write A row for the same trip and calendar day that this repository did not write is not a hypothetical: a peer on a build that predates the derived id, or a database written before this class normalized, produces one. Two things went wrong with it. The upsert threw. The unique index is on (trip_id, date) but Drift emits ON CONFLICT("id"), so a foreign-id row on the same midnight is not a conflict the statement can absorb: the insert misses the ON CONFLICT target and hits the index instead. The day could not be written at all, not merely written twice. The read collapsed it arbitrarily. getForTrip keyed a map by normalized day over an unordered result, so which of two same-day rows showed depended on the order SQLite happened to return them in. upsert now deletes every same-day row whose id is not the derived one before inserting the canonical row, both in one transaction so a day is never left with its old row gone and no new one in its place. The strays are tombstoned after the commit: they are synced records, and dropping one without a tombstone lets the peer that sent it hand it straight back. createdAt is the minimum across the day's rows, because a stray is this day under an old id rather than a different record. Reads resolve deterministically instead: the canonical row wins, then the most recently updated, then the id. They deliberately do not clean up, since a write here would fire the table tick the display provider subscribes to and invalidate the read in flight. The backfill no longer passes a placeholder id. The local-midnight conversion moved next to tripDayWeatherRowId as tripDayMillis, so the caller derives the real id and the repository's _dayKey delegates to the same function; the two agree by construction rather than by comment. The repository still derives its own id and ignores the caller's. --- .../trip_day_weather_repository.dart | 163 +++++++++++++---- .../domain/entities/trip_day_weather.dart | 13 +- .../providers/trip_day_weather_providers.dart | 9 +- .../trip_day_weather_repository_test.dart | 165 ++++++++++++++++++ 4 files changed, 309 insertions(+), 41 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 cd8e73af6d..046e2030c7 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:drift/drift.dart'; import 'package:submersion/core/constants/enums.dart'; @@ -9,7 +10,7 @@ import 'package:submersion/core/services/sync/sync_event_bus.dart'; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' as domain; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' - show tripDayWeatherRowId; + show tripDayMillis, tripDayWeatherRowId; /// Reads and writes stored per-day trip weather. /// @@ -29,8 +30,7 @@ class TripDayWeatherRepository { /// calendar day, invisible to every midnight-keyed lookup and refetched on /// every view. Reads normalize too, because a row can also arrive through /// sync from a peer, bypassing this class entirely. - static int _dayKey(DateTime date) => - DateTime(date.year, date.month, date.day).millisecondsSinceEpoch; + static int _dayKey(DateTime date) => tripDayMillis(date); /// Emits whenever `trip_day_weather` changes, so the display provider /// refreshes after a backfill write or a sync import. @@ -38,15 +38,26 @@ class TripDayWeatherRepository { _db.tableUpdates(TableUpdateQuery.onTable(_db.tripDayWeather)); /// Stored weather for a trip, keyed by `date.millisecondsSinceEpoch`. + /// + /// 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 + /// gets there in the first place. Future> getForTrip(String tripId) async { try { final rows = await (_db.select( _db.tripDayWeather, )..where((t) => t.tripId.equals(tripId))).get(); - return { - for (final row in rows) - _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)): _mapRow(row), - }; + + final winners = {}; + for (final row in rows) { + final day = _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)); + final held = winners[day]; + winners[day] = held == null + ? row + : _preferred(held, row, tripId: tripId, dayMillis: day); + } + + return winners.map((day, row) => MapEntry(day, _mapRow(row))); } catch (e, stackTrace) { _log.error( 'Failed to read weather for trip: $tripId', @@ -71,37 +82,67 @@ class TripDayWeatherRepository { dayMillis: dateMillis, ); + final sameDay = await _rowsForDay( + tripId: weather.tripId, + dayMillis: dateMillis, + ); // Only to preserve createdAt across an update; insertOnConflictUpdate - // would otherwise overwrite it with this write's timestamp. - final existing = await (_db.select( - _db.tripDayWeather, - )..where((t) => t.id.equals(id))).getSingleOrNull(); - - await _db - .into(_db.tripDayWeather) - .insertOnConflictUpdate( - TripDayWeatherCompanion( - id: Value(id), - tripId: Value(weather.tripId), - date: Value(dateMillis), - latitude: Value(weather.latitude), - longitude: Value(weather.longitude), - airTemp: Value(weather.airTemp), - cloudCover: Value(weather.cloudCover?.name), - precipitation: Value(weather.precipitation?.name), - windSpeed: Value(weather.windSpeed), - windDirection: Value(weather.windDirection?.name), - humidity: Value(weather.humidity), - surfacePressure: Value(weather.surfacePressure), - weatherCode: Value(weather.weatherCode), - weatherSource: Value(weather.weatherSource.name), - fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), - createdAt: Value( - existing?.createdAt ?? weather.createdAt.millisecondsSinceEpoch, + // would otherwise overwrite it with this write's timestamp. A stray is + // this day under an old id rather than a different record, so the day + // keeps the age it already had when one is absorbed. + final createdAt = sameDay.map((r) => r.createdAt).minOrNull; + final strays = sameDay.where((r) => r.id != id).map((r) => r.id).toList(); + + await _db.transaction(() async { + // Strays go before the insert, not after. insertOnConflictUpdate + // targets the primary key, so a stray sitting on this same + // (trip_id, date) is not a conflict it can absorb: the insert misses + // the ON CONFLICT target and hits the unique index instead, which + // throws and fails the whole write. One transaction, so a day is + // never left with its old row deleted and no new one in its place. + if (strays.isNotEmpty) { + await (_db.delete( + _db.tripDayWeather, + )..where((t) => t.id.isIn(strays))).go(); + } + + await _db + .into(_db.tripDayWeather) + .insertOnConflictUpdate( + TripDayWeatherCompanion( + id: Value(id), + tripId: Value(weather.tripId), + date: Value(dateMillis), + latitude: Value(weather.latitude), + longitude: Value(weather.longitude), + airTemp: Value(weather.airTemp), + cloudCover: Value(weather.cloudCover?.name), + precipitation: Value(weather.precipitation?.name), + windSpeed: Value(weather.windSpeed), + windDirection: Value(weather.windDirection?.name), + humidity: Value(weather.humidity), + surfacePressure: Value(weather.surfacePressure), + weatherCode: Value(weather.weatherCode), + weatherSource: Value(weather.weatherSource.name), + fetchedAt: Value(weather.fetchedAt.millisecondsSinceEpoch), + createdAt: Value( + createdAt ?? weather.createdAt.millisecondsSinceEpoch, + ), + updatedAt: Value(now), ), - updatedAt: Value(now), - ), - ); + ); + }); + + // After the row is in place, so a failed write leaves no tombstone for + // a day that still has its original row. A stray is a synced record: + // dropping it without one lets the peer that sent it hand it back on + // the next pull. + for (final stray in strays) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: stray, + ); + } await _syncRepository.markRecordPending( entityType: 'tripDayWeather', @@ -150,6 +191,56 @@ class TripDayWeatherRepository { } } + /// Every stored row for this trip that falls on [dayMillis]'s calendar day. + /// + /// Filtered in Dart rather than SQL: local midnight is not something SQLite + /// can derive from the stored epoch millis without knowing the zone and its + /// DST history. A trip holds one row per day, so the scan is a few dozen + /// rows at most. + Future> _rowsForDay({ + required String tripId, + required int dayMillis, + }) async { + final rows = await (_db.select( + _db.tripDayWeather, + )..where((t) => t.tripId.equals(tripId))).get(); + return rows + .where( + (r) => + _dayKey(DateTime.fromMillisecondsSinceEpoch(r.date)) == dayMillis, + ) + .toList(); + } + + /// Which of two rows for the same calendar day to show. + /// + /// Two rows reach one day only when [upsert] did not write one of them: a + /// peer on a build that predates the derived id, or a database written + /// before this class normalized. [upsert] clears them out, but a read can + /// land between a sync import and the next write, and it cannot tidy up + /// itself: a delete here would fire the table tick that the display + /// provider subscribes to and invalidate the read in flight. So it chooses. + /// + /// The canonical row wins, so what shows now is what the next upsert keeps. + /// Failing that the most recently updated wins, and an exact tie falls back + /// to the id, so the answer never depends on the order SQLite returned the + /// rows in. + TripDayWeatherData _preferred( + TripDayWeatherData a, + TripDayWeatherData b, { + required String tripId, + required int dayMillis, + }) { + final canonicalId = tripDayWeatherRowId( + tripId: tripId, + dayMillis: dayMillis, + ); + if (a.id == canonicalId) return a; + if (b.id == canonicalId) return b; + if (a.updatedAt != b.updatedAt) return a.updatedAt > b.updatedAt ? a : b; + return a.id.compareTo(b.id) <= 0 ? a : b; + } + domain.TripDayWeather _mapRow(TripDayWeatherData row) { return domain.TripDayWeather( id: row.id, diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index ed63935617..227b404b6b 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -8,6 +8,15 @@ import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; /// Never change: the ids already stored depend on it. const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; +/// 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; + /// Deterministic row id for one trip day. /// /// The day is the identity, so the id must be derived from it rather than @@ -18,8 +27,8 @@ const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; /// target entirely and hits the index instead. That throws inside the merge /// transaction and aborts the whole sync pull. /// -/// [dayMillis] must already be normalized to local midnight; the repository -/// does that before calling here. +/// [dayMillis] must already be normalized to local midnight; pass it through +/// [tripDayMillis]. String tripDayWeatherRowId({required String tripId, required int dayMillis}) => const Uuid().v5(kTripDayWeatherNamespace, '$tripId|$dayMillis'); 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 e16b69db6b..2591b4ff15 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -66,10 +66,13 @@ final tripDayWeatherBackfillProvider = FutureProvider.family(( if (weather == null) continue; final now = DateTime.now(); + final dayMillis = tripDayMillis(target.date); final row = TripDayWeather( - // Ignored by the repository, which derives the id from (trip, day) so - // every device converges on one row. - id: '', + // The repository derives this same id from (trip, day) and never takes + // the caller's, so every device converges on one row. Derived here too + // rather than left as a placeholder: an entity carrying an id that is + // not its own reaches logs and any future validation as a lie. + id: tripDayWeatherRowId(tripId: tripId, dayMillis: dayMillis), tripId: tripId, date: target.date, latitude: target.latitude, diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart index 503be868e0..4379f53a3d 100644 --- a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -1,5 +1,9 @@ +import 'package:drift/drift.dart' show Value; import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart' as db; +import 'package:submersion/core/services/database_service.dart'; import 'package:submersion/features/trips/data/repositories/trip_day_weather_repository.dart'; import 'package:submersion/features/trips/data/repositories/trip_repository.dart'; import 'package:submersion/features/trips/domain/entities/trip.dart'; @@ -203,4 +207,165 @@ void main() { expect(emissions, isNotEmpty); }); }); + + group('rows upsert did not write', () { + // Every row in this group goes straight into the table, because that is + // the only way one can carry an id other than the derived one: a peer on + // an older build of this feature, or a database written before the id + // became deterministic. Reconciling rows it did not create is exactly + // what the repository is being asked to do here. + Future insertRaw({ + required String id, + required DateTime date, + double? airTemp, + int updatedAt = 0, + int createdAt = 0, + }) async { + await DatabaseService.instance.database + .into(DatabaseService.instance.database.tripDayWeather) + .insert( + db.TripDayWeatherCompanion( + id: Value(id), + tripId: Value(testTripId), + date: Value(date.millisecondsSinceEpoch), + latitude: const Value(12.16), + longitude: const Value(-68.28), + airTemp: Value(airTemp), + weatherSource: Value(WeatherSource.openMeteo.name), + fetchedAt: const Value(0), + createdAt: Value(createdAt), + updatedAt: Value(updatedAt), + ), + ); + } + + Future> allRows() => DatabaseService + .instance + .database + .select(DatabaseService.instance.database.tripDayWeather) + .get(); + + test('upsert replaces a same-day row stored under a foreign id', () async { + // The unique index is on (trip_id, date) but insertOnConflictUpdate + // targets the primary key, so a foreign-id row on the same midnight + // makes the canonical insert miss the conflict target and hit the + // index. Without cleanup this throws rather than merging. + await insertRaw(id: 'from-a-peer', date: day1, airTemp: 10); + + await repository.upsert(sample(airTemp: 25)); + + final rows = await allRows(); + expect(rows, hasLength(1)); + expect(rows.single.airTemp, 25); + expect(rows.single.id, isNot('from-a-peer')); + }); + + test('upsert replaces a same-day row stored off midnight', () async { + await insertRaw( + id: 'from-an-older-build', + date: DateTime(2026, 3, 8, 17, 30), + airTemp: 10, + ); + + await repository.upsert(sample(airTemp: 25)); + + final rows = await allRows(); + expect(rows, hasLength(1)); + expect(rows.single.date, day1.millisecondsSinceEpoch); + expect(rows.single.airTemp, 25); + }); + + test('a row for another day is left alone', () async { + await insertRaw(id: 'other-day', date: day2, airTemp: 10); + + await repository.upsert(sample()); + + final rows = await allRows(); + expect(rows, hasLength(2)); + expect(rows.map((r) => r.id), contains('other-day')); + }); + + test('replacing a stray logs its deletion for sync', () async { + // A stray is a synced record. Dropping it without a tombstone lets the + // peer that sent it hand it straight back on the next pull. + await insertRaw(id: 'from-a-peer', date: day1); + + await repository.upsert(sample()); + + final deletions = await SyncRepository().getAllDeletions(); + expect( + deletions.where( + (d) => + d.entityType == 'tripDayWeather' && d.recordId == 'from-a-peer', + ), + hasLength(1), + ); + }); + + test('upsert keeps the createdAt of the stray it absorbs', () async { + // The stray is this day's row under an old id, not a different record, + // so the day keeps the age it already had. + await insertRaw( + id: 'from-a-peer', + date: DateTime(2026, 3, 8, 17, 30), + createdAt: 1000, + ); + + await repository.upsert(sample()); + + expect((await allRows()).single.createdAt, 1000); + }); + + test( + 'getForTrip prefers the canonical row over a same-day stray', + () async { + // Reads land between a sync import and the next upsert, so the choice + // cannot wait for the write side to tidy up, and it cannot depend on + // the order SQLite happens to return rows in. + await repository.upsert(sample(airTemp: 25)); + await insertRaw( + id: 'from-a-peer', + date: DateTime(2026, 3, 8, 17, 30), + airTemp: 10, + updatedAt: 9999999, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + }, + ); + + test('getForTrip falls back to the most recently updated stray', () async { + await insertRaw( + id: 'peer-a', + date: DateTime(2026, 3, 8, 6), + airTemp: 10, + updatedAt: 100, + ); + await insertRaw( + id: 'peer-b', + date: DateTime(2026, 3, 8, 23), + airTemp: 20, + updatedAt: 200, + ); + + final stored = await repository.getForTrip(testTripId); + + expect(stored, hasLength(1)); + expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 20); + }); + + test('getForTrip does not write while resolving strays', () async { + // Reads stay pure: a cleanup here would fire the table tick and + // invalidate the provider that just read. + await insertRaw(id: 'peer-a', date: DateTime(2026, 3, 8, 6)); + await insertRaw(id: 'peer-b', date: DateTime(2026, 3, 8, 23)); + + await repository.getForTrip(testTripId); + + expect(await allRows(), hasLength(2)); + }); + }); } From c5801500c6c7d867002af66c00479998bed832f4 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 01:48:54 -0400 Subject: [PATCH 14/18] fix(trips): let the weather backfill retry a miss on the next view The provider documented two contradictory things: that a miss is retried on the next view, and that it runs one pass per provider container lifetime. The second was true. A non-autoDispose family serves its completed state to every later navigation, so a transient network failure stood until the app restarted, which is the opposite of the policy the miss-writes-no-row rule exists to support. autoDispose ties the pass to the view. Returning to the trip runs it again, skipping the days that succeeded and retrying only those still missing; while the view stays mounted the provider stays alive, so scrolling does not refetch. Both halves are now tested. The display provider stays non-autoDispose deliberately: it rides the table tick and caching its rows across navigation is what makes a revisited trip render its badges immediately. --- .../providers/trip_day_weather_providers.dart | 123 +++++++++--------- .../trip_day_weather_providers_test.dart | 53 ++++++++ 2 files changed, 117 insertions(+), 59 deletions(-) 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 2591b4ff15..d112eb431e 100644 --- a/lib/features/trips/presentation/providers/trip_day_weather_providers.dart +++ b/lib/features/trips/presentation/providers/trip_day_weather_providers.dart @@ -33,67 +33,72 @@ final tripDayWeatherProvider = /// stored and no dive to supply it, then writes what it finds. /// /// The story is its only reactive input, so assigning dives to a trip -/// re-evaluates what is still missing. Not auto-disposed: one pass per trip -/// per provider container lifetime. -final tripDayWeatherBackfillProvider = FutureProvider.family(( - ref, - tripId, -) async { - final story = await ref.watch(tripStoryProvider(tripId).future); - final repository = ref.watch(tripDayWeatherRepositoryProvider); - final service = ref.watch(weatherServiceProvider); +/// re-evaluates what is still missing. +/// +/// autoDispose is what makes the retry policy above true. A miss writes no +/// row precisely so the day is tried again later; a provider that outlived +/// the view would serve its completed state on every subsequent navigation +/// and a transient failure would stand until the app restarted. Disposing +/// with the view means returning to the trip runs the pass again, which skips +/// the days that succeeded and retries only the ones still missing. While the +/// view stays mounted the provider stays alive, so scrolling does not refetch. +final tripDayWeatherBackfillProvider = FutureProvider.autoDispose + .family((ref, tripId) async { + final story = await ref.watch(tripStoryProvider(tripId).future); + final repository = ref.watch(tripDayWeatherRepositoryProvider); + final service = ref.watch(weatherServiceProvider); - final stored = await repository.getForTrip(tripId); - final targets = TripDayWeatherBackfill.targetsFor( - story: story, - stored: stored, - ); - if (targets.isEmpty) return; + final stored = await repository.getForTrip(tripId); + final targets = TripDayWeatherBackfill.targetsFor( + story: story, + stored: stored, + ); + if (targets.isEmpty) return; - // Sequential on purpose: a two-week trip would otherwise open with a burst - // of parallel requests, and rows landing one at a time let the day headers - // fill in progressively. - for (final target in targets) { - final weather = await service.fetchWeather( - latitude: target.latitude, - longitude: target.longitude, - date: target.date, - entryTime: target.localNoon, - useLocationTimezone: true, - ); - // A miss writes nothing and is retried on the next view, which is what - // makes this correct against the archive's few-day publication lag. - if (weather == null) continue; + // Sequential on purpose: a two-week trip would otherwise open with a burst + // of parallel requests, and rows landing one at a time let the day headers + // fill in progressively. + for (final target in targets) { + final weather = await service.fetchWeather( + latitude: target.latitude, + longitude: target.longitude, + date: target.date, + entryTime: target.localNoon, + useLocationTimezone: true, + ); + // A miss writes nothing and is retried on the next view, which is what + // makes this correct against the archive's few-day publication lag. + if (weather == null) continue; - final now = DateTime.now(); - final dayMillis = tripDayMillis(target.date); - final row = TripDayWeather( - // The repository derives this same id from (trip, day) and never takes - // the caller's, so every device converges on one row. Derived here too - // rather than left as a placeholder: an entity carrying an id that is - // not its own reaches logs and any future validation as a lie. - id: tripDayWeatherRowId(tripId: tripId, dayMillis: dayMillis), - tripId: tripId, - date: target.date, - latitude: target.latitude, - longitude: target.longitude, - airTemp: weather.airTemp, - cloudCover: weather.cloudCover, - precipitation: weather.precipitation, - windSpeed: weather.windSpeed, - windDirection: weather.windDirection, - humidity: weather.humidity, - surfacePressure: weather.surfacePressure, - weatherCode: weather.weatherCode, - fetchedAt: now, - createdAt: now, - updatedAt: now, - ); + final now = DateTime.now(); + final dayMillis = tripDayMillis(target.date); + final row = TripDayWeather( + // The repository derives this same id from (trip, day) and never takes + // the caller's, so every device converges on one row. Derived here too + // rather than left as a placeholder: an entity carrying an id that is + // not its own reaches logs and any future validation as a lie. + id: tripDayWeatherRowId(tripId: tripId, dayMillis: dayMillis), + tripId: tripId, + date: target.date, + latitude: target.latitude, + longitude: target.longitude, + airTemp: weather.airTemp, + cloudCover: weather.cloudCover, + precipitation: weather.precipitation, + windSpeed: weather.windSpeed, + windDirection: weather.windDirection, + humidity: weather.humidity, + surfacePressure: weather.surfacePressure, + weatherCode: weather.weatherCode, + fetchedAt: now, + createdAt: now, + updatedAt: now, + ); - // A row the header could render nothing from is worse than no row: it - // would suppress the retry that a later archive update would satisfy. - if (!row.hasRenderableWeather) continue; + // A row the header could render nothing from is worse than no row: it + // would suppress the retry that a later archive update would satisfy. + if (!row.hasRenderableWeather) continue; - await repository.upsert(row); - } -}); + await repository.upsert(row); + } + }); diff --git a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart index 27d1646aea..d153fedb10 100644 --- a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +++ b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart @@ -215,6 +215,59 @@ void main() { expect(repository.upserts.single.date, DateTime(2026, 3, 9)); }); + test('a miss is retried when the trip is viewed again', () async { + // The documented policy is that a miss writes nothing and is retried on + // the next view. A provider that stays alive for the container's + // lifetime would serve its completed state instead, so a transient + // failure would not be retried until the app restarted. + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + calls++; + return http.Response('', 500); + }), + repository: repository, + ); + + // First visit: the view mounts, watches, and the pass runs. + final first = container.listen( + tripDayWeatherBackfillProvider('trip-1'), + (_, _) {}, + ); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + expect(calls, 2, reason: 'two days, both missing'); + + // Leaving the trip drops the last listener. + first.close(); + await Future.delayed(Duration.zero); + + // Returning to the trip must run the pass again. + container.listen(tripDayWeatherBackfillProvider('trip-1'), (_, _) {}); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 4); + }); + + test('the pass does not re-run while the view stays mounted', () async { + var calls = 0; + final repository = FakeTripDayWeatherRepository(); + final container = containerWith( + client: MockClient((_) async { + calls++; + return http.Response('', 500); + }), + repository: repository, + ); + + container.listen(tripDayWeatherBackfillProvider('trip-1'), (_, _) {}); + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + // A rebuild re-watches; that must not start another pass. + await container.read(tripDayWeatherBackfillProvider('trip-1').future); + + expect(calls, 2); + }); + test('fetches run one at a time', () async { final gate = Completer(); var started = 0; From 9f3ef4b87dee94db1ff21437f6efbb29da0e07db Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 01:52:23 -0400 Subject: [PATCH 15/18] fix(trips): commit the weather stray tombstones with their delete The strays were deleted inside the transaction but tombstoned after it, so a logDeletion failure could leave a stray deleted locally with nothing to stop the peer that sent it from handing it back. I had justified that ordering as the safe direction, on the assumption that a resurrected stray would be cleaned up by the next upsert for the day. It would not. TripDayWeatherBackfill.targetsFor skips any day that already has a stored row, so once the canonical row exists the day is never upserted again and the stray would sit in the table for good, syncing. The delete, the insert, and both pieces of sync bookkeeping now commit together, with notifyLocalChange still fired after the commit. This is the pattern the #553 review established in BuddyRepository.deleteBuddy, for the same reason: a row deleted without its tombstone resurrects on the next sync. --- .../trip_day_weather_repository.dart | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 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 046e2030c7..3a07cafc19 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -93,13 +93,20 @@ class TripDayWeatherRepository { final createdAt = sameDay.map((r) => r.createdAt).minOrNull; final strays = sameDay.where((r) => r.id != id).map((r) => r.id).toList(); + // Atomic, following the pattern the #553 review established in + // BuddyRepository.deleteBuddy: the row change and its sync bookkeeping + // commit together, and only the observable event is deferred to after + // the commit. A tombstone written outside the transaction could be lost + // while its delete stood, and nothing would ever repair that: the + // backfill skips any day that already has a stored row, so once the + // canonical row exists this day is never upserted again and a + // resurrected stray would sit in the table for good. await _db.transaction(() async { // Strays go before the insert, not after. insertOnConflictUpdate // targets the primary key, so a stray sitting on this same // (trip_id, date) is not a conflict it can absorb: the insert misses // the ON CONFLICT target and hits the unique index instead, which - // throws and fails the whole write. One transaction, so a day is - // never left with its old row deleted and no new one in its place. + // throws and fails the whole write. if (strays.isNotEmpty) { await (_db.delete( _db.tripDayWeather, @@ -131,24 +138,23 @@ class TripDayWeatherRepository { updatedAt: Value(now), ), ); - }); - // After the row is in place, so a failed write leaves no tombstone for - // a day that still has its original row. A stray is a synced record: - // dropping it without one lets the peer that sent it hand it back on - // the next pull. - for (final stray in strays) { - await _syncRepository.logDeletion( + // A stray is a synced record: dropping it without a tombstone lets + // the peer that sent it hand it straight back on the next pull. + for (final stray in strays) { + await _syncRepository.logDeletion( + entityType: 'tripDayWeather', + recordId: stray, + ); + } + + await _syncRepository.markRecordPending( entityType: 'tripDayWeather', - recordId: stray, + recordId: id, + localUpdatedAt: now, ); - } + }); - await _syncRepository.markRecordPending( - entityType: 'tripDayWeather', - recordId: id, - localUpdatedAt: now, - ); SyncEventBus.notifyLocalChange(); } catch (e, stackTrace) { _log.error( From 61c28d013246c071a8957cec92f578d08973f4f9 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 17:31:00 -0400 Subject: [PATCH 16/18] fix(trips): key a trip weather day in UTC, not the device timezone 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. --- ...6-08-26-trip-day-weather-storage-design.md | 13 +++++- .../trip_day_weather_repository.dart | 13 ++---- .../domain/entities/trip_day_weather.dart | 26 ++++++++++- .../widgets/story/trip_story_view.dart | 8 ++-- .../trip_day_weather_repository_test.dart | 36 ++++++++------- .../entities/trip_day_weather_test.dart | 44 +++++++++++++++++++ .../widgets/story/trip_story_view_test.dart | 2 +- 7 files changed, 109 insertions(+), 33 deletions(-) diff --git a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md index 7173932653..7feed8bc67 100644 --- a/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md +++ b/docs/superpowers/specs/2026-08-26-trip-day-weather-storage-design.md @@ -63,9 +63,9 @@ New table `TripDayWeather` in `lib/core/database/database.dart`: | Column | Type | Notes | | --- | --- | --- | -| `id` | text, pk | uuid v4 | +| `id` | text, pk | deterministic UUIDv5 over (`tripId`, day), via `tripDayWeatherRowId` | | `tripId` | text | references `Trips(#id)` | -| `date` | int | epoch **milliseconds** at local midnight, matching what `ItineraryDayRepository` writes for `trip_itinerary_days.date` (the column comment there says "Unix timestamp", but the repository writes `millisecondsSinceEpoch`) | +| `date` | int | epoch **milliseconds** at **UTC** midnight for the calendar day, via `tripDayMillis`. Milliseconds because that is what `ItineraryDayRepository` writes for `trip_itinerary_days.date`, despite that column comment saying "Unix timestamp". UTC rather than local midnight because the value is part of the row identity: a local midnight epoch differs in every timezone, so two devices would key the same trip day differently and never converge | | `latitude` | real | the coordinate the lookup used | | `longitude` | real | the coordinate the lookup used | | `airTemp` | real, nullable | celsius | @@ -84,6 +84,15 @@ New table `TripDayWeather` in `lib/core/database/database.dart`: Unique index on (`tripId`, `date`). +The id is **not** a v4 uuid. It is derived from the day it describes, +`UUIDv5(namespace, "$tripId|$dayMillis")`, following the same convention as +`importedDiveComputerId` and `qualityFindingId`. A per-device v4 would let two +devices store the same day under different primary keys; the serializer upserts +by primary key, so the peer's row would miss the `ON CONFLICT` target and hit +the unique index instead, throwing inside the merge transaction and aborting +the whole sync pull. The repository derives the id itself and ignores whatever +a caller passes. + The stored field set is the full `WeatherData` payload, not just the three fields the day header renders. The API returns them in one response at no extra cost, dive rows already store exactly this set, and a later migration to 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 046e2030c7..ffa4392cd4 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -10,7 +10,7 @@ import 'package:submersion/core/services/sync/sync_event_bus.dart'; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' as domain; import 'package:submersion/features/trips/domain/entities/trip_day_weather.dart' - show tripDayMillis, tripDayWeatherRowId; + show tripDayDate, tripDayMillis, tripDayWeatherRowId; /// Reads and writes stored per-day trip weather. /// @@ -50,7 +50,7 @@ class TripDayWeatherRepository { final winners = {}; for (final row in rows) { - final day = _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)); + final day = _dayKey(tripDayDate(row.date)); final held = winners[day]; winners[day] = held == null ? row @@ -205,10 +205,7 @@ class TripDayWeatherRepository { _db.tripDayWeather, )..where((t) => t.tripId.equals(tripId))).get(); return rows - .where( - (r) => - _dayKey(DateTime.fromMillisecondsSinceEpoch(r.date)) == dayMillis, - ) + .where((r) => _dayKey(tripDayDate(r.date)) == dayMillis) .toList(); } @@ -249,9 +246,7 @@ class TripDayWeatherRepository { // written by an older build or an out-of-date peer can still carry a // time component, and handing that back would put time-bearing dates // into downstream logic. - date: DateTime.fromMillisecondsSinceEpoch( - _dayKey(DateTime.fromMillisecondsSinceEpoch(row.date)), - ), + date: tripDayDate(_dayKey(tripDayDate(row.date))), latitude: row.latitude, longitude: row.longitude, airTemp: row.airTemp, diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index 227b404b6b..7d16ac340c 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -8,14 +8,36 @@ import 'package:submersion/features/trips/domain/entities/trip_story_day.dart'; /// Never change: the ids already stored depend on it. const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; -/// Local midnight for [date], as epoch milliseconds. +/// The calendar day of [date] as epoch milliseconds at UTC midnight. /// /// 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. +/// +/// UTC, not local midnight, and the distinction is the whole point. A local +/// `DateTime(y, m, d)` has a different epoch value in every timezone, so two +/// devices looking at the same trip day would derive different keys, and +/// therefore different row ids, and never converge: each would store and +/// refetch its own copy of the day. Divers cross timezones by definition, and +/// one diver flying home is enough to trigger it. +/// +/// The calendar fields are taken as given rather than converted. `toUtc()` +/// would shift a late evening onto the following day; the trip story already +/// 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. int tripDayMillis(DateTime date) => - DateTime(date.year, date.month, date.day).millisecondsSinceEpoch; + DateTime.utc(date.year, date.month, date.day).millisecondsSinceEpoch; + +/// The calendar day [dayMillis] denotes, as a UTC DateTime at midnight. +/// +/// The inverse of [tripDayMillis], and the only correct way to read a stored +/// day back. `DateTime.fromMillisecondsSinceEpoch` without `isUtc` returns a +/// LOCAL DateTime, so re-extracting y/m/d from it reads the calendar fields in +/// the device's frame: on any negative UTC offset, UTC midnight is the +/// previous evening locally, and the day walks backwards on every round trip. +DateTime tripDayDate(int dayMillis) => + DateTime.fromMillisecondsSinceEpoch(dayMillis, isUtc: true); /// Deterministic row id for one trip day. /// diff --git a/lib/features/trips/presentation/widgets/story/trip_story_view.dart b/lib/features/trips/presentation/widgets/story/trip_story_view.dart index a426887884..c687480488 100644 --- a/lib/features/trips/presentation/widgets/story/trip_story_view.dart +++ b/lib/features/trips/presentation/widgets/story/trip_story_view.dart @@ -249,9 +249,11 @@ class _TripStoryViewState extends ConsumerState Map storedWeather, ) { final day = story.days[index]; - // 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]; + // Keyed through the same helper the repository stores under, so the + // lookup cannot drift from the write. Computing the key inline here was + // how the two came apart: it silently found nothing and every badge + // disappeared. + final stored = storedWeather[tripDayMillis(day.date)]; final showTodayDivider = todayIndex != null && index == todayIndex; const divider = SliverPadding( padding: EdgeInsets.symmetric(horizontal: 16), diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart index 4379f53a3d..6b314b5722 100644 --- a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -89,14 +89,15 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(1)); - final row = stored[day1.millisecondsSinceEpoch]!; + final row = stored[tripDayMillis(day1)]!; expect(row.airTemp, 21.5); expect(row.cloudCover, CloudCover.clear); expect(row.windDirection, CurrentDirection.north); expect(row.weatherCode, 0); expect(row.weatherSource, WeatherSource.openMeteo); expect(row.latitude, 12.16); - expect(row.date, day1); + // A calendar day in a device-independent frame, so UTC. + expect(row.date, DateTime.utc(2026, 3, 8)); }); test('a null payload field round-trips as null', () async { @@ -104,7 +105,7 @@ void main() { final row = (await repository.getForTrip( testTripId, - ))[day1.millisecondsSinceEpoch]!; + ))[tripDayMillis(day1)]!; expect(row.airTemp, isNull); expect(row.cloudCover, isNull); @@ -120,21 +121,21 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(1)); - expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); }); - test('a date with a time component is stored under local midnight', () { + test('a date with a time component is stored under UTC midnight', () { // The repository owns the (trip, date) uniqueness invariant, so it // normalizes rather than trusting every caller to. A row keyed on a - // stray time would be invisible to midnight-keyed lookups and would - // refetch forever. + // stray time would be invisible to day-keyed lookups and would refetch + // forever. UTC, so the key does not move with the device timezone. return () async { await repository.upsert(sample(date: DateTime(2026, 3, 8, 17, 30))); final stored = await repository.getForTrip(testTripId); - expect(stored.keys.single, day1.millisecondsSinceEpoch); - expect(stored[day1.millisecondsSinceEpoch]!.date, day1); + expect(stored.keys.single, tripDayMillis(day1)); + expect(stored[tripDayMillis(day1)]!.date, DateTime.utc(2026, 3, 8)); }(); }); @@ -147,7 +148,7 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(1)); - expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); }); test('two different days both persist', () async { @@ -157,7 +158,7 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(2)); - expect(stored[day2.millisecondsSinceEpoch]!.airTemp, 19); + expect(stored[tripDayMillis(day2)]!.airTemp, 19); }); test('getForTrip is scoped to one trip', () async { @@ -271,7 +272,7 @@ void main() { final rows = await allRows(); expect(rows, hasLength(1)); - expect(rows.single.date, day1.millisecondsSinceEpoch); + expect(rows.single.date, tripDayMillis(day1)); expect(rows.single.airTemp, 25); }); @@ -333,20 +334,23 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(1)); - expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 25); + expect(stored[tripDayMillis(day1)]!.airTemp, 25); }, ); test('getForTrip falls back to the most recently updated stray', () async { + // Off-midnight strays within the same UTC day. "Same day" is a UTC + // question now: a row stored at 23:00 local on a negative offset falls + // on the following UTC day and is genuinely a different day's row. await insertRaw( id: 'peer-a', - date: DateTime(2026, 3, 8, 6), + date: DateTime.utc(2026, 3, 8, 6), airTemp: 10, updatedAt: 100, ); await insertRaw( id: 'peer-b', - date: DateTime(2026, 3, 8, 23), + date: DateTime.utc(2026, 3, 8, 23), airTemp: 20, updatedAt: 200, ); @@ -354,7 +358,7 @@ void main() { final stored = await repository.getForTrip(testTripId); expect(stored, hasLength(1)); - expect(stored[day1.millisecondsSinceEpoch]!.airTemp, 20); + expect(stored[tripDayMillis(day1)]!.airTemp, 20); }); test('getForTrip does not write while resolving strays', () async { diff --git a/test/features/trips/domain/entities/trip_day_weather_test.dart b/test/features/trips/domain/entities/trip_day_weather_test.dart index 5b9334fd53..c3a8bb8f38 100644 --- a/test/features/trips/domain/entities/trip_day_weather_test.dart +++ b/test/features/trips/domain/entities/trip_day_weather_test.dart @@ -28,6 +28,50 @@ void main() { ); } + group('tripDayMillis', () { + test('is the calendar day in UTC, not the device local midnight', () { + // The key must not depend on where the device is standing. A local + // DateTime(y, m, d) has a different epoch value in every timezone, so + // two devices would derive different row ids for the same calendar day + // and never converge. + expect( + tripDayMillis(DateTime(2026, 3, 8, 17, 30)), + DateTime.utc(2026, 3, 8).millisecondsSinceEpoch, + ); + }); + + test('takes the calendar fields as given, never shifting the day', () { + // Guards the wrong fix: converting with toUtc() would move a late + // evening local time onto the following calendar day. + expect( + tripDayMillis(DateTime(2026, 3, 8, 23, 59)), + tripDayMillis(DateTime.utc(2026, 3, 8, 0, 1)), + ); + }); + + test('distinct days stay distinct', () { + expect( + tripDayMillis(DateTime(2026, 3, 8)), + isNot(tripDayMillis(DateTime(2026, 3, 9))), + ); + }); + }); + + group('tripDayWeatherRowId', () { + test('is stable for one calendar day regardless of the time given', () { + expect( + tripDayWeatherRowId( + tripId: 't1', + dayMillis: tripDayMillis(DateTime(2026, 3, 8, 1)), + ), + tripDayWeatherRowId( + tripId: 't1', + dayMillis: tripDayMillis(DateTime(2026, 3, 8, 22)), + ), + ); + }); + }); + group('hasRenderableWeather', () { test('air temperature alone counts', () { expect(weather(airTemp: 24).hasRenderableWeather, isTrue); diff --git a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart index b7792afa8e..8083c1df94 100644 --- a/test/features/trips/presentation/widgets/story/trip_story_view_test.dart +++ b/test/features/trips/presentation/widgets/story/trip_story_view_test.dart @@ -426,7 +426,7 @@ void main() { tester, story, tripDayWeather: { - surfaceDate.millisecondsSinceEpoch: TripDayWeather( + tripDayMillis(surfaceDate): TripDayWeather( id: 'w1', tripId: trip.id, date: surfaceDate, From 70ca2ba23a0cdbb4a2c9a04d9a4cfaefcab20644 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 17:40:11 -0400 Subject: [PATCH 17/18] docs(trips): correct the day-key comments the UTC move left behind 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. --- .../trip_day_weather_repository.dart | 12 +++++---- .../domain/entities/trip_day_weather.dart | 26 +++++++++++++------ 2 files changed, 25 insertions(+), 13 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 ffa4392cd4..cf2b826d4c 100644 --- a/lib/features/trips/data/repositories/trip_day_weather_repository.dart +++ b/lib/features/trips/data/repositories/trip_day_weather_repository.dart @@ -22,7 +22,7 @@ class TripDayWeatherRepository { final SyncRepository _syncRepository = SyncRepository(); final _log = LoggerService.forClass(TripDayWeatherRepository); - /// Local midnight for [date], as epoch milliseconds. + /// The UTC-midnight day key for [date], as epoch milliseconds. /// /// The day is the identity, so normalizing here is what actually enforces /// the (trip, date) uniqueness intent. A caller that passes a DateTime with @@ -193,10 +193,12 @@ class TripDayWeatherRepository { /// Every stored row for this trip that falls on [dayMillis]'s calendar day. /// - /// Filtered in Dart rather than SQL: local midnight is not something SQLite - /// can derive from the stored epoch millis without knowing the zone and its - /// DST history. A trip holds one row per day, so the scan is a few dozen - /// rows at most. + /// Filtered in Dart rather than SQL. It could be pushed down now that the + /// key is UTC, since the day is plain integer arithmetic on the stored + /// millis with no zone or DST to consult, but there is nothing to gain: a + /// trip holds one row per day, so the scan is a few dozen rows, and keeping + /// the rule in one Dart function is what stops it drifting from + /// [tripDayMillis]. Future> _rowsForDay({ required String tripId, required int dayMillis, diff --git a/lib/features/trips/domain/entities/trip_day_weather.dart b/lib/features/trips/domain/entities/trip_day_weather.dart index 7d16ac340c..c8fb0108c1 100644 --- a/lib/features/trips/domain/entities/trip_day_weather.dart +++ b/lib/features/trips/domain/entities/trip_day_weather.dart @@ -29,13 +29,21 @@ const String kTripDayWeatherNamespace = '3f1c8a52-9e47-4d6b-8b3a-16c9d0f27e45'; int tripDayMillis(DateTime date) => DateTime.utc(date.year, date.month, date.day).millisecondsSinceEpoch; -/// The calendar day [dayMillis] denotes, as a UTC DateTime at midnight. +/// The instant [dayMillis] denotes, read in UTC. /// -/// The inverse of [tripDayMillis], and the only correct way to read a stored -/// day back. `DateTime.fromMillisecondsSinceEpoch` without `isUtc` returns a -/// LOCAL DateTime, so re-extracting y/m/d from it reads the calendar fields in -/// the device's frame: on any negative UTC offset, UTC midnight is the -/// previous evening locally, and the day walks backwards on every round trip. +/// The only correct way to read a stored day back. `fromMillisecondsSinceEpoch` +/// without `isUtc` returns a LOCAL DateTime, so re-extracting y/m/d from it +/// reads the calendar fields in the device's frame: on any negative UTC +/// offset, UTC midnight is the previous evening locally, and the day walks +/// backwards on every round trip. +/// +/// It reads the value it is given and normalizes nothing, so it is the +/// inverse of [tripDayMillis] only for a value [tripDayMillis] produced. A +/// stored `date` is not guaranteed to be one: rows written before this branch +/// derived ids can carry a time component, and the repository deliberately +/// calls this on those raw values. Run the result back through +/// [tripDayMillis] whenever you need the normalized day rather than the +/// instant as stored. DateTime tripDayDate(int dayMillis) => DateTime.fromMillisecondsSinceEpoch(dayMillis, isUtc: true); @@ -49,8 +57,10 @@ DateTime tripDayDate(int dayMillis) => /// target entirely and hits the index instead. That throws inside the merge /// transaction and aborts the whole sync pull. /// -/// [dayMillis] must already be normalized to local midnight; pass it through -/// [tripDayMillis]. +/// [dayMillis] must already be the normalized UTC-midnight day key; pass it +/// through [tripDayMillis]. Local midnight would defeat the purpose: its epoch +/// value differs in every timezone, so the derived id would too and the two +/// devices this exists to converge would not. String tripDayWeatherRowId({required String tripId, required int dayMillis}) => const Uuid().v5(kTripDayWeatherNamespace, '$tripId|$dayMillis'); From fc2ede89cfc888d832ac9442c59ec4860192a4db Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 17:49:57 -0400 Subject: [PATCH 18/18] fix(trips): ask for a stored day with the key it was stored under The backfill's skip check was the one read site the UTC move missed. It built its lookup from a local DateTime's epoch while getForTrip keys by tripDayMillis, which is UTC midnight. The two agree only at UTC+0, so on any other device every stored day read as missing and was fetched again on every view: exactly the behaviour storing the rows exists to stop. The tests could not have caught it. Each one keyed its stored fixture the same local way the buggy lookup did, so both sides moved together and the mismatch stayed invisible in any timezone. They now key through tripDayMillis, which is what getForTrip actually returns, and one names the invariant outright. The repository's raw-insert fixtures had the same shape: rows placed at a local DateTime's epoch land on a neighbouring UTC day under a large enough offset and stop being strays for the day under test. They now build from the day key plus an explicit offset, so they assert reconciliation rather than timezone arithmetic. Two had already been half-migrated to DateTime.utc. The trip_day_weather.date schema comment still described local midnight, which is the scheme this branch replaced, and it sits on the column that is half the row identity. Verified in four timezones, including a half-hour offset: 634 trips tests pass under UTC, America/New_York, Asia/Tokyo and Australia/Adelaide. Before the fix the new tests fail under all three non-UTC zones. --- lib/core/database/database.dart | 9 +++- .../services/trip_day_weather_backfill.dart | 11 +++-- .../trip_day_weather_repository_test.dart | 44 ++++++++++++++----- .../trip_day_weather_backfill_test.dart | 30 +++++++++++-- .../trip_day_weather_providers_test.dart | 5 ++- 5 files changed, 77 insertions(+), 22 deletions(-) diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 91dddf5a62..ea102b5a04 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -152,8 +152,13 @@ class TripDayWeather extends Table { TextColumn get id => text()(); TextColumn get tripId => text().references(Trips, #id)(); - /// Local midnight for the day, as epoch milliseconds (the convention - /// TripItineraryDays.date is written with). + /// UTC midnight for the day, as epoch milliseconds (milliseconds being the + /// convention TripItineraryDays.date is written with). + /// + /// UTC rather than local: this column is half the row identity, and it + /// feeds the derived id. A local midnight epoch differs in every timezone, + /// so two devices would key the same trip day differently and never + /// converge. Write it through tripDayMillis. IntColumn get date => integer()(); /// The coordinates the lookup actually used, so a row records what it was 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 0842cfa18f..5cd9a66bca 100644 --- a/lib/features/trips/domain/services/trip_day_weather_backfill.dart +++ b/lib/features/trips/domain/services/trip_day_weather_backfill.dart @@ -56,11 +56,14 @@ class TripDayWeatherBackfill { // A historical archive has nothing for a day that has not happened. if (day.kind == TripStoryDayKind.future) continue; - // Normalize to midnight: stored rows are keyed on midnight millis, so a - // stray time component would never match and the day would refetch on - // every view. + // 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 + // thing this whole feature exists to stop. final date = DateTime(day.date.year, day.date.month, day.date.day); - if (stored.containsKey(date.millisecondsSinceEpoch)) continue; + if (stored.containsKey(tripDayMillis(date))) continue; // nearestPointForDay walks outward from the day, so a dive-free day // between two dived days borrows the closer one's coordinates. A trip diff --git a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart index 6b314b5722..c31e36c9ae 100644 --- a/test/features/trips/data/repositories/trip_day_weather_repository_test.dart +++ b/test/features/trips/data/repositories/trip_day_weather_repository_test.dart @@ -215,9 +215,16 @@ void main() { // an older build of this feature, or a database written before the id // became deterministic. Reconciling rows it did not create is exactly // what the repository is being asked to do here. + // Built from the day key plus an explicit offset, never from a local + // DateTime's epoch. The day is UTC midnight, so a local value lands on a + // neighbouring UTC day under a large enough offset and stops being a + // stray for the day under test: the fixture would be asserting timezone + // arithmetic instead of reconciliation, which is how these passed at + // UTC+0 and failed at UTC+9. Future insertRaw({ required String id, - required DateTime date, + required DateTime day, + Duration offset = Duration.zero, double? airTemp, int updatedAt = 0, int createdAt = 0, @@ -228,7 +235,7 @@ void main() { db.TripDayWeatherCompanion( id: Value(id), tripId: Value(testTripId), - date: Value(date.millisecondsSinceEpoch), + date: Value(tripDayMillis(day) + offset.inMilliseconds), latitude: const Value(12.16), longitude: const Value(-68.28), airTemp: Value(airTemp), @@ -251,7 +258,7 @@ void main() { // targets the primary key, so a foreign-id row on the same midnight // makes the canonical insert miss the conflict target and hit the // index. Without cleanup this throws rather than merging. - await insertRaw(id: 'from-a-peer', date: day1, airTemp: 10); + await insertRaw(id: 'from-a-peer', day: day1, airTemp: 10); await repository.upsert(sample(airTemp: 25)); @@ -264,7 +271,8 @@ void main() { test('upsert replaces a same-day row stored off midnight', () async { await insertRaw( id: 'from-an-older-build', - date: DateTime(2026, 3, 8, 17, 30), + day: day1, + offset: const Duration(hours: 17, minutes: 30), airTemp: 10, ); @@ -277,7 +285,7 @@ void main() { }); test('a row for another day is left alone', () async { - await insertRaw(id: 'other-day', date: day2, airTemp: 10); + await insertRaw(id: 'other-day', day: day2, airTemp: 10); await repository.upsert(sample()); @@ -289,7 +297,7 @@ void main() { test('replacing a stray logs its deletion for sync', () async { // A stray is a synced record. Dropping it without a tombstone lets the // peer that sent it hand it straight back on the next pull. - await insertRaw(id: 'from-a-peer', date: day1); + await insertRaw(id: 'from-a-peer', day: day1); await repository.upsert(sample()); @@ -308,7 +316,8 @@ void main() { // so the day keeps the age it already had. await insertRaw( id: 'from-a-peer', - date: DateTime(2026, 3, 8, 17, 30), + day: day1, + offset: const Duration(hours: 17, minutes: 30), createdAt: 1000, ); @@ -326,7 +335,8 @@ void main() { await repository.upsert(sample(airTemp: 25)); await insertRaw( id: 'from-a-peer', - date: DateTime(2026, 3, 8, 17, 30), + day: day1, + offset: const Duration(hours: 17, minutes: 30), airTemp: 10, updatedAt: 9999999, ); @@ -344,13 +354,15 @@ void main() { // on the following UTC day and is genuinely a different day's row. await insertRaw( id: 'peer-a', - date: DateTime.utc(2026, 3, 8, 6), + day: day1, + offset: const Duration(hours: 6), airTemp: 10, updatedAt: 100, ); await insertRaw( id: 'peer-b', - date: DateTime.utc(2026, 3, 8, 23), + day: day1, + offset: const Duration(hours: 23), airTemp: 20, updatedAt: 200, ); @@ -364,8 +376,16 @@ void main() { test('getForTrip does not write while resolving strays', () async { // Reads stay pure: a cleanup here would fire the table tick and // invalidate the provider that just read. - await insertRaw(id: 'peer-a', date: DateTime(2026, 3, 8, 6)); - await insertRaw(id: 'peer-b', date: DateTime(2026, 3, 8, 23)); + await insertRaw( + id: 'peer-a', + day: day1, + offset: const Duration(hours: 6), + ); + await insertRaw( + id: 'peer-b', + day: day1, + offset: const Duration(hours: 23), + ); await repository.getForTrip(testTripId); diff --git a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart index 44e1152522..e9f75dcdb2 100644 --- a/test/features/trips/domain/services/trip_day_weather_backfill_test.dart +++ b/test/features/trips/domain/services/trip_day_weather_backfill_test.dart @@ -145,11 +145,13 @@ void main() { }); test('a day with a stored row is skipped', () { + // Keyed the way the repository keys it. getForTrip returns a map keyed + // by tripDayMillis, so keying this fixture any other way would assert a + // contract production never offers: it would pass here and still refetch + // every day on a real device. final story = storyWith([day(index: 0)], points: [pointFor(0)]); final stored = { - DateTime(2026, 3, 8).millisecondsSinceEpoch: storedFor( - DateTime(2026, 3, 8), - ), + tripDayMillis(DateTime(2026, 3, 8)): storedFor(DateTime(2026, 3, 8)), }; expect( @@ -158,6 +160,28 @@ void main() { ); }); + test('the stored lookup is not the device local midnight', () { + // The day key is UTC midnight so two devices converge on one row. A + // lookup built from a local DateTime's epoch agrees with it only at + // UTC+0, so on any other device every stored day reads as missing and + // is refetched on every view, which is the whole thing this feature + // exists to stop. Discriminating only off UTC; run with + // TZ=America/New_York to see it bite. + final story = storyWith([day(index: 0)], points: [pointFor(0)]); + final localKey = DateTime(2026, 3, 8).millisecondsSinceEpoch; + final utcKey = tripDayMillis(DateTime(2026, 3, 8)); + + final stored = {utcKey: storedFor(DateTime(2026, 3, 8))}; + + expect( + TripDayWeatherBackfill.targetsFor(story: story, stored: stored), + isEmpty, + reason: + 'a row stored under the repository key must skip the day ' + '(local key $localKey vs day key $utcKey)', + ); + }); + test('a day with no map point anywhere in the story is skipped', () { final story = storyWith([day(index: 0)]); diff --git a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart index d153fedb10..479fe7c06d 100644 --- a/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart +++ b/test/features/trips/presentation/providers/trip_day_weather_providers_test.dart @@ -186,7 +186,10 @@ void main() { test('a day already stored is not fetched', () async { final repository = FakeTripDayWeatherRepository( stored: { - DateTime(2026, 3, 8).millisecondsSinceEpoch: TripDayWeather( + // Keyed as getForTrip keys it. A local DateTime's epoch matches the + // day key only at UTC+0, so keying it that way would assert a + // contract production never offers. + tripDayMillis(DateTime(2026, 3, 8)): TripDayWeather( id: 'w1', tripId: 'trip-1', date: DateTime(2026, 3, 8),