fix(macdive-sqlite): address PR #256 round 2 review (equipmentRefs, probe dedup, doc fixes)

Round 2 of Copilot's review on PR #256 — 7 items, 5 implemented and 2
doc/rename fixes.

- equipmentRefs (the big one): my earlier pushback was wrong.
  `UddfEntityImporter` DOES resolve `diveData['equipmentRefs']` via
  `equipmentIdMapping`, so SQLite imports now emit per-dive
  equipmentRefs pulled from `diveToGearPks`. Each gear item gets a
  stable `uddfId` (MacDive UUID, with name fallback) so the importer
  can link gear back to dives the same way M2 XML would. Also filters
  name-less gear rows so `_importEquipment` doesn't silently drop
  entries while `equipmentIdMapping` keeps stale refs.
- Format detector: probe SQLite table names ONCE per input, not once
  per flavor check. Previously detection wrote the full byte array to
  a temp file + opened sqlite twice (once for Shearwater, once for
  MacDive). Added `probeSqliteTableNames` on ShearwaterDbReader plus
  sync `matchesTables` on both readers so the detector can probe once
  and match all flavors.
- Doc fix: `BPlistDecoder.decode` docstring no longer lists UIDs as
  unsupported — they've been supported since M3.
- Doc fix: explicit comment at the bplist int-decode case explaining
  the 16-byte best-effort low-64-bit truncation behavior (implicit
  via Dart int overflow on `(value << 8) | byte`).
- Doc+code: `MacDiveDbReader.readAll` now validates required tables
  up front and throws a clear FormatException if any are missing, so
  the docstring's "validated up-front" promise matches reality even
  for callers that bypass `isMacDiveDb`.
- Rename: `samplesBplist` and `rawDataBplist` → `samplesBlob` /
  `rawDataBlob`. The data in these fields is NOT bplist (ZSAMPLES is
  MacDive-proprietary, ZRAWDATA is raw sensor dump); the old names
  would have misled future callers into trying to decode them.
- Tests: 2 new synthetic-DB parser tests covering uddfId emission and
  equipmentRefs resolution; 1 new real-sample test verifying every
  emitted equipmentRef points at an equipment uddfId.

Author: ericgriffin

lib/core/utils/bplist/bplist_decoder.dart

@@ -27,7 +27,7 @@ class BPlistDecoder {
 
   /// Entry point: decode [bytes] into the root [BPlistObject].
   /// Throws [FormatException] if [bytes] is not a valid bplist00 stream
-  /// or uses unsupported type markers (sets, UIDs).
+  /// or uses unsupported type markers (sets).
   static BPlistObject decode(Uint8List bytes) {
     if (bytes.length < 8 + 32) {
       throw const FormatException('bplist00 stream too short');
@@ -79,6 +79,12 @@ class BPlistDecoder {
 
       case 0x1: // int
         // info bits 0..3 encode log2(byteCount): 0->1, 1->2, 2->4, 3->8, 4->16.
+        // 16-byte ints are decoded as a best-effort truncation to the low
+        // 64 bits: Dart's `int` is 64-bit on native platforms, and the
+        // `(value << 8) | byte` accumulator in `_readBigEndianInt` silently
+        // drops the high bytes once the value overflows — which for a big-
+        // endian read keeps exactly the low 64 bits of the source integer.
+        // MacDive has not been seen to emit 16-byte ints in practice.
         final byteCount = 1 << info;
         return BPlistInt(_readBigEndianInt(_bytes, offset + 1, byteCount));
 

lib/features/universal_import/data/services/macdive_db_reader.dart

@@ -18,6 +18,13 @@ class MacDiveDbReader {
   /// have a `ZDIVE` table coincidentally.
   static const _requiredTables = ['ZDIVE', 'ZDIVESITE', 'ZGAS', 'ZTANKANDGAS'];
 
+  /// Synchronous companion to [isMacDiveDb] for callers that have
+  /// already probed the SQLite table set (e.g. the format detector
+  /// runs every DB-flavor check against one probe to avoid doubling
+  /// temp-file I/O).
+  static bool matchesTables(Set<String> tables) =>
+      _requiredTables.every(tables.contains);
+
   /// True when [bytes] is a SQLite database containing every table in
   /// [_requiredTables]. Returns false (doesn't throw) for non-SQLite
   /// inputs or unrelated schemas.
@@ -45,8 +52,10 @@ class MacDiveDbReader {
 
   /// Reads every MacDive table relevant to the import pipeline and
   /// returns a fully populated [MacDiveRawLogbook]. The four tables in
-  /// [_requiredTables] are validated up-front and must exist. For every
-  /// other table (e.g. `ZCRITTER`, `ZBUDDY`, `ZGEARITEM`), a missing or
+  /// [_requiredTables] are validated up-front (a [FormatException] is
+  /// thrown if any are missing) so subsequent reads on them can rely
+  /// on direct `db.select` without per-call guards. For every other
+  /// table (e.g. `ZCRITTER`, `ZBUDDY`, `ZGEARITEM`), a missing or
   /// unpopulated table produces an empty collection rather than an
   /// error — some MacDive schema versions omit tables the user has
   /// never touched.
@@ -57,6 +66,7 @@ class MacDiveDbReader {
       await tmpFile.writeAsBytes(bytes);
       final db = sqlite3.open(tmpPath, mode: OpenMode.readOnly);
       try {
+        _validateRequiredTables(db);
         final unitsPreference = _readUnitsPreference(db);
         final sites = _readSites(db);
         final buddies = _readBuddies(db);
@@ -365,8 +375,8 @@ class MacDiveDbReader {
         weight: _str(r['ZWEIGHT']),
         diveSiteFk: r['ZRELATIONSHIPDIVESITE'] as int?,
         certificationFk: r['ZRELATIONSHIPCERTIFICATION'] as int?,
-        samplesBplist: _bytes(r['ZSAMPLES']),
-        rawDataBplist: _bytes(r['ZRAWDATA']),
+        samplesBlob: _bytes(r['ZSAMPLES']),
+        rawDataBlob: _bytes(r['ZRAWDATA']),
       );
     }).toList();
   }
@@ -408,6 +418,24 @@ class MacDiveDbReader {
     }
   }
 
+  /// Confirms every table in [_requiredTables] exists in [db]. Called
+  /// at the top of [readAll] so the readers below can count on these
+  /// tables being present, and so that a missing-table failure surfaces
+  /// as a clear error instead of a confusing "no such table" from a
+  /// later query.
+  static void _validateRequiredTables(Database db) {
+    final rows = db.select("SELECT name FROM sqlite_master WHERE type='table'");
+    final tables = rows.map<String>((r) => r['name'] as String).toSet();
+    final missing = _requiredTables
+        .where((t) => !tables.contains(t))
+        .toList(growable: false);
+    if (missing.isNotEmpty) {
+      throw FormatException(
+        'MacDive SQLite is missing required tables: ${missing.join(', ')}',
+      );
+    }
+  }
+
   // ---- utilities ----
 
   static String _tmpPath() =>

lib/features/universal_import/data/services/macdive_dive_mapper.dart

@@ -122,14 +122,32 @@ class MacDiveDiveMapper {
     return out;
   }
 
+  /// Returns the stable uddf-style id for a gear row. Prefers the MacDive
+  /// UUID (guaranteed unique per gear item in Core Data); falls back to
+  /// the name so older exports without UUIDs still link. Returns null
+  /// when neither is present — callers should skip the gear entirely in
+  /// that case.
+  static String? _gearUddfId(MacDiveRawGear g) {
+    if (g.uuid.isNotEmpty) return g.uuid;
+    final name = g.name;
+    if (name != null && name.isNotEmpty) return name;
+    return null;
+  }
+
   static List<Map<String, dynamic>> _buildGearMaps(
     MacDiveRawLogbook logbook,
     MacDiveUnitConverter c,
   ) {
     final out = <Map<String, dynamic>>[];
     for (final g in logbook.gearByPk.values) {
-      final map = <String, dynamic>{};
-      if (g.name != null) map['name'] = g.name;
+      final name = g.name;
+      // UddfEntityImporter._importEquipment skips items without a name.
+      // Dropping them here avoids phantom entries in the review UI and
+      // keeps equipmentIdMapping in sync with the emitted entities.
+      if (name == null || name.isEmpty) continue;
+      final uddfId = _gearUddfId(g);
+      if (uddfId == null) continue;
+      final map = <String, dynamic>{'name': name, 'uddfId': uddfId};
       if (g.manufacturer != null) map['brand'] = g.manufacturer;
       if (g.model != null) map['model'] = g.model;
       if (g.serial != null) map['serialNumber'] = g.serial;
@@ -254,13 +272,18 @@ class MacDiveDiveMapper {
     ];
     if (buddyNames.isNotEmpty) map['unmatchedBuddyNames'] = buddyNames;
 
-    // Per-dive gear linkage (`logbook.diveToGearPks`) is read but not
-    // emitted: the unified import payload currently has no dive→gear
-    // reference convention (`equipmentRefs` isn't recognised by the
-    // entity importer), and M2 behaves identically — gear is imported
-    // as standalone equipment entities dedup'd across all dives. Wiring
-    // dive↔gear links end-to-end is a cross-parser change tracked as
-    // follow-up work.
+    // Per-dive gear linkage via `equipmentRefs`. UddfEntityImporter
+    // resolves each ref through `equipmentIdMapping[uddfId]` to find
+    // the newly-created equipment row, so we emit the same uddfId
+    // values produced by `_buildGearMaps` above (MacDive gear UUID,
+    // with name fallback for older exports). Gear items skipped in
+    // the equipment map are also omitted here.
+    final gearPks = logbook.diveToGearPks[d.pk] ?? const <int>[];
+    final equipmentRefs = <String>[
+      for (final gpk in gearPks)
+        if (logbook.gearByPk[gpk] case final g?) ?_gearUddfId(g),
+    ];
+    if (equipmentRefs.isNotEmpty) map['equipmentRefs'] = equipmentRefs;
 
     // Tags - emit names under `tagRefs`.
     final tagPks = logbook.diveToTagPks[d.pk] ?? const <int>[];

lib/features/universal_import/data/services/macdive_raw_types.dart

@@ -59,10 +59,11 @@ class MacDiveRawDive {
   /// `ZSAMPLES` BLOB — MacDive's proprietary profile-sample format
   /// (NOT bplist; left on the model for future decode work, not
   /// used by M3).
-  final Uint8List? samplesBplist;
+  final Uint8List? samplesBlob;
 
-  /// `ZRAWDATA` BLOB — raw dive-computer sensor dump.
-  final Uint8List? rawDataBplist;
+  /// `ZRAWDATA` BLOB — raw dive-computer sensor dump (format varies by
+  /// computer; not bplist).
+  final Uint8List? rawDataBlob;
 
   const MacDiveRawDive({
     required this.pk,
@@ -104,8 +105,8 @@ class MacDiveRawDive {
     this.weight,
     this.diveSiteFk,
     this.certificationFk,
-    this.samplesBplist,
-    this.rawDataBplist,
+    this.samplesBlob,
+    this.rawDataBlob,
   });
 }
 

lib/features/universal_import/data/services/shearwater_db_reader.dart

@@ -115,6 +115,35 @@ LEFT JOIN log_data ld ON dd.DiveId = ld.log_id
 ORDER BY dd.DiveDate
 ''';
 
+  /// Synchronous companion to [isShearwaterCloudDb] for callers that
+  /// have already probed the SQLite table set (e.g. the format detector
+  /// runs every DB-flavor check against one probe to avoid doubling
+  /// temp-file I/O).
+  static bool matchesTables(Set<String> tables) =>
+      _requiredTables.every(tables.contains);
+
+  /// Writes [bytes] to a temp SQLite file, opens it read-only, and
+  /// returns the set of table names. Returns an empty set (doesn't
+  /// throw) for non-SQLite inputs. Shared by the format detector so
+  /// multiple DB-flavor checks can run against one probe.
+  static Future<Set<String>> probeSqliteTableNames(Uint8List bytes) async {
+    final tempPath = _tempPath();
+    final tempFile = File(tempPath);
+    try {
+      await tempFile.writeAsBytes(bytes);
+      final db = sqlite3.open(tempPath, mode: OpenMode.readOnly);
+      try {
+        return _listTables(db);
+      } finally {
+        db.dispose();
+      }
+    } catch (_) {
+      return const <String>{};
+    } finally {
+      _deleteTempFile(tempFile);
+    }
+  }
+
   /// Returns true if the given bytes represent a Shearwater Cloud database.
   ///
   /// Writes the bytes to a temporary file and opens it as SQLite. Checks

lib/features/universal_import/presentation/providers/universal_import_providers.dart

@@ -83,14 +83,18 @@ class UniversalImportNotifier extends StateNotifier<UniversalImportState> {
     var detection = detector.detect(bytes);
 
     if (detection.format == ImportFormat.sqlite) {
-      final isShearwater = await ShearwaterDbReader.isShearwaterCloudDb(bytes);
-      if (isShearwater) {
+      // Probe the SQLite table set once and reuse for each DB flavor
+      // check. Without this, every flavor would re-write the full byte
+      // array to its own temp file and re-open sqlite — wasteful for
+      // large dive databases on mobile/low-end devices.
+      final tables = await ShearwaterDbReader.probeSqliteTableNames(bytes);
+      if (ShearwaterDbReader.matchesTables(tables)) {
         detection = const DetectionResult(
           format: ImportFormat.shearwaterDb,
           sourceApp: SourceApp.shearwater,
           confidence: 0.95,
         );
-      } else if (await MacDiveDbReader.isMacDiveDb(bytes)) {
+      } else if (MacDiveDbReader.matchesTables(tables)) {
         detection = const DetectionResult(
           format: ImportFormat.macdiveSqlite,
           sourceApp: SourceApp.macdive,

test/features/universal_import/data/parsers/macdive_sqlite_parser_test.dart

@@ -68,5 +68,33 @@ void main() {
       expect(payload.isEmpty, isTrue);
       expect(payload.warnings.first.severity, ImportWarningSeverity.error);
     });
+
+    test(
+      'gear entities carry a stable uddfId matching the gear UUID',
+      () async {
+        final payload = await const MacDiveSqliteParser().parse(validBytes);
+        final equipment = payload.entitiesOf(ImportEntityType.equipment);
+        expect(equipment, hasLength(1));
+        final gear = equipment.single;
+        expect(gear['name'], 'Hydros Pro');
+        expect(gear['uddfId'], 'gear-uuid-1');
+        expect(gear['sourceUuid'], 'gear-uuid-1');
+      },
+    );
+
+    test(
+      'dives emit equipmentRefs pointing at gear uddfIds, enabling linking',
+      () async {
+        final payload = await const MacDiveSqliteParser().parse(validBytes);
+        final dives = payload.entitiesOf(ImportEntityType.dives);
+        // Dive 1 in the synthetic fixture has the only dive↔gear junction.
+        final withGear = dives.where(
+          (d) => (d['equipmentRefs'] as List?)?.isNotEmpty ?? false,
+        );
+        expect(withGear, hasLength(1));
+        final refs = withGear.single['equipmentRefs'] as List;
+        expect(refs, ['gear-uuid-1']);
+      },
+    );
   });
 }

test/features/universal_import/data/parsers/macdive_sqlite_real_sample_test.dart

@@ -124,6 +124,38 @@ void main() {
       expect(withTanks, isNotEmpty);
     });
 
+    test('at least one dive has equipmentRefs linking to imported gear', () async {
+      final payload = await const MacDiveSqliteParser().parse(bytes);
+      final dives = payload.entitiesOf(ImportEntityType.dives);
+      final withGear = dives.where(
+        (d) => (d['equipmentRefs'] as List?)?.isNotEmpty ?? false,
+      );
+      expect(
+        withGear,
+        isNotEmpty,
+        reason:
+            'MacDive SQLite has dive↔gear junctions (Z_5RELATIONSHIPGEARITEMS); '
+            'at least one dive in the 540-dive sample should carry equipmentRefs',
+      );
+
+      final equipment = payload.entitiesOf(ImportEntityType.equipment);
+      final uddfIds = equipment
+          .map((g) => g['uddfId'] as String?)
+          .whereType<String>()
+          .toSet();
+      for (final dive in withGear) {
+        for (final ref in (dive['equipmentRefs'] as List).cast<String>()) {
+          expect(
+            uddfIds,
+            contains(ref),
+            reason:
+                'every equipmentRef must resolve to an emitted gear uddfId so '
+                'UddfEntityImporter.equipmentIdMapping picks it up',
+          );
+        }
+      }
+    });
+
     test(
       'profile is always empty (ZSAMPLES proprietary, not decoded)',
       () async {