Skip to content

Backfill missing dive metadata on Shearwater re-import (PRO-62) - #166

Merged
shaheinm merged 2 commits into
mainfrom
dev/pro-62-replay-sheet-gfs-not-pre-populating-despite-being-imported
Jun 11, 2026
Merged

shaheinm merged 2 commits into
mainfrom
dev/pro-62-replay-sheet-gfs-not-pre-populating-despite-being-imported

Conversation

@shaheinm

Copy link
Copy Markdown
Contributor

Summary

  • Root cause of PRO-62: GF settings are only extracted from the libdivecomputer binary-log parse, and dives imported before that extraction existed have NULL gf_low/gf_high. Fingerprint dedupe made this permanent: re-importing the same Shearwater Cloud .db skipped existing dives entirely, so old rows could never be repaired — and the replay sheet fell back to its 30/70 defaults.
  • Fix: when a re-import encounters an already-known dive (by source fingerprint or legacy fingerprint), it now backfills metadata fields that are nil — GF low/high, deco model, salinity, surface pressure, end GF99 — from the source row before skipping. Existing non-nil values are never overwritten, and the binary log is only re-parsed when at least one target field is actually missing, so re-imports of healthy databases stay cheap.
  • Adds a divesBackfilled counter to ShearwaterCloudImportResult (defaulted, source-compatible).

User action to repair existing data: re-export from Shearwater Cloud and re-import the .db — affected dives will be backfilled in place.

Closes PRO-62.

Tests

  • testReimportBackfillsMissingMetadata — first import populates, fields nulled to simulate an old import, re-import restores them (divesBackfilled == 1, no duplicate dives)
  • testReimportDoesNotBackfillWhenNothingMissing — fully populated dives are untouched and existing values are never overwritten (divesBackfilled == 0)
  • Full Swift suite green locally; SwiftLint strict: 0 violations

Made with Cursor

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 <cursoragent@cursor.com>
@codecov

codecov Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.91549% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ources/Services/ShearwaterCloudImportService.swift 85.91% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

…ckfill

- 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 <cursoragent@cursor.com>
@shaheinm

Copy link
Copy Markdown
Contributor Author

Second-opinion review (gpt-5.3-codex) — no blockers → addressed in latest commit

Should-fixes (both addressed):

  • updatedDiveIds short-circuited multi-computer groups: once row A updated a dive, row B couldn't fill remaining nil fields. → Removed the skip; the set now only counts unique dives. Test: testReimportBackfillsFromSecondComputerRow.
  • Partial-merge path (existingFp != nil) didn't backfill the existing dive. → Backfill now runs after the merge write so the newly inserted fingerprints resolve and the new computer's row fills gaps. Test: testPartialMergeBackfillsMissingMetadata.

Confirmed correct by review: legacy fingerprint fallback, nil-only updates (never overwrites), transaction safety, source-compatible divesBackfilled struct change.

Full Swift suite green, SwiftLint clean.

@shaheinm
shaheinm merged commit bbbace4 into main Jun 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant