From 8b31decda6e6108996395eed636f6243cdace0c8 Mon Sep 17 00:00:00 2001 From: Shahein Moussavi Date: Thu, 11 Jun 2026 10:52:44 -0700 Subject: [PATCH 1/2] Backfill missing dive metadata on Shearwater re-import (PRO-62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dives imported before GF extraction existed have NULL gf_low/gf_high (and related metadata), and fingerprint dedupe meant re-importing the same Shearwater Cloud .db skipped them without ever backfilling — so the replay sheet fell back to 30/70 defaults. Re-imports of already-known dives now backfill nil-only metadata fields (GF low/high, deco model, salinity, surface pressure, end GF99) from the source row. Existing values are never overwritten, and the binary log is only re-parsed when at least one field is missing. New divesBackfilled counter on the import result surfaces the activity. Co-authored-by: Cursor --- .../ShearwaterCloudImportService.swift | 88 ++++++++++++++++++- .../Tests/ShearwaterCloudImportTests.swift | 73 +++++++++++++++ 2 files changed, 160 insertions(+), 1 deletion(-) diff --git a/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift b/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift index d888a3c..801ff2f 100644 --- a/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift +++ b/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift @@ -12,6 +12,9 @@ public struct ShearwaterCloudImportResult: Equatable, Sendable { public var teammatesCreated: Int /// Diagnostic: number of samples with PPO2 sensor data. public var samplesWithPpo2: Int = 0 + /// Existing dives whose missing metadata (GF settings, deco model, etc.) + /// was backfilled during a re-import. + public var divesBackfilled: Int = 0 } /// Imports dive data from a Shearwater Cloud SQLite `.db` export file. @@ -56,6 +59,7 @@ public final class ShearwaterCloudImportService: Sendable { var divesImported = 0 var divesSkipped = 0 var divesMerged = 0 + var divesBackfilled = 0 var devicesCreated = 0 var sitesCreated = 0 var teammatesCreated = 0 @@ -275,6 +279,13 @@ public final class ShearwaterCloudImportService: Sendable { } if allExist { + // Dives imported by older app versions may be missing metadata + // that wasn't extracted at the time (GF settings — PRO-62). + // Backfill nil fields from this row before skipping. + divesBackfilled += try backfillMissingMetadata( + rows: group.map { ($0.row, $0.fingerprint) }, + dateFormatter: dateFormatter + ) divesSkipped += group.count processedRows += group.count for _ in group { @@ -308,6 +319,10 @@ public final class ShearwaterCloudImportService: Sendable { if legacyExists && existingFp == nil { // Legacy dedup — these dives exist but don't have source fingerprints yet + divesBackfilled += try backfillMissingMetadata( + rows: group.map { ($0.row, $0.fingerprint) }, + dateFormatter: dateFormatter + ) divesSkipped += group.count processedRows += group.count for _ in group { @@ -652,10 +667,81 @@ public final class ShearwaterCloudImportService: Sendable { devicesCreated: devicesCreated, sitesCreated: sitesCreated, teammatesCreated: teammatesCreated, - samplesWithPpo2: totalSamplesWithPpo2 + samplesWithPpo2: totalSamplesWithPpo2, + divesBackfilled: divesBackfilled ) } + // MARK: - Metadata Backfill + + /// Backfills metadata fields on already-imported dives that are nil because + /// an older app version didn't extract them (GF settings, deco model, + /// salinity, surface pressure, end GF99 — PRO-62). + /// + /// The binary dive log is only parsed when at least one target field is + /// missing, so re-imports of fully populated databases stay cheap. Existing + /// non-nil values are never overwritten. + /// + /// - Returns: Number of dives updated. + private func backfillMissingMetadata( + rows: [(row: Row, fingerprint: Data)], + dateFormatter: DateFormatter + ) throws -> Int { + var updatedCount = 0 + try database.dbQueue.write { db in + var updatedDiveIds = Set() + for entry in rows { + // Resolve the dive via source fingerprint, falling back to the + // legacy dives.fingerprint column. + let diveId: String? + if let fpRecord = try DiveSourceFingerprint + .filter(Column("fingerprint") == entry.fingerprint) + .fetchOne(db) { + diveId = fpRecord.diveId + } else { + diveId = try Dive + .filter(Column("fingerprint") == entry.fingerprint) + .fetchOne(db)?.id + } + guard let diveId, !updatedDiveIds.contains(diveId), + var dive = try Dive.fetchOne(db, key: diveId) else { continue } + + // Only parse the binary log when something is actually missing. + let needsBackfill = dive.gfLow == nil || dive.gfHigh == nil + || dive.decoModel == nil || dive.salinity == nil + || dive.surfacePressureBar == nil || dive.endGf99 == nil + guard needsBackfill else { continue } + + let calcVals: CalculatedValues? = decodeJSON( + entry.row["calculated_values_from_samples"] as DatabaseValue + ) + let meta: DiveMetadata? = decodeJSON(entry.row["data_bytes_2"] as DatabaseValue) + let parsed = parseRow( + entry.row, dateFormatter: dateFormatter, + calcValues: calcVals, metadata: meta + ) + + var changed = false + if dive.gfLow == nil, let v = parsed.gfLow { dive.gfLow = v; changed = true } + if dive.gfHigh == nil, let v = parsed.gfHigh { dive.gfHigh = v; changed = true } + if dive.decoModel == nil, let v = parsed.decoModel { dive.decoModel = v; changed = true } + if dive.salinity == nil, let v = parsed.salinity { dive.salinity = v; changed = true } + if dive.surfacePressureBar == nil, let v = parsed.surfacePressureBar { + dive.surfacePressureBar = v + changed = true + } + if dive.endGf99 == nil, let v = parsed.endGf99 { dive.endGf99 = v; changed = true } + + if changed { + try dive.update(db) + updatedDiveIds.insert(diveId) + updatedCount += 1 + } + } + } + return updatedCount + } + // MARK: - Row Parsing struct ParseRowResult { diff --git a/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift b/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift index 24ef219..53be1ba 100644 --- a/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift +++ b/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift @@ -948,6 +948,79 @@ final class ShearwaterCloudImportTests: XCTestCase { XCTAssertEqual(fpsAfter.count, 3) } + // MARK: - Metadata Backfill on Re-import (PRO-62) + + /// Dives imported by older app versions can have nil GF/deco metadata. + /// Re-importing the same Shearwater DB must backfill the missing fields + /// instead of skipping the dive entirely. + func testReimportBackfillsMissingMetadata() throws { + let path = try createShearwaterDB(dives: [ + ShearwaterTestDive( + diveId: 1, diveDate: "2024-06-15 10:30:00", depthFt: 100, + durationSec: 3600, serial: "SN001", endGf99: 42.0 + ), + ]) + + let firstImport = try importService.importFromFile(at: path) + XCTAssertEqual(firstImport.divesImported, 1) + XCTAssertEqual(firstImport.divesBackfilled, 0) + + var dive = try diveService.listDives()[0] + XCTAssertEqual(dive.endGf99, 42.0) + + // Simulate a dive imported before metadata extraction existed. + try database.dbQueue.write { db in + try db.execute( + sql: "UPDATE dives SET end_gf99 = NULL, gf_low = NULL, gf_high = NULL WHERE id = ?", + arguments: [dive.id] + ) + } + + let reimport = try importService.importFromFile(at: path) + XCTAssertEqual(reimport.divesImported, 0) + XCTAssertEqual(reimport.divesSkipped, 1) + XCTAssertEqual(reimport.divesBackfilled, 1, "Re-import should backfill nil metadata") + + dive = try diveService.listDives()[0] + XCTAssertEqual(dive.endGf99, 42.0, "endGf99 should be restored from the source row") + // GFs stay nil here because the fixture has no parseable binary log — + // the point is the dive was updated, not skipped. + } + + /// Re-importing fully populated dives must not touch them. + func testReimportDoesNotBackfillWhenNothingMissing() throws { + let path = try createShearwaterDB(dives: [ + ShearwaterTestDive( + diveId: 1, diveDate: "2024-06-15 10:30:00", depthFt: 100, + durationSec: 3600, serial: "SN001", endGf99: 42.0 + ), + ]) + + _ = try importService.importFromFile(at: path) + + // Populate every backfill-target field so nothing is missing. + let diveId = try diveService.listDives()[0].id + try database.dbQueue.write { db in + try db.execute( + sql: """ + UPDATE dives SET gf_low = 50, gf_high = 80, deco_model = 'buhlmann', + salinity = 'salt', surface_pressure_bar = 1.013, end_gf99 = 99.0 + WHERE id = ? + """, + arguments: [diveId] + ) + } + + let reimport = try importService.importFromFile(at: path) + XCTAssertEqual(reimport.divesBackfilled, 0) + + // Existing values must never be overwritten by the source row. + let dive = try diveService.listDives()[0] + XCTAssertEqual(dive.gfLow, 50) + XCTAssertEqual(dive.gfHigh, 80) + XCTAssertEqual(dive.endGf99, 99.0, "Existing endGf99 must not be overwritten") + } + // MARK: - Helper: Create Shearwater Test Database struct ShearwaterTestDive { From 20e0de12a37d017630f11272bdf72a64960141cf Mon Sep 17 00:00:00 2001 From: Shahein Moussavi Date: Thu, 11 Jun 2026 12:16:40 -0700 Subject: [PATCH 2/2] Address PRO-62 review: multi-row group continuation, partial-merge backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - backfillMissingMetadata no longer short-circuits a dive after its first update — a second computer's row in the same group can fill fields the first row couldn't (matches first-non-nil-across-rows merge semantics) - Partial-merge path now backfills the existing dive after the merge write, so newly added fingerprints resolve and the new computer's row can fill gaps - Tests for both cases Co-authored-by: Cursor --- .../ShearwaterCloudImportService.swift | 15 ++++- .../Tests/ShearwaterCloudImportTests.swift | 67 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift b/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift index 801ff2f..2fc6314 100644 --- a/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift +++ b/apple/DivelogCore/Sources/Services/ShearwaterCloudImportService.swift @@ -417,6 +417,13 @@ public final class ShearwaterCloudImportService: Sendable { divesMerged += 1 } } + // Backfill metadata the existing dive may be missing (PRO-62). + // Runs after the merge write so newly inserted fingerprints also + // resolve, letting the new computer's row fill gaps too. + divesBackfilled += try backfillMissingMetadata( + rows: group.map { ($0.row, $0.fingerprint) }, + dateFormatter: dateFormatter + ) processedRows += group.count for _ in group { progress?(processedRows, totalDives) @@ -703,7 +710,11 @@ public final class ShearwaterCloudImportService: Sendable { .filter(Column("fingerprint") == entry.fingerprint) .fetchOne(db)?.id } - guard let diveId, !updatedDiveIds.contains(diveId), + // Note: a dive already updated by an earlier row in the group is + // still re-checked — a second computer's row may fill fields the + // first one couldn't (matches "first non-nil across rows" merge + // semantics). + guard let diveId, var dive = try Dive.fetchOne(db, key: diveId) else { continue } // Only parse the binary log when something is actually missing. @@ -735,9 +746,9 @@ public final class ShearwaterCloudImportService: Sendable { if changed { try dive.update(db) updatedDiveIds.insert(diveId) - updatedCount += 1 } } + updatedCount = updatedDiveIds.count } return updatedCount } diff --git a/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift b/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift index 53be1ba..61ef72b 100644 --- a/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift +++ b/apple/DivelogCore/Tests/ShearwaterCloudImportTests.swift @@ -987,6 +987,73 @@ final class ShearwaterCloudImportTests: XCTestCase { // the point is the dive was updated, not skipped. } + /// In a multi-computer group, a later row must still be able to fill + /// fields the first row couldn't (first-non-nil-across-rows semantics). + func testReimportBackfillsFromSecondComputerRow() throws { + let startTime: Int64 = 1718444400 + let path = try createShearwaterDB(dives: [ + // Computer A: no EndGF99 data + ShearwaterTestDive( + diveId: 100, diveDate: "2024-06-15 10:00:00", depthFt: 100, + durationSec: 3600, serial: "SERIAL_A", + dataBytes2: "{\"DIVE_START_TIME\": \(startTime)}" + ), + // Computer B: has EndGF99 + ShearwaterTestDive( + diveId: 200, diveDate: "2024-06-15 10:00:30", depthFt: 98, + durationSec: 3580, serial: "SERIAL_B", + dataBytes2: "{\"DIVE_START_TIME\": \(startTime + 30)}", endGf99: 55.0 + ), + ]) + + let firstImport = try importService.importFromFile(at: path) + XCTAssertEqual(firstImport.divesImported, 1, "Rows should merge into one dive") + + let diveId = try diveService.listDives()[0].id + try database.dbQueue.write { db in + try db.execute(sql: "UPDATE dives SET end_gf99 = NULL WHERE id = ?", arguments: [diveId]) + } + + let reimport = try importService.importFromFile(at: path) + XCTAssertEqual(reimport.divesBackfilled, 1) + + let dive = try diveService.listDives()[0] + XCTAssertEqual(dive.endGf99, 55.0, "Second computer's row should fill the gap") + } + + /// Partial merge (new computer row added to a known dive) must also + /// backfill missing metadata on the existing dive. + func testPartialMergeBackfillsMissingMetadata() throws { + let startTime: Int64 = 1718444400 + let diveA = ShearwaterTestDive( + diveId: 100, diveDate: "2024-06-15 10:00:00", depthFt: 100, + durationSec: 3600, serial: "SERIAL_A", + dataBytes2: "{\"DIVE_START_TIME\": \(startTime)}" + ) + let diveB = ShearwaterTestDive( + diveId: 200, diveDate: "2024-06-15 10:00:30", depthFt: 98, + durationSec: 3580, serial: "SERIAL_B", + dataBytes2: "{\"DIVE_START_TIME\": \(startTime + 30)}", endGf99: 55.0 + ) + + // First import only computer A (no EndGF99 anywhere). + let pathA = try createShearwaterDB(dives: [diveA]) + let firstImport = try importService.importFromFile(at: pathA) + XCTAssertEqual(firstImport.divesImported, 1) + XCTAssertNil(try diveService.listDives()[0].endGf99) + + // Re-import with both computers: partial merge adds B's row and must + // backfill the existing dive's metadata from it. + let pathAB = try createShearwaterDB(dives: [diveA, diveB]) + let reimport = try importService.importFromFile(at: pathAB) + XCTAssertEqual(reimport.divesMerged, 1, "Computer B's row should partial-merge") + XCTAssertEqual(reimport.divesBackfilled, 1) + + let dives = try diveService.listDives() + XCTAssertEqual(dives.count, 1) + XCTAssertEqual(dives[0].endGf99, 55.0, "New computer's row should backfill the existing dive") + } + /// Re-importing fully populated dives must not touch them. func testReimportDoesNotBackfillWhenNothingMissing() throws { let path = try createShearwaterDB(dives: [