Skip to content

Add per-device truth for multi-computer dives (PRO-67, PRO-34) - #167

Merged
shaheinm merged 3 commits into
mainfrom
dev/pro-67-multi-computer-device-truth
Jul 30, 2026
Merged

shaheinm merged 3 commits into
mainfrom
dev/pro-67-multi-computer-device-truth

Conversation

@shaheinm

Copy link
Copy Markdown
Contributor

Summary

Implements the multi-computer dive design: one dive record, per-device truth for samples, gases, and deco settings, with the device picker as the source of truth in the UI.

Phase A — import correctness (PRO-67):

  • Extract shared GasMixMergeHelper (dedupe by composition, gasmixIndex remap, per-device device_id) used by both BLE and Shearwater Cloud import paths — Cloud merge previously never remapped sample gas indices
  • Filter imported gases to slots actually referenced by that device's samples (no more phantom programmed-slot gases)
  • Gate Cloud time-proximity merging on device ownership — buddy computers (ownership: other) no longer merge into owned dives
  • Dive-level GF/deco metadata comes from the primary device instead of silent first-wins

Phase B — per-device settings + UI (PRO-34):

  • Migration 018: dive_device_settings table (GF low/high, deco model, salinity, surface pressure, is_primary per computer); populated on all import paths, backfilled on re-import (extends the PRO-62 pattern)
  • DiveDetailView: device picker drives gas list, deco/GF display, and Replay prefill; defaults to primary device on multi-computer dives; subtle footnote when computers disagree ("Varies by computer — showing Petrel")
  • ReplayProfileSheet: GF/deco model prefill from the selected device's settings
  • splitDive moves the device's settings row to the new dive and promotes a new primary on the original
  • DiveService.hasConflictingDeviceSettings() + getDeviceSettings() helpers

Repair path: re-importing the Shearwater Cloud .db fixes existing merged dives (gas attribution, per-device settings rows) — same flow as PRO-62.

Closes PRO-67, PRO-34.

Test plan

  • 413 Swift tests pass (swift test), including new GasMixMergeHelperTests, Cloud ownership-gate/settings tests, and split settings tests
  • make lint clean (SwiftLint 0 violations)
  • macOS build (xcodebuild -scheme Profundum -destination 'platform=macOS')
  • Manual QA: re-import Shearwater .db on a Petrel+Perdix dive; confirm gas list and GF follow the device picker; verify conflict footnote when GFs differ

Made with Cursor

Phase A — import correctness:
- Extract shared GasMixMergeHelper (dedupe by composition, gasmixIndex
  remap, per-device device_id) used by both BLE and Shearwater Cloud
  import paths; Cloud merge previously never remapped sample indices
- Filter imported gases to slots actually referenced by samples
- Gate Cloud time-proximity merging on device ownership so buddy
  computers (ownership: other) no longer merge into owned dives
- Dive-level GF/deco metadata now comes from the primary device
  instead of silent first-wins across computers

Phase B — per-device settings and UI:
- Migration 018: dive_device_settings (GF, deco model, salinity,
  surface pressure, is_primary per computer), populated on all import
  paths and backfilled on re-import
- Device picker drives gas list, deco/GF display, and Replay prefill;
  defaults to primary device with subtle conflict footnote when
  computers disagree
- splitDive moves the device's settings row and promotes a new
  primary on the original dive

Co-authored-by: Cursor <cursoragent@cursor.com>
@shaheinm

Copy link
Copy Markdown
Contributor Author

Self-review (project checklist)

Validation run locally:

  • make lint — SwiftLint 0 violations
  • swift test — 413 tests, 0 failures (Rust core untouched, so rust-test/security/mutants not applicable)
  • macOS build via xcodebuild -scheme Profundum — succeeds, only pre-existing ImportSession Swift-6 warnings
  • Migration 018 is append-only; composite PK + ON DELETE CASCADE on dive_id; GRDB decodes is_primary INTEGER → Bool correctly (covered by split/import tests)

Correctness spot-checks:

  • gasmixIndex remap is nil-safe: a sample referencing an index with no persisted mix stores nil (previously the Cloud path stored dangling raw indices)
  • Cloud new-dive path re-fetches existing mixes per device inside one write transaction — SQLite sees its own writes, so cross-device dedupe and nextMixIndex sequencing are correct
  • Backfill only parses the binary log when a dive-level field or the per-device settings row is missing — re-import of fully populated DBs stays cheap (PRO-62 invariant preserved)
  • Legacy dives (no settings rows yet): deco section falls back to dive-level fields in both the visibility gate and the section body

Known gaps, deliberately deferred:

  1. ExportService does not export dive_device_settings — an export/import round-trip loses per-device settings (recoverable via Shearwater re-import). Suggest a small follow-up ticket.
  2. divesBackfilled counter reports dive-level field backfills only; settings-row-only backfills aren't counted (cosmetic).
  3. Ownership gate uses chain grouping: an ownership: other row between two owned rows within the 2-min window prevents the owned rows from merging with each other. Unlikely in practice (requires a buddy device starting between your two computers).
  4. Devices with settings rows can't be hard-deleted (FK, no cascade) — consistent with existing samples.device_id / gas_mixes.device_id FKs; devices are archived, not deleted.

Manual QA still required (device-dependent): re-import Shearwater .db on a Petrel+Perdix dive; confirm gas list + GF follow the device picker and the conflict footnote appears when GFs differ.

…ings

- Cloud import grouping: buddy-owned rows now form singleton groups
  without breaking the current group, so two owned computers still
  merge when a buddy row falls between them chronologically
- mergeDevices: reassign dive_device_settings rows from loser to
  winner, dropping the loser's row when the dive already has one for
  the winner (avoids PK violation)
- Regression tests for both scenarios

Co-authored-by: Cursor <cursoragent@cursor.com>
@shaheinm

Copy link
Copy Markdown
Contributor Author

Second-opinion review (GPT-5.3 Codex) — findings addressed in f9b9996

The independent review found two major issues, both real and both fixed:

1. Interleaved buddy row broke owned-computer merging. The ownership gate compared each row only to the previous one, so mine A → other B → mine C within the 2-minute window split A and C into separate dives. Fixed: buddy-owned rows now form singleton groups without terminating the current group, so owned computers chain directly. Regression test: testOwnedComputersMergeAcrossInterleavedBuddyRow.

2. mergeDevices didn't reassign dive_device_settings. Device merge (Cloud/BLE duplicate consolidation) moved dives, samples, gas mixes, and fingerprints to the winner but left settings rows pointing at the archived loser. Fixed: settings rows are reassigned, and when a dive already has a winner row the loser's duplicate is dropped instead of violating the composite PK. Regression tests: testMergeDevicesReassignsDeviceSettings, testMergeDevicesDropsDuplicateSettingsRow.

3 (minor). Test coverage gaps — covered by the three tests above.

Full suite: 416 tests, 0 failures. Reviewer verdict was "request changes"; both changes are now in.

@codecov

codecov Bot commented Jul 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

… detection

Codecov flagged untested paths that no existing test reached:
- DiveDeviceSettings.backfillIfMissing (import paths create rows
  directly, so the re-import backfill never fired in tests)
- Legacy dives.fingerprint fallback in backfillMissingMetadata
- DiveService.hasConflictingDeviceSettings (GF and deco model
  disagreement, agreement, and single-device cases)

Co-authored-by: Cursor <cursoragent@cursor.com>
@shaheinm
shaheinm merged commit 4b3f4d7 into main Jul 30, 2026
8 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