Fix MacDive UDDF import (Milestone 1 of 4)

CHANGELOG.md

@@ -3,6 +3,38 @@
 All notable changes to Submersion are documented in this file.
 
 
+## Unreleased
+
+### Added
+
+- MacDive UDDF imports now capture substantially richer dive data: boat
+  name and captain, dive operator, surface conditions, weather (stored
+  in the existing weather description field), plus site water type, body
+  of water, and difficulty rating.
+- Cross-format import deduplication: stable per-dive UUIDs from MacDive,
+  Shearwater Cloud, Subsurface SSRF, and generic UDDF are now preserved on
+  the `dive_data_sources` sidecar. Re-importing the same dives in a
+  different format no longer creates duplicates.
+
+### Fixed
+
+- MacDive UDDF: equipment / gear now imports correctly. The parser
+  previously only scanned Submersion's private equipment extension,
+  missing the standard UDDF gear location (`<diver><owner><equipment>`)
+  where MacDive and other compliant exporters place their inventory.
+- MacDive UDDF: `<equipmentused><link ref>` is now captured from both
+  `<informationbeforedive>` and `<informationafterdive>`.
+- MacDive UDDF: `<surfaceintervalbeforedive><infinity/></…>` is now
+  explicitly handled as "no prior dive" rather than relying on silent
+  int-parse failure.
+
+### Known limitations (to be addressed in a follow-up)
+
+- Gas switch markers (`<switchmix ref>`) in MacDive dive profiles are
+  parsed but not yet persisted to the profile samples table. A future
+  milestone will wire them through, likely via the dive-events table.
+
+
 ## 1.4.6 (2026-04-22)
 
 ### Bug Fixes

dart_test.yaml

@@ -4,3 +4,7 @@ tags:
     # Run explicitly: flutter test --run-skipped --tags performance
     # Or: flutter test --run-skipped test/performance/
     skip: "Slow performance tests; run with --run-skipped --tags performance"
+  real-data:
+    # Real-data tests use actual user exports and may take time to run.
+    # Run explicitly: flutter test --run-skipped --tags real-data
+    skip: "Real-data regression tests; run with --run-skipped --tags real-data"

docs/superpowers/plans/2026-04-21-macdive-uddf-gap-fill.md

@@ -12,11 +12,49 @@
 
 ---
 
+## Milestone 1 Status — COMPLETE
+
+- All 12 tasks landed or explicitly skipped (Task 3 proven non-bug; see Task 3 section).
+- Schema bumped v69 → v70 (source_uuid on dive_data_sources) → v71
+  (6 new dive + site metadata columns: boat_name, boat_captain,
+  dive_operator, surface_conditions on dives; water_type, body_of_water
+  on dive_sites). Task 8 and Task 10's snippets below show an earlier
+  plan that also added `dive_number_of_day`; see "Post-completion
+  revision" note directly below.
+- Cross-format import dedup now works via `dive_data_sources.source_uuid`.
+- Parser-to-DB gap closed for MacDive rich fields. `weather` now lands
+  on the existing weather_description column. difficulty continues to
+  flow through the DiveSite entity path.
+- Dropped from scope: personalMode, altitudeMode, signature, site flag
+  (niche / redundant). LinkRefKind/LinkRefIndex (Task 2) also removed
+  after Task 3 investigation showed the bug they targeted didn't exist.
+- Known limitation: profile `gasMixRef` (from `<switchmix ref>`) is
+  parsed but not yet persisted to the profile samples table — deferred
+  to a future milestone, likely via dive-events.
+- Real-sample regression test passes (gated behind `@Tags(['real-data'])`).
+
+### Post-completion revision: dive_number_of_day is derived, not stored
+
+Originally this milestone planned a `dive_number_of_day` INT column on
+`dives`, populated from UDDF `<divenumberofday>`. After landing,
+review found the value is trivially derivable from `diveDateTime`
+(`ROW_NUMBER() OVER (PARTITION BY DATE(diveDateTime) ORDER BY
+diveDateTime)`) and has no current reader in the app. Storing it would
+also desync the moment a diver manually adds a dive between two
+imported dives. The column, its v71 ALTER, parser extraction,
+`IncomingDiveData` field, and importer write path were all removed.
+Task 8's "Add fields to the domain model" snippet and Task 10's DB
+write snippet in this document are the pre-revision plan; they are
+kept for historical context. `<divenumberofday>` is now ignored on
+import and computed on demand if/when the UI needs it.
+
+---
+
 ## File Structure
 
 | File | Role | New / Modified |
 |---|---|---|
-| `lib/core/database/database.dart` | Add `sourceUuid` columns to `Dives`, `DiveSites`, `Buddies`, `Gear`, `Tags`, `Certifications`, `Species`, `DiveTypes`, `Trips`. Bump schema version and add migration step. | Modified |
+| `lib/core/database/database.dart` | Add single `sourceUuid TEXT NULL` column to the existing `DiveDataSources` 1:N sidecar table. That's where per-source dive identity already lives (alongside `rawFingerprint`, `sourceFormat`, `sourceFileName`, `computerSerial`). Bump schema version 69 → 70 and add migration step. No changes to `dives`, `dive_sites`, `buddies`, or any other top-level entity tables. | Modified |
 | `lib/core/services/export/uddf/dialects/macdive_dialect.dart` | Add `<infinity/>` normalization for surface interval; preserve idempotency for all existing rewrites. | Modified |
 | `lib/core/services/export/uddf/uddf_import_parsers.dart` | Add `resolveLinkRef(XmlElement, Map<String, LinkRefKind>)` helper that resolves a `<link ref="…"/>` against a pre-built index of top-level IDs and returns the entity kind. | Modified |
 | `lib/core/services/export/uddf/uddf_full_import_service.dart` | Build ID→kind index once per import; use the helper inside `informationbeforedive` / `equipmentused` parsing; extract newly-supported dive and site fields; store each entity's source UUID on its dive/site/buddy/etc. map under `sourceUuid`. | Modified |
@@ -29,7 +67,9 @@
 
 ---
 
-## Task 1: Schema migration — add `source_uuid` columns
+## Task 1: Schema migration — add `source_uuid` column to `dive_data_sources`
+
+**Design rationale:** The project already has a 1:N `dive_data_sources` sidecar table carrying per-source dive metadata: `raw_fingerprint` (libdivecomputer), `source_format`, `source_file_name`, `computer_serial`, etc. This is where "where this dive came from" already lives. Adding `source_uuid` here (rather than on `dives` or on every top-level table) keeps the schema change minimal (one column, one table) and aligns with the existing architecture. libdivecomputer continues to use its existing `raw_fingerprint` BLOB — different mechanism, same role. The new `source_uuid` column captures the string/UUID identifiers that Shearwater Cloud (`DiveId`), MacDive (UDDF `<dive id>`, XML `<identifier>`, SQLite `ZUUID`), Subsurface SSRF, and generic UDDF all provide.
 
 **Files:**
 - Modify: `lib/core/database/database.dart`
@@ -40,37 +80,22 @@
 Create `test/core/database/source_uuid_migration_test.dart`:
 
 ```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() {
-  test('fresh database has source_uuid column on dives and related tables',
-      () async {
+  test('fresh database has source_uuid column on dive_data_sources', () async {
     final db = AppDatabase.forTesting(NativeDatabase.memory());
-    final tables = [
-      'dives',
-      'dive_sites',
-      'buddies',
-      'gear',
-      'tags',
-      'certifications',
-      'species',
-      'dive_types',
-      'trips',
-    ];
-    for (final table in tables) {
-      final cols = await db
-          .customSelect("PRAGMA table_info('$table')")
-          .get();
-      final names = cols.map((r) => r.data['name'] as String).toSet();
-      expect(
-        names.contains('source_uuid'),
-        isTrue,
-        reason: 'table $table must have source_uuid column',
-      );
-    }
+    final cols = await db
+        .customSelect("PRAGMA table_info('dive_data_sources')")
+        .get();
+    final names = cols.map((r) => r.data['name'] as String).toSet();
+    expect(
+      names.contains('source_uuid'),
+      isTrue,
+      reason: 'dive_data_sources must have source_uuid column',
+    );
     await db.close();
   });
 }
@@ -79,33 +104,32 @@ void main() {
 - [ ] **Step 2: Run test to verify it fails**
 
 Run: `flutter test test/core/database/source_uuid_migration_test.dart`
-Expected: FAIL — `table dives must have source_uuid column`.
+Expected: FAIL — `dive_data_sources must have source_uuid column`.
 
-- [ ] **Step 3: Add columns to each table definition in `database.dart`**
+- [ ] **Step 3: Add the column to `DiveDataSources`**
 
-For each of the nine tables, add:
+In `lib/core/database/database.dart`, locate `class DiveDataSources extends Table` (around line 953). Add a nullable text column near the other per-source identity columns (e.g. immediately after `rawFingerprint`):
 
 ```dart
-TextColumn get sourceUuid => text().nullable()();
+  TextColumn get sourceUuid => text().nullable()();
 ```
 
-Place it at the end of each table class before the `override Set<Column> get primaryKey => …` (if any).
-
 - [ ] **Step 4: Bump schema version and add migration step**
 
-Open `lib/core/database/database.dart`, find the `schemaVersion` getter, bump it by one (e.g. `49 → 50`). In the `MigrationStrategy.onUpgrade`, add a new case matching the new version that runs:
+Find `currentSchemaVersion` (around line 1329) and bump from `69` to `70`.
+
+Locate the existing `dive_data_sources` ALTER migration pattern (around lines 3083-3108 — where `raw_fingerprint`, `descriptor_vendor`, `descriptor_product`, etc. are added with `if (!existing.contains('X')) { await customStatement('ALTER TABLE dive_data_sources ADD COLUMN X Y') }`). Append to the same block (inside the same `if (from < N)` guard or in a new `if (from < 70)` block, matching the file's existing idiom):
 
 ```dart
-if (from < 50) {
-  for (final table in const [
-    'dives', 'dive_sites', 'buddies', 'gear', 'tags',
-    'certifications', 'species', 'dive_types', 'trips',
-  ]) {
-    await m.customStatement('ALTER TABLE $table ADD COLUMN source_uuid TEXT;');
-  }
+if (!existing.contains('source_uuid')) {
+  await customStatement(
+    'ALTER TABLE dive_data_sources ADD COLUMN source_uuid TEXT',
+  );
 }
 ```
 
+Grep the existing migration code for the exact `existing` variable name and reuse it; don't duplicate the `PRAGMA table_info` introspection if it's already in scope.
+
 - [ ] **Step 5: Regenerate drift outputs and run the test**
 
 Run:
@@ -118,9 +142,8 @@ Expected: PASS.
 - [ ] **Step 6: Commit**
 
 ```bash
-git add lib/core/database/database.dart test/core/database/source_uuid_migration_test.dart
-git add lib/core/database/database.g.dart  # generated
-git commit -m "feat(db): add source_uuid columns for cross-format import dedup"
+git add lib/core/database/database.dart lib/core/database/database.g.dart test/core/database/source_uuid_migration_test.dart
+git commit -m "feat(db): add source_uuid to dive_data_sources for cross-format import dedup"
 ```
 
 ---
@@ -202,7 +225,19 @@ git commit -m "feat(uddf): add LinkRefIndex for ref-kind disambiguation"
 
 ---
 
-## Task 3: Build and use the ID index in `UddfFullImportService`
+## Task 3: Build and use the ID index in `UddfFullImportService` — **SKIPPED**
+
+**Investigation outcome:** The plan's premise (that the current parser assumes positional order for `<link ref>` children in `<informationbeforedive>`) turned out to be wrong. The existing `_parseUddfDive` at `uddf_full_import_service.dart:1173-1211` already classifies each link by looking up its `ref` attribute in pre-built entity maps (`sites`, `buddies`, `decoModels`, `diveComputers`). The prescribed failing test passes against unmodified code.
+
+**What the real gap is (deferred):** `_parseFullDive` at lines 557-571 uses string-prefix matching (`ref.startsWith('trip_')`, `'center_'`, `'course_'`) to classify trip/center/course refs. This WOULD fail for MacDive-style UUIDs — but MacDive UDDF (per the user's 29MB sample) does not emit `<trip>` / `<divecenter>` / `<course>` references at all. For Milestone 1 scope (MacDive UDDF), this codepath is not exercised. Revisit in Milestone 3 (MacDive SQLite) if trip/center refs start showing up.
+
+**LinkRefKind / LinkRefIndex from Task 2 are retained** — they cost ~22 lines + 2 tests and may be useful for the Milestone 3 SQLite work where cross-entity UUID resolution matters.
+
+**Status:** Task 3 marked as SKIPPED in the task tracker. No code change for this task.
+
+---
+
+## Task 3 (ORIGINAL, for reference) — not executed
 
 **Files:**
 - Modify: `lib/core/services/export/uddf/uddf_full_import_service.dart`

lib/core/database/database.dart

@@ -126,6 +126,11 @@ class Dives extends Table {
       text().withDefault(const Constant('recreational'))();
   TextColumn get buddy => text().nullable()();
   TextColumn get diveMaster => text().nullable()();
+  // MacDive import fields — common dive metadata
+  TextColumn get boatName => text().nullable()();
+  TextColumn get boatCaptain => text().nullable()();
+  TextColumn get diveOperator => text().nullable()();
+  TextColumn get surfaceConditions => text().nullable()();
   TextColumn get notes => text().withDefault(const Constant(''))();
   TextColumn get siteId => text().nullable().references(DiveSites, #id)();
   IntColumn get rating => integer().nullable()();
@@ -300,6 +305,9 @@ class DiveSites extends Table {
   RealColumn get maxDepth => real().nullable()(); // Deepest point
   TextColumn get difficulty =>
       text().nullable()(); // Beginner, Intermediate, Advanced, Technical
+  // MacDive site metadata
+  TextColumn get waterType => text().nullable()();
+  TextColumn get bodyOfWater => text().nullable()();
   TextColumn get country => text().nullable()();
   TextColumn get region => text().nullable()();
   RealColumn get rating => real().nullable()();
@@ -983,6 +991,7 @@ class DiveDataSources extends Table {
   DateTimeColumn get createdAt => dateTime()();
   BlobColumn get rawData => blob().nullable()();
   BlobColumn get rawFingerprint => blob().nullable()();
+  TextColumn get sourceUuid => text().nullable()();
   TextColumn get descriptorVendor => text().nullable()();
   TextColumn get descriptorProduct => text().nullable()();
   IntColumn get descriptorModel => integer().nullable()();
@@ -1326,7 +1335,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 = 69;
+  static const int currentSchemaVersion = 71;
 
   /// Every schema version that has a migration block in onUpgrade.
   /// Used to calculate progress step counts. When adding a new migration,
@@ -1399,6 +1408,8 @@ class AppDatabase extends _$AppDatabase {
     67,
     68,
     69,
+    70,
+    71,
   ];
 
   /// Returns the number of migration steps that will execute when upgrading
@@ -3244,6 +3255,75 @@ class AppDatabase extends _$AppDatabase {
           }
         }
         if (from < 69) await reportProgress();
+        if (from < 70) {
+          // Migration 70: add source_uuid to dive_data_sources for
+          // cross-format import deduplication (MacDive UUID, Shearwater
+          // DiveId, Subsurface SSRF id, generic UDDF dive id).
+          // libdivecomputer continues to use raw_fingerprint.
+          // Guard: dive_data_sources may not exist in older migration tests.
+          final cols = await customSelect(
+            "PRAGMA table_info('dive_data_sources')",
+          ).get();
+          if (cols.isNotEmpty) {
+            final existing = cols.map((c) => c.read<String>('name')).toSet();
+            if (!existing.contains('source_uuid')) {
+              await customStatement(
+                'ALTER TABLE dive_data_sources ADD COLUMN source_uuid TEXT',
+              );
+            }
+          }
+        }
+        if (from < 70) await reportProgress();
+        if (from < 71) {
+          // Migration 71: add MacDive dive + site metadata fields.
+          final divesCols = await customSelect(
+            "PRAGMA table_info('dives')",
+          ).get();
+          final divesExisting = divesCols
+              .map((r) => r.data['name'] as String)
+              .toSet();
+          if (divesCols.isNotEmpty) {
+            if (!divesExisting.contains('boat_name')) {
+              await customStatement(
+                'ALTER TABLE dives ADD COLUMN boat_name TEXT',
+              );
+            }
+            if (!divesExisting.contains('boat_captain')) {
+              await customStatement(
+                'ALTER TABLE dives ADD COLUMN boat_captain TEXT',
+              );
+            }
+            if (!divesExisting.contains('dive_operator')) {
+              await customStatement(
+                'ALTER TABLE dives ADD COLUMN dive_operator TEXT',
+              );
+            }
+            if (!divesExisting.contains('surface_conditions')) {
+              await customStatement(
+                'ALTER TABLE dives ADD COLUMN surface_conditions TEXT',
+              );
+            }
+          }
+          final sitesCols = await customSelect(
+            "PRAGMA table_info('dive_sites')",
+          ).get();
+          final sitesExisting = sitesCols
+              .map((r) => r.data['name'] as String)
+              .toSet();
+          if (sitesCols.isNotEmpty) {
+            if (!sitesExisting.contains('water_type')) {
+              await customStatement(
+                'ALTER TABLE dive_sites ADD COLUMN water_type TEXT',
+              );
+            }
+            if (!sitesExisting.contains('body_of_water')) {
+              await customStatement(
+                'ALTER TABLE dive_sites ADD COLUMN body_of_water TEXT',
+              );
+            }
+          }
+        }
+        if (from < 71) await reportProgress();
       },
       beforeOpen: (details) async {
         // Enable foreign keys

lib/core/domain/models/incoming_dive_data.dart

@@ -18,6 +18,12 @@ class IncomingDiveData {
   final String? computerSerial;
   final List<DiveProfilePoint> profile;
   final String? siteName;
+  final String? weather;
+  final String? surfaceConditions;
+  final String? boatName;
+  final String? boatCaptain;
+  final String? diveOperator;
+  final String? sourceUuid;
 
   const IncomingDiveData({
     this.startTime,
@@ -30,6 +36,12 @@ class IncomingDiveData {
     this.computerSerial,
     this.profile = const [],
     this.siteName,
+    this.weather,
+    this.surfaceConditions,
+    this.boatName,
+    this.boatCaptain,
+    this.diveOperator,
+    this.sourceUuid,
   });
 
   /// Create from a [DownloadedDive] (dive computer download flow).
@@ -86,6 +98,12 @@ class IncomingDiveData {
       computerSerial: data['diveComputerSerial'] as String?,
       siteName: data['siteName'] as String?,
       profile: profile,
+      weather: data['weather'] as String?,
+      surfaceConditions: data['surfaceConditions'] as String?,
+      boatName: data['boatName'] as String?,
+      boatCaptain: data['boatCaptain'] as String?,
+      diveOperator: data['diveOperator'] as String?,
+      sourceUuid: data['sourceUuid'] as String?,
     );
   }
 }